diff options
author | Rémy Coutable <remy@rymai.me> | 2017-01-09 23:47:32 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-01-18 18:38:34 +0300 |
commit | e950830ba6a0efa3b0992e6e55cb5b5842f8573a (patch) | |
tree | 4f4a5a9ddb72466e2bdc8f7df4d16e78b4e5f15c /app/presenters | |
parent | 78874519db2ca2f18a2fbca5fe070292e521d0c1 (diff) |
Fix typos and improve presenters documentation
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'app/presenters')
-rw-r--r-- | app/presenters/README.md | 67 |
1 files changed, 28 insertions, 39 deletions
diff --git a/app/presenters/README.md b/app/presenters/README.md index 38399b5e18b..91c1d2609f5 100644 --- a/app/presenters/README.md +++ b/app/presenters/README.md @@ -8,18 +8,15 @@ methods from models to presenters. ### When your view is full of logic -When your view is full of logic (`if`, `else`, `select` on arrays etc.), it's time -to create a presenter! - -For instance this view is full of logic: https://gitlab.com/gitlab-org/gitlab-ce/blob/d61f8a18e0f7e9d0ed162827f4e8ae2de3756f5c/app/views/projects/builds/_sidebar.html.haml -can be improved as follows: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7073/diffs +When your view is full of logic (`if`, `else`, `select` on arrays etc.), it's +time to create a presenter! ### When your model has a lot of view-related logic/data methods When your model has a lot of view-related logic/data methods, you can easily move them to a presenter. -## Why using presenters instead of helpers? +## Why are we using presenters instead of helpers? We don't use presenters to generate complex view output that would rely on helpers. @@ -28,11 +25,11 @@ Presenters should be used for: - Data and logic methods that can be pulled & combined into single methods from view. This can include loops extracted from views too. A good example is https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7073/diffs. -- Data and logic methods that can be pulled from models +- Data and logic methods that can be pulled from models. - Simple text output methods: it's ok if the method returns a string, but not a whole DOM element for which we'd need HAML, a view context, helpers etc. -## Why using presenters instead of model concerns? +## Why use presenters instead of model concerns? We should strive to follow the single-responsibility principle, and view-related logic/data methods are definitely not the responsibility of models! @@ -53,8 +50,8 @@ By moving pure view-related logic/data methods from models & views to presenters we gain the following benefits: - rules are more explicit and centralized in the presenter => improves security -- makes the testing easier & faster as presenters are Plain Old Ruby Object (PORO) -- makes views much more readable and maintainable +- testing is easier and faster as presenters are Plain Old Ruby Object (PORO) +- views are more readable and maintainable - decreases number of CE -> EE merge conflicts since code is in separate files - moves the conflicts from views (not always obvious) to presenters (a lot easier to resolve) @@ -62,26 +59,24 @@ we gain the following benefits: - Don't use helpers in presenters. Presenters are not aware of the view context. - Don't generate complex DOM elements, forms etc. with presenters. Presenters - can return simple data as texts, and URL using URL helpers from + can return simple data as texts, and URLs using URL helpers from `Gitlab::Routing` but nothing much more fancy. ## Implementation ### Presenter definition -Every presenters should include `Gitlab::View::Presenter`, which provides a -`.presents` method which allows you to define an accessor for the presented -object. It also includes common helpers like `Gitlab::Routing` and +Every presenters should inherit from `Gitlab::View::Presenter::Simple`, which +provides a `.presents` method which allows you to define an accessor for the +presented object. It also includes common helpers like `Gitlab::Routing` and `Gitlab::Allowable`. ```ruby -class LabelPresenter - include Gitlab::View::Presenter - +class LabelPresenter < Gitlab::View::Presenter::Simple presents :label - def blue? - label.color == :blue + def text_color + LabelsHelper.text_color_for_bg(label.color) end def to_partial_path @@ -90,19 +85,17 @@ class LabelPresenter end ``` -In some cases, it can be more practical to transparently delegates all missing +In some cases, it can be more practical to transparently delegate all missing method calls to the presented object, in these cases, you can make your -presenter inherit from `SimpleDelegator`: +presenter inherit from `Gitlab::View::Presenter::Delegated`: ```ruby -class LabelPresenter < SimpleDelegator - include Gitlab::View::Presenter - +class LabelPresenter < Gitlab::View::Presenter::Delegated presents :label - def blue? + def text_color # color is delegated to label - color == :blue + LabelsHelper.text_color_for_bg(color) end def to_partial_path @@ -113,28 +106,24 @@ end ### Presenter instantiation -Instantiation must be done via the `Gitlab::View::PresenterFactory` class which -handles presenters subclassing `SimpleDelegator` as well as those who don't. +Instantiation must be done via the `Gitlab::View::Presenter::Factory` class which +detects the presenter based on the presented subject's class. ```ruby class Projects::LabelsController < Projects::ApplicationController def edit - @label = Gitlab::View::PresenterFactory + @label = Gitlab::View::Presenter::Factory .new(@label, user: current_user) .fabricate! end end ``` -You can also define a method on the model: +You can also include the `Presentable` concern in the model: ```ruby class Label - def present(current_user) - Gitlab::View::PresenterFactory - .new(self, user: current_user) - .fabricate! - end + include Presentable end ``` @@ -151,9 +140,8 @@ end ### Presenter usage ```ruby -= @label.blue? - -= render partial: @label, label: @label +%div{ class: @label.text_color } + = render partial: @label, label: @label ``` You can also present the model in the view: @@ -161,5 +149,6 @@ You can also present the model in the view: ```ruby - label = @label.present(current_user) -= render partial: label, label: label +%div{ class: label.text_color } + = render partial: label, label: label ``` |