From e7a01b741f197eae67af73d7aca8a4c6dabb63d5 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Mon, 1 Jun 2026 14:29:06 +0200 Subject: [PATCH] [OP-19415] Convert FilterForm to ViewComponent Replaces `Filters::FilterForm` (an `ApplicationForm` subclass) with `Filters::FilterFormComponent` (an `ApplicationComponent`). The old form overrode `:nodoc:` Primer hooks (`before_render`, `perform_render`) and read semi-public ivars (`@builder`, `@view_context`). The new component receives the builder as an explicit keyword arg and uses a standard ERB template, reducing Primer internal coupling from five semi-public APIs to one (`FormList`). https://community.openproject.org/wp/OP-19415 --- app/components/filter/filter_component.rb | 2 +- .../filters/filter_form_component.html.erb | 9 ++ .../filters/filter_form_component.rb} | 63 +++++--------- .../filter/filter_for_wp_mixin.rb | 2 +- app/models/query.rb | 4 +- lookbook/docs/patterns/11-filter-forms.md.erb | 84 ++++++++++--------- .../combined_with_other_inputs.html.erb | 6 +- .../filter_form_preview/default.html.erb | 4 +- .../for_a_work_package_query.html.erb | 16 ++-- .../with_active_filter.html.erb | 4 +- .../with_hidden_input.html.erb | 4 +- .../filters/filter_form_component_spec.rb} | 9 +- 12 files changed, 99 insertions(+), 108 deletions(-) create mode 100644 app/components/filters/filter_form_component.html.erb rename app/{forms/filters/filter_form.rb => components/filters/filter_form_component.rb} (72%) rename spec/{forms/filters/filter_form_spec.rb => components/filters/filter_form_component_spec.rb} (95%) diff --git a/app/components/filter/filter_component.rb b/app/components/filter/filter_component.rb index 3c098cecc75..92a4abd260e 100644 --- a/app/components/filter/filter_component.rb +++ b/app/components/filter/filter_component.rb @@ -38,7 +38,7 @@ module Filter options initially_expanded: false def filter_form(form) - Filters::FilterForm.new(form, query:, allowed_filters:) + Filters::FilterFormComponent.new(builder: form, query:, allowed_filters:) end def allowed_filters diff --git a/app/components/filters/filter_form_component.html.erb b/app/components/filters/filter_form_component.html.erb new file mode 100644 index 00000000000..2c5408a8285 --- /dev/null +++ b/app/components/filters/filter_form_component.html.erb @@ -0,0 +1,9 @@ +<% if @wrap_with_controller %> + <%= tag.div(class: "op-filters-form -expanded", data: controller_data_attributes) do %> + <%= hidden_filters_input if @hidden_input_name %> + <%= render(form_list) %> + <% end %> +<% else %> + <%= hidden_filters_input if @hidden_input_name %> + <%= render(form_list) %> +<% end %> diff --git a/app/forms/filters/filter_form.rb b/app/components/filters/filter_form_component.rb similarity index 72% rename from app/forms/filters/filter_form.rb rename to app/components/filters/filter_form_component.rb index ba1f9b4d395..7d19a594cbd 100644 --- a/app/forms/filters/filter_form.rb +++ b/app/components/filters/filter_form_component.rb @@ -31,29 +31,28 @@ # Renders the list of filter input fields (one row per available filter plus an # "add filter" select) for a given query as part of a Primer form. # -# Unlike most primer forms, this form does not declare a static set of inputs -# via the `form do |f| ... end` DSL. The set of inputs depends on the query's -# available and active filters and is built dynamically at render time. The -# form re-uses the builder of the surrounding `primer_form_with` so that the -# emitted field names match what the controller expects (top-level -# `operator_` and `_value` fields). +# The set of inputs depends on the query's available and active filters and is +# built dynamically at render time. The component receives the builder of the +# surrounding `primer_form_with` so that the emitted field names match what the +# controller expects (top-level `operator_` and `_value` fields). # -# Embed it in any primer form like a normal sub-form: +# Embed it in any primer form: # # <%= primer_form_with(url: ...) do |f| %> # <%= f.text_field(name: :title) %> -# <%= render(Filters::FilterForm.new(f, query: @query)) %> +# <%= render(Filters::FilterFormComponent.new(builder: f, query: @query)) %> # <% end %> # # Customise the set of advertised filters by passing `allowed_filters:` (used # by `Filter::FilterComponent` subclasses that restrict or reorder the list). # -# By default the form does *not* attach the `filter--filters-form` Stimulus +# By default the component does *not* attach the `filter--filters-form` Stimulus # controller, because in the standard layout (e.g. `Projects::IndexSubHeaderComponent`) # the controller has to sit on a common ancestor of the advanced filter form # *and* the inline quick filter input so that `sendForm()` can collect values # from both. For standalone embeds with no co-located quick filter, pass -# `wrap_with_controller: true` and the form will emit its own controller wrapper. +# `wrap_with_controller: true` and the component will emit its own controller +# wrapper. # # Pass `hidden_input_name:` (e.g. `"filters"`) to also emit a hidden input # bound to the Stimulus controller's `filtersInput` target. The controller @@ -65,24 +64,27 @@ # hidden field (and into the URL when `sendForm` redirects). Supported values: # * `:params` (default) — URL-style string: `name ~ "foo"&login ! "bar"`. # * `:json` — JSON array: `[{"name":{"operator":"~","values":["foo"]}}, ...]`. -# Only meaningful when this form owns the controller (`wrap_with_controller: true`); -# otherwise the host's controller wrapper decides. +# Only meaningful when this component owns the controller +# (`wrap_with_controller: true`); otherwise the host's controller wrapper +# decides. # # `autocomplete_append_to:` forwards an `appendTo` selector (or DOM reference # string ng-select understands, e.g. `"#my-dialog"` or `"body"`) to every -# autocompleter the form renders. Use this when the form is embedded in a -# Primer dialog or another container that clips overflow, so the dropdown +# autocompleter the component renders. Use this when the component is embedded +# in a Primer dialog or another container that clips overflow, so the dropdown # portal renders outside that container instead of being clipped. -class Filters::FilterForm < ApplicationForm +class Filters::FilterFormComponent < ApplicationComponent OUTPUT_FORMATS = %i[params json].freeze - def initialize(query:, + def initialize(builder:, + query:, allowed_filters: nil, wrap_with_controller: false, hidden_input_name: nil, output_format: nil, autocomplete_append_to: nil) super() + @builder = builder @query = query @allowed_filters = allowed_filters || query.available_advanced_filters @wrap_with_controller = wrap_with_controller @@ -91,31 +93,14 @@ class Filters::FilterForm < ApplicationForm @autocomplete_append_to = autocomplete_append_to end - # Skip the autofocus traversal `Primer::Forms::Base#before_render` performs: - # it walks `inputs`, which requires a static `form do |f| ... end` block. - # The sub-forms rendered via `FormList` run their own `before_render`. - def before_render; end - - def perform_render(&) - list = @view_context.render(Primer::Forms::FormList.new(*sub_forms)) - content = @hidden_input_name ? @view_context.safe_join([hidden_filters_input, list]) : list - return content unless @wrap_with_controller - - # `op-filters-form -expanded` carries the layout styles for the filter - # rows (label on its own line above operator/value) and makes the form - # visible (`op-filters-form` alone is `display: none`). - @view_context.content_tag( - :div, - content, - class: "op-filters-form -expanded", - data: controller_data_attributes - ) - end - private attr_reader :query, :allowed_filters + def form_list + Primer::Forms::FormList.new(*sub_forms) + end + def controller_data_attributes attrs = { controller: "filter--filters-form" } attrs["filter--filters-form-output-format-value"] = @output_format.to_s if @output_format @@ -134,7 +119,7 @@ class Filters::FilterForm < ApplicationForm end def hidden_filters_input - @view_context.hidden_field_tag( + hidden_field_tag( @hidden_input_name, "", data: { "filter--filters-form-target": "filtersInput" } @@ -154,8 +139,6 @@ class Filters::FilterForm < ApplicationForm ) end - # Maps over all filters (active and inactive). - # In case a filter is active, the active one will be preferred over the inactive one. def map_filter allowed_filters.map do |allowed_filter| active_filter = query.find_active_filter(allowed_filter.name) diff --git a/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb b/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb index d2cef34cb11..9e75a32abb2 100644 --- a/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb +++ b/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb @@ -37,7 +37,7 @@ module Queries::WorkPackages::Filter::FilterForWpMixin raise NotImplementedError, "There would be too many candidates" end - # Tell `Filters::FilterForm`'s dispatch to render these filters with a + # Tell `Filters::FilterFormComponent`'s dispatch to render these filters with a # server-side autocompleter (the candidate set is too large for an inline # `