Skip to content

Commit a5ee2b5

Browse files
committed
[#75446] 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/75446
1 parent c4cfbb2 commit a5ee2b5

12 files changed

Lines changed: 98 additions & 108 deletions

File tree

app/components/filter/filter_component.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class FilterComponent < ApplicationComponent
3838
options initially_expanded: false
3939

4040
def filter_form(form)
41-
Filters::FilterForm.new(form, query:, allowed_filters:)
41+
Filters::FilterFormComponent.new(builder: form, query:, allowed_filters:)
4242
end
4343

4444
def allowed_filters
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<% if @wrap_with_controller %>
2+
<%= tag.div(class: "op-filters-form -expanded", data: controller_data_attributes) do %>
3+
<%= hidden_filters_input if @hidden_input_name %>
4+
<%= render(form_list) %>
5+
<% end %>
6+
<% else %>
7+
<%= hidden_filters_input if @hidden_input_name %>
8+
<%= render(form_list) %>
9+
<% end %>

app/forms/filters/filter_form.rb renamed to app/components/filters/filter_form_component.rb

Lines changed: 22 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,28 @@
3131
# Renders the list of filter input fields (one row per available filter plus an
3232
# "add filter" select) for a given query as part of a Primer form.
3333
#
34-
# Unlike most primer forms, this form does not declare a static set of inputs
35-
# via the `form do |f| ... end` DSL. The set of inputs depends on the query's
36-
# available and active filters and is built dynamically at render time. The
37-
# form re-uses the builder of the surrounding `primer_form_with` so that the
38-
# emitted field names match what the controller expects (top-level
39-
# `operator_<filter>` and `<filter>_value` fields).
34+
# The set of inputs depends on the query's available and active filters and is
35+
# built dynamically at render time. The component receives the builder of the
36+
# surrounding `primer_form_with` so that the emitted field names match what the
37+
# controller expects (top-level `operator_<filter>` and `<filter>_value` fields).
4038
#
41-
# Embed it in any primer form like a normal sub-form:
39+
# Embed it in any primer form:
4240
#
4341
# <%= primer_form_with(url: ...) do |f| %>
4442
# <%= f.text_field(name: :title) %>
45-
# <%= render(Filters::FilterForm.new(f, query: @query)) %>
43+
# <%= render(Filters::FilterFormComponent.new(builder: f, query: @query)) %>
4644
# <% end %>
4745
#
4846
# Customise the set of advertised filters by passing `allowed_filters:` (used
4947
# by `Filter::FilterComponent` subclasses that restrict or reorder the list).
5048
#
51-
# By default the form does *not* attach the `filter--filters-form` Stimulus
49+
# By default the component does *not* attach the `filter--filters-form` Stimulus
5250
# controller, because in the standard layout (e.g. `Projects::IndexSubHeaderComponent`)
5351
# the controller has to sit on a common ancestor of the advanced filter form
5452
# *and* the inline quick filter input so that `sendForm()` can collect values
5553
# from both. For standalone embeds with no co-located quick filter, pass
56-
# `wrap_with_controller: true` and the form will emit its own controller wrapper.
54+
# `wrap_with_controller: true` and the component will emit its own controller
55+
# wrapper.
5756
#
5857
# Pass `hidden_input_name:` (e.g. `"filters"`) to also emit a hidden input
5958
# bound to the Stimulus controller's `filtersInput` target. The controller
@@ -65,24 +64,26 @@
6564
# hidden field (and into the URL when `sendForm` redirects). Supported values:
6665
# * `:params` (default) — URL-style string: `name ~ "foo"&login ! "bar"`.
6766
# * `:json` — JSON array: `[{"name":{"operator":"~","values":["foo"]}}, ...]`.
68-
# Only meaningful when this form owns the controller (`wrap_with_controller: true`);
69-
# otherwise the host's controller wrapper decides.
67+
# Only meaningful when this component owns the controller
68+
# (`wrap_with_controller: true`); otherwise the host's controller wrapper
69+
# decides.
7070
#
7171
# `autocomplete_append_to:` forwards an `appendTo` selector (or DOM reference
7272
# string ng-select understands, e.g. `"#my-dialog"` or `"body"`) to every
73-
# autocompleter the form renders. Use this when the form is embedded in a
74-
# Primer dialog or another container that clips overflow, so the dropdown
73+
# autocompleter the component renders. Use this when the component is embedded
74+
# in a Primer dialog or another container that clips overflow, so the dropdown
7575
# portal renders outside that container instead of being clipped.
76-
class Filters::FilterForm < ApplicationForm
76+
class Filters::FilterFormComponent < ApplicationComponent
7777
OUTPUT_FORMATS = %i[params json].freeze
7878

79-
def initialize(query:,
79+
def initialize(builder:, query:,
8080
allowed_filters: nil,
8181
wrap_with_controller: false,
8282
hidden_input_name: nil,
8383
output_format: nil,
8484
autocomplete_append_to: nil)
8585
super()
86+
@builder = builder
8687
@query = query
8788
@allowed_filters = allowed_filters || query.available_advanced_filters
8889
@wrap_with_controller = wrap_with_controller
@@ -91,31 +92,14 @@ def initialize(query:,
9192
@autocomplete_append_to = autocomplete_append_to
9293
end
9394

94-
# Skip the autofocus traversal `Primer::Forms::Base#before_render` performs:
95-
# it walks `inputs`, which requires a static `form do |f| ... end` block.
96-
# The sub-forms rendered via `FormList` run their own `before_render`.
97-
def before_render; end
98-
99-
def perform_render(&)
100-
list = @view_context.render(Primer::Forms::FormList.new(*sub_forms))
101-
content = @hidden_input_name ? @view_context.safe_join([hidden_filters_input, list]) : list
102-
return content unless @wrap_with_controller
103-
104-
# `op-filters-form -expanded` carries the layout styles for the filter
105-
# rows (label on its own line above operator/value) and makes the form
106-
# visible (`op-filters-form` alone is `display: none`).
107-
@view_context.content_tag(
108-
:div,
109-
content,
110-
class: "op-filters-form -expanded",
111-
data: controller_data_attributes
112-
)
113-
end
114-
11595
private
11696

11797
attr_reader :query, :allowed_filters
11898

99+
def form_list
100+
Primer::Forms::FormList.new(*sub_forms)
101+
end
102+
119103
def controller_data_attributes
120104
attrs = { controller: "filter--filters-form" }
121105
attrs["filter--filters-form-output-format-value"] = @output_format.to_s if @output_format
@@ -134,7 +118,7 @@ def validate_output_format(format)
134118
end
135119

136120
def hidden_filters_input
137-
@view_context.hidden_field_tag(
121+
hidden_field_tag(
138122
@hidden_input_name,
139123
"",
140124
data: { "filter--filters-form-target": "filtersInput" }
@@ -154,8 +138,6 @@ def sub_forms
154138
)
155139
end
156140

157-
# Maps over all filters (active and inactive).
158-
# In case a filter is active, the active one will be preferred over the inactive one.
159141
def map_filter
160142
allowed_filters.map do |allowed_filter|
161143
active_filter = query.find_active_filter(allowed_filter.name)

app/models/queries/work_packages/filter/filter_for_wp_mixin.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def allowed_values
3737
raise NotImplementedError, "There would be too many candidates"
3838
end
3939

40-
# Tell `Filters::FilterForm`'s dispatch to render these filters with a
40+
# Tell `Filters::FilterFormComponent`'s dispatch to render these filters with a
4141
# server-side autocompleter (the candidate set is too large for an inline
4242
# `<select>`). Mirrors what the legacy Angular WP filter UI does — see
4343
# `filter-searchable-multiselect-value.component.html`, which renders an

app/models/query.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ def remove_filter(name)
210210
end
211211

212212
# Mirrors `Queries::BaseQuery#find_active_filter` so that consumers built
213-
# on top of the modern query API (e.g. `Filters::FilterForm`) can ask any
213+
# on top of the modern query API (e.g. `Filters::FilterFormComponent`) can ask any
214214
# query — including this legacy work-package one — for its active filter
215215
# by name. Signature kept identical to BaseQuery's (symbol arg in, filter
216216
# or nil out).
@@ -221,7 +221,7 @@ def find_active_filter(name)
221221
# The manual-sort filter is added programmatically when the user drags
222222
# work packages to reorder them — it has no operator/value UI of its own
223223
# (type `:empty_value`), so it doesn't belong in the picker that
224-
# `Filters::FilterForm` builds. Mirrors how
224+
# `Filters::FilterFormComponent` builds. Mirrors how
225225
# `Queries::Filters::AvailableFilters#available_advanced_filters` already
226226
# excludes the inline `name_and_identifier` quick-filter on projects.
227227
def available_advanced_filters

lookbook/docs/patterns/11-filter-forms.md.erb

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ building blocks for rendering those filters as a UI:
77
index pages: a turbo frame, a `BorderBox`, lazy loading, action buttons
88
(Apply / Close), and the filter rows themselves. Use this when you want
99
the standard OpenProject filter dropdown experience.
10-
* **`Filters::FilterForm`** — just the filter rows plus the "Add filter"
11-
select, rendered as a primer form object. Use this when you want filters
12-
inside *your own* form (e.g. a configuration dialog) or when the
10+
* **`Filters::FilterFormComponent`** — just the filter rows plus the "Add
11+
filter" select, rendered as a ViewComponent. Use this when you want
12+
filters inside *your own* form (e.g. a configuration dialog) or when the
1313
surrounding chrome of `FilterComponent` doesn't fit.
1414

15-
Internally `FilterComponent` delegates to `FilterForm`, so the two stay in
16-
lockstep — `FilterForm` is the single source of truth for which inputs to
17-
render, and `FilterComponent` adds the wrapping chrome on top.
15+
Internally `FilterComponent` delegates to `FilterFormComponent`, so the two
16+
stay in lockstep — `FilterFormComponent` is the single source of truth for
17+
which inputs to render, and `FilterComponent` adds the wrapping chrome on
18+
top.
1819

1920
## Filter::FilterComponent
2021

@@ -50,17 +51,17 @@ rendering until the dropdown is opened (a skeleton is shown in the meantime).
5051

5152
<%= embed OpenProject::Filter::FiltersComponentPreview, :default, panels: %i[preview source] %>
5253

53-
## Filters::FilterForm
54+
## Filters::FilterFormComponent
5455

55-
For everything that isn't a standard filter dropdown, render `FilterForm`
56-
inside any `primer_form_with` block. It accepts the parent's primer form
57-
builder so its inputs sit at the top level of the surrounding form (field
58-
names like `operator_<filter>` and `<filter>_value` — same as
59-
`FilterComponent`).
56+
For everything that isn't a standard filter dropdown, render
57+
`FilterFormComponent` inside any `primer_form_with` block. It accepts the
58+
parent's primer form builder so its inputs sit at the top level of the
59+
surrounding form (field names like `operator_<filter>` and `<filter>_value`
60+
— same as `FilterComponent`).
6061

6162
### Default usage
6263

63-
`wrap_with_controller: true` makes the form emit its own
64+
`wrap_with_controller: true` makes the component emit its own
6465
`<div class="op-filters-form -expanded" data-controller="filter--filters-form">`
6566
wrapper. Use this in any standalone embed (a dialog body, a settings page,
6667
etc.) where there's no surrounding sub-header attaching the controller for
@@ -78,11 +79,11 @@ hidden until the user picks them from the "Add filter" select.
7879

7980
### Submitting via a hidden field
8081

81-
By default the form relies on the Stimulus controller to redirect via
82+
By default the component relies on the Stimulus controller to redirect via
8283
`sendForm` (the projects-index style). Inside a regular form you usually
8384
want the filter state to ride along with a normal submit instead. Pass
84-
`hidden_input_name:` and the form renders a hidden input whose value is
85-
kept in sync with the serialized filter selections.
85+
`hidden_input_name:` and the component renders a hidden input whose value
86+
is kept in sync with the serialized filter selections.
8687

8788
`output_format:` controls the serialization:
8889

@@ -97,25 +98,25 @@ The host server receives the canonical string in
9798

9899
### Combining with non-filter inputs
99100

100-
`FilterForm` is a regular primer form object, so it composes with other
101-
forms via `Primer::Forms::FormList`. All children share the same builder
102-
and therefore submit through the same `<form>`.
101+
`FilterFormComponent` composes with other forms via
102+
`Primer::Forms::FormList`. All children share the same builder and
103+
therefore submit through the same `<form>`.
103104

104105
<%= embed OpenProject::Filter::FilterFormPreview, :combined_with_other_inputs, panels: %i[preview source] %>
105106

106107
### Inside a clipping container (dialogs)
107108

108-
ng-select dropdowns are positioned by their parent; if the form lives
109+
ng-select dropdowns are positioned by their parent; if the component lives
109110
inside a Primer dialog or any other overflow-clipping container, the
110111
dropdown gets cut off. Pass `autocomplete_append_to:` with a CSS selector
111112
that ng-select can resolve (typically the dialog id, or `"body"`) — the
112-
form forwards it as `appendTo` to every autocompleter it renders.
113+
component forwards it as `appendTo` to every autocompleter it renders.
113114

114115
```erb
115116
<%%= primer_form_with(...) do |f| %>
116117
<%%= render(
117-
Filters::FilterForm.new(
118-
f,
118+
Filters::FilterFormComponent.new(
119+
builder: f,
119120
query: @query,
120121
wrap_with_controller: true,
121122
hidden_input_name: "filters",
@@ -130,7 +131,7 @@ form forwards it as `appendTo` to every autocompleter it renders.
130131
| You want… | Use |
131132
|----------------------------------------------------------|---------------------------|
132133
| The standard OpenProject filter panel on an index page | `Filter::FilterComponent` |
133-
| Filters inside a dialog or a non-filter form | `Filters::FilterForm` + `hidden_input_name:` |
134+
| Filters inside a dialog or a non-filter form | `Filters::FilterFormComponent` + `hidden_input_name:` |
134135

135136
## Stimulus controller placement
136137

@@ -148,26 +149,27 @@ to forget about:
148149
That's why `Filter::FilterComponent` does *not* attach the controller
149150
itself — the surrounding `IndexSubHeaderComponent` does, so quick filter
150151
and advanced form share one. For standalone embeds without a co-located
151-
quick filter, `FilterForm`'s `wrap_with_controller: true` is the right
152-
default.
152+
quick filter, `FilterFormComponent`'s `wrap_with_controller: true` is the
153+
right default.
153154

154155
## Compatibility with the legacy `Query` (work packages)
155156

156-
`Filters::FilterForm` reads three things off the query: `available_advanced_filters`,
157-
`filters`, and `find_active_filter(name)`. The first two come from the
158-
`Queries::Filters::AvailableFilters` concern, which the legacy work-package
159-
`Query` model also includes. `find_active_filter` is defined directly on
160-
`Queries::BaseQuery` and used to live only there — `Query` now mirrors it
161-
with the same signature, so passing a `Query` (or any of its subclasses)
162-
to `FilterForm` works exactly like passing a `BaseQuery` subclass.
157+
`Filters::FilterFormComponent` reads three things off the query:
158+
`available_advanced_filters`, `filters`, and `find_active_filter(name)`.
159+
The first two come from the `Queries::Filters::AvailableFilters` concern,
160+
which the legacy work-package `Query` model also includes.
161+
`find_active_filter` is defined directly on `Queries::BaseQuery` and used
162+
to live only there — `Query` now mirrors it with the same signature, so
163+
passing a `Query` (or any of its subclasses) to `FilterFormComponent` works
164+
exactly like passing a `BaseQuery` subclass.
163165

164166
<%= embed OpenProject::Filter::FilterFormPreview, :for_a_work_package_query, panels: %i[preview source] %>
165167

166-
What `FilterForm` does *not* do for you on the legacy side: parsing the
167-
form submission back into a `Query#filters` collection. The work-package
168-
filter pipeline still uses its own serialization (URL `filters=[...]`
169-
JSON / YAML in the DB), so a controller receiving a `FilterForm` submit
170-
either needs to use `hidden_input_name:` with a format the existing
171-
parser understands, or translate the `operator_<name>` / `<name>_value`
172-
fields by hand. The form renders fine either way; what to do with the
173-
submitted values is the caller's call.
168+
What `FilterFormComponent` does *not* do for you on the legacy side:
169+
parsing the form submission back into a `Query#filters` collection. The
170+
work-package filter pipeline still uses its own serialization (URL
171+
`filters=[...]` JSON / YAML in the DB), so a controller receiving a
172+
`FilterFormComponent` submit either needs to use `hidden_input_name:` with
173+
a format the existing parser understands, or translate the
174+
`operator_<name>` / `<name>_value` fields by hand. The component renders
175+
fine either way; what to do with the submitted values is the caller's call.

lookbook/previews/open_project/filter/filter_form_preview/combined_with_other_inputs.html.erb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<%# `Filters::FilterForm` is a regular primer form object — combine it with %>
1+
<%# `Filters::FilterFormComponent` is a component — combine it with %>
22
<%# other forms in a single `Primer::Forms::FormList` to share one builder %>
33
<%# (and therefore one submission) with non-filter inputs. %>
44
<%
@@ -13,8 +13,8 @@
1313
render(
1414
Primer::Forms::FormList.new(
1515
note_form.new(f),
16-
Filters::FilterForm.new(
17-
f,
16+
Filters::FilterFormComponent.new(
17+
builder: f,
1818
query: query,
1919
wrap_with_controller: true,
2020
hidden_input_name: "filters"

lookbook/previews/open_project/filter/filter_form_preview/default.html.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
44
<%=
55
render(
6-
Filters::FilterForm.new(
7-
f,
6+
Filters::FilterFormComponent.new(
7+
builder: f,
88
query: query,
99
wrap_with_controller: true
1010
)

lookbook/previews/open_project/filter/filter_form_preview/for_a_work_package_query.html.erb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
<%# `Query` is the legacy work-package query model. `Filters::FilterForm` %>
2-
<%# works against it the same way it does against `Queries::BaseQuery` %>
3-
<%# subclasses — the only requirement is `available_advanced_filters`, %>
4-
<%# `filters`, and `find_active_filter(name)`, all of which `Query` %>
5-
<%# exposes (the last one was mirrored from `BaseQuery` to bring the %>
6-
<%# legacy query in line with the new form). %>
1+
<%# `Query` is the legacy work-package query model. %>
2+
<%# `Filters::FilterFormComponent` works against it the same way it does %>
3+
<%# against `Queries::BaseQuery` subclasses — the only requirement is %>
4+
<%# `available_advanced_filters`, `filters`, and `find_active_filter(name)`, %>
5+
<%# all of which `Query` exposes (the last one was mirrored from `BaseQuery` %>
6+
<%# to bring the legacy query in line with the new component). %>
77
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
88
<%=
99
render(
10-
Filters::FilterForm.new(
11-
f,
10+
Filters::FilterFormComponent.new(
11+
builder: f,
1212
query: query,
1313
wrap_with_controller: true
1414
)

lookbook/previews/open_project/filter/filter_form_preview/with_active_filter.html.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
55
<%=
66
render(
7-
Filters::FilterForm.new(
8-
f,
7+
Filters::FilterFormComponent.new(
8+
builder: f,
99
query: query,
1010
wrap_with_controller: true
1111
)

0 commit comments

Comments
 (0)