mirror of
https://github.com/opf/openproject.git
synced 2026-06-13 19:20:00 +00:00
[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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 %>
|
||||
+23
-40
@@ -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_<filter>` and `<filter>_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_<filter>` and `<filter>_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)
|
||||
@@ -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
|
||||
# `<select>`). Mirrors what the legacy Angular WP filter UI does — see
|
||||
# `filter-searchable-multiselect-value.component.html`, which renders an
|
||||
|
||||
+2
-2
@@ -210,7 +210,7 @@ class Query < ApplicationRecord
|
||||
end
|
||||
|
||||
# Mirrors `Queries::BaseQuery#find_active_filter` so that consumers built
|
||||
# on top of the modern query API (e.g. `Filters::FilterForm`) can ask any
|
||||
# on top of the modern query API (e.g. `Filters::FilterFormComponent`) can ask any
|
||||
# query — including this legacy work-package one — for its active filter
|
||||
# by name. Signature kept identical to BaseQuery's (symbol arg in, filter
|
||||
# or nil out).
|
||||
@@ -221,7 +221,7 @@ class Query < ApplicationRecord
|
||||
# The manual-sort filter is added programmatically when the user drags
|
||||
# work packages to reorder them — it has no operator/value UI of its own
|
||||
# (type `:empty_value`), so it doesn't belong in the picker that
|
||||
# `Filters::FilterForm` builds. Mirrors how
|
||||
# `Filters::FilterFormComponent` builds. Mirrors how
|
||||
# `Queries::Filters::AvailableFilters#available_advanced_filters` already
|
||||
# excludes the inline `name_and_identifier` quick-filter on projects.
|
||||
def available_advanced_filters
|
||||
|
||||
@@ -7,14 +7,15 @@ building blocks for rendering those filters as a UI:
|
||||
index pages: a turbo frame, a `BorderBox`, lazy loading, action buttons
|
||||
(Apply / Close), and the filter rows themselves. Use this when you want
|
||||
the standard OpenProject filter dropdown experience.
|
||||
* **`Filters::FilterForm`** — just the filter rows plus the "Add filter"
|
||||
select, rendered as a primer form object. Use this when you want filters
|
||||
inside *your own* form (e.g. a configuration dialog) or when the
|
||||
* **`Filters::FilterFormComponent`** — just the filter rows plus the "Add
|
||||
filter" select, rendered as a ViewComponent. Use this when you want
|
||||
filters inside *your own* form (e.g. a configuration dialog) or when the
|
||||
surrounding chrome of `FilterComponent` doesn't fit.
|
||||
|
||||
Internally `FilterComponent` delegates to `FilterForm`, so the two stay in
|
||||
lockstep — `FilterForm` is the single source of truth for which inputs to
|
||||
render, and `FilterComponent` adds the wrapping chrome on top.
|
||||
Internally `FilterComponent` delegates to `FilterFormComponent`, so the two
|
||||
stay in lockstep — `FilterFormComponent` is the single source of truth for
|
||||
which inputs to render, and `FilterComponent` adds the wrapping chrome on
|
||||
top.
|
||||
|
||||
## Filter::FilterComponent
|
||||
|
||||
@@ -50,17 +51,17 @@ rendering until the dropdown is opened (a skeleton is shown in the meantime).
|
||||
|
||||
<%= embed OpenProject::Filter::FiltersComponentPreview, :default, panels: %i[preview source] %>
|
||||
|
||||
## Filters::FilterForm
|
||||
## Filters::FilterFormComponent
|
||||
|
||||
For everything that isn't a standard filter dropdown, render `FilterForm`
|
||||
inside any `primer_form_with` block. It accepts the parent's primer form
|
||||
builder so its inputs sit at the top level of the surrounding form (field
|
||||
names like `operator_<filter>` and `<filter>_value` — same as
|
||||
`FilterComponent`).
|
||||
For everything that isn't a standard filter dropdown, render
|
||||
`FilterFormComponent` inside any `primer_form_with` block. It accepts the
|
||||
parent's primer form builder so its inputs sit at the top level of the
|
||||
surrounding form (field names like `operator_<filter>` and `<filter>_value`
|
||||
— same as `FilterComponent`).
|
||||
|
||||
### Default usage
|
||||
|
||||
`wrap_with_controller: true` makes the form emit its own
|
||||
`wrap_with_controller: true` makes the component emit its own
|
||||
`<div class="op-filters-form -expanded" data-controller="filter--filters-form">`
|
||||
wrapper. Use this in any standalone embed (a dialog body, a settings page,
|
||||
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.
|
||||
|
||||
### Submitting via a hidden field
|
||||
|
||||
By default the form relies on the Stimulus controller to redirect via
|
||||
By default the component relies on the Stimulus controller to redirect via
|
||||
`sendForm` (the projects-index style). Inside a regular form you usually
|
||||
want the filter state to ride along with a normal submit instead. Pass
|
||||
`hidden_input_name:` and the form renders a hidden input whose value is
|
||||
kept in sync with the serialized filter selections.
|
||||
`hidden_input_name:` and the component renders a hidden input whose value
|
||||
is kept in sync with the serialized filter selections.
|
||||
|
||||
`output_format:` controls the serialization:
|
||||
|
||||
@@ -97,25 +98,25 @@ The host server receives the canonical string in
|
||||
|
||||
### Combining with non-filter inputs
|
||||
|
||||
`FilterForm` is a regular primer form object, so it composes with other
|
||||
forms via `Primer::Forms::FormList`. All children share the same builder
|
||||
and therefore submit through the same `<form>`.
|
||||
`FilterFormComponent` composes with other forms via
|
||||
`Primer::Forms::FormList`. All children share the same builder and
|
||||
therefore submit through the same `<form>`.
|
||||
|
||||
<%= embed OpenProject::Filter::FilterFormPreview, :combined_with_other_inputs, panels: %i[preview source] %>
|
||||
|
||||
### Inside a clipping container (dialogs)
|
||||
|
||||
ng-select dropdowns are positioned by their parent; if the form lives
|
||||
ng-select dropdowns are positioned by their parent; if the component lives
|
||||
inside a Primer dialog or any other overflow-clipping container, the
|
||||
dropdown gets cut off. Pass `autocomplete_append_to:` with a CSS selector
|
||||
that ng-select can resolve (typically the dialog id, or `"body"`) — the
|
||||
form forwards it as `appendTo` to every autocompleter it renders.
|
||||
component forwards it as `appendTo` to every autocompleter it renders.
|
||||
|
||||
```erb
|
||||
<%%= primer_form_with(...) do |f| %>
|
||||
<%%= render(
|
||||
Filters::FilterForm.new(
|
||||
f,
|
||||
Filters::FilterFormComponent.new(
|
||||
builder: f,
|
||||
query: @query,
|
||||
wrap_with_controller: true,
|
||||
hidden_input_name: "filters",
|
||||
@@ -130,7 +131,7 @@ form forwards it as `appendTo` to every autocompleter it renders.
|
||||
| You want… | Use |
|
||||
|----------------------------------------------------------|---------------------------|
|
||||
| The standard OpenProject filter panel on an index page | `Filter::FilterComponent` |
|
||||
| Filters inside a dialog or a non-filter form | `Filters::FilterForm` + `hidden_input_name:` |
|
||||
| Filters inside a dialog or a non-filter form | `Filters::FilterFormComponent` + `hidden_input_name:` |
|
||||
|
||||
## Stimulus controller placement
|
||||
|
||||
@@ -148,26 +149,27 @@ to forget about:
|
||||
That's why `Filter::FilterComponent` does *not* attach the controller
|
||||
itself — the surrounding `IndexSubHeaderComponent` does, so quick filter
|
||||
and advanced form share one. For standalone embeds without a co-located
|
||||
quick filter, `FilterForm`'s `wrap_with_controller: true` is the right
|
||||
default.
|
||||
quick filter, `FilterFormComponent`'s `wrap_with_controller: true` is the
|
||||
right default.
|
||||
|
||||
## Compatibility with the legacy `Query` (work packages)
|
||||
|
||||
`Filters::FilterForm` reads three things off the query: `available_advanced_filters`,
|
||||
`filters`, and `find_active_filter(name)`. The first two come from the
|
||||
`Queries::Filters::AvailableFilters` concern, which the legacy work-package
|
||||
`Query` model also includes. `find_active_filter` is defined directly on
|
||||
`Queries::BaseQuery` and used to live only there — `Query` now mirrors it
|
||||
with the same signature, so passing a `Query` (or any of its subclasses)
|
||||
to `FilterForm` works exactly like passing a `BaseQuery` subclass.
|
||||
`Filters::FilterFormComponent` reads three things off the query:
|
||||
`available_advanced_filters`, `filters`, and `find_active_filter(name)`.
|
||||
The first two come from the `Queries::Filters::AvailableFilters` concern,
|
||||
which the legacy work-package `Query` model also includes.
|
||||
`find_active_filter` is defined directly on `Queries::BaseQuery` and used
|
||||
to live only there — `Query` now mirrors it with the same signature, so
|
||||
passing a `Query` (or any of its subclasses) to `FilterFormComponent` works
|
||||
exactly like passing a `BaseQuery` subclass.
|
||||
|
||||
<%= embed OpenProject::Filter::FilterFormPreview, :for_a_work_package_query, panels: %i[preview source] %>
|
||||
|
||||
What `FilterForm` does *not* do for you on the legacy side: parsing the
|
||||
form submission back into a `Query#filters` collection. The work-package
|
||||
filter pipeline still uses its own serialization (URL `filters=[...]`
|
||||
JSON / YAML in the DB), so a controller receiving a `FilterForm` submit
|
||||
either needs to use `hidden_input_name:` with a format the existing
|
||||
parser understands, or translate the `operator_<name>` / `<name>_value`
|
||||
fields by hand. The form renders fine either way; what to do with the
|
||||
submitted values is the caller's call.
|
||||
What `FilterFormComponent` does *not* do for you on the legacy side:
|
||||
parsing the form submission back into a `Query#filters` collection. The
|
||||
work-package filter pipeline still uses its own serialization (URL
|
||||
`filters=[...]` JSON / YAML in the DB), so a controller receiving a
|
||||
`FilterFormComponent` submit either needs to use `hidden_input_name:` with
|
||||
a format the existing parser understands, or translate the
|
||||
`operator_<name>` / `<name>_value` fields by hand. The component renders
|
||||
fine either way; what to do with the submitted values is the caller's call.
|
||||
|
||||
+3
-3
@@ -1,4 +1,4 @@
|
||||
<%# `Filters::FilterForm` is a regular primer form object — combine it with %>
|
||||
<%# `Filters::FilterFormComponent` is a component — combine it with %>
|
||||
<%# other forms in a single `Primer::Forms::FormList` to share one builder %>
|
||||
<%# (and therefore one submission) with non-filter inputs. %>
|
||||
<%
|
||||
@@ -13,8 +13,8 @@
|
||||
render(
|
||||
Primer::Forms::FormList.new(
|
||||
note_form.new(f),
|
||||
Filters::FilterForm.new(
|
||||
f,
|
||||
Filters::FilterFormComponent.new(
|
||||
builder: f,
|
||||
query: query,
|
||||
wrap_with_controller: true,
|
||||
hidden_input_name: "filters"
|
||||
|
||||
@@ -3,8 +3,8 @@
|
||||
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
|
||||
<%=
|
||||
render(
|
||||
Filters::FilterForm.new(
|
||||
f,
|
||||
Filters::FilterFormComponent.new(
|
||||
builder: f,
|
||||
query: query,
|
||||
wrap_with_controller: true
|
||||
)
|
||||
|
||||
+8
-8
@@ -1,14 +1,14 @@
|
||||
<%# `Query` is the legacy work-package query model. `Filters::FilterForm` %>
|
||||
<%# works against it the same way it does against `Queries::BaseQuery` %>
|
||||
<%# subclasses — the only requirement is `available_advanced_filters`, %>
|
||||
<%# `filters`, and `find_active_filter(name)`, all of which `Query` %>
|
||||
<%# exposes (the last one was mirrored from `BaseQuery` to bring the %>
|
||||
<%# legacy query in line with the new form). %>
|
||||
<%# `Query` is the legacy work-package query model. %>
|
||||
<%# `Filters::FilterFormComponent` works against it the same way it does %>
|
||||
<%# against `Queries::BaseQuery` subclasses — the only requirement is %>
|
||||
<%# `available_advanced_filters`, `filters`, and `find_active_filter(name)`, %>
|
||||
<%# all of which `Query` exposes (the last one was mirrored from `BaseQuery` %>
|
||||
<%# to bring the legacy query in line with the new component). %>
|
||||
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
|
||||
<%=
|
||||
render(
|
||||
Filters::FilterForm.new(
|
||||
f,
|
||||
Filters::FilterFormComponent.new(
|
||||
builder: f,
|
||||
query: query,
|
||||
wrap_with_controller: true
|
||||
)
|
||||
|
||||
+2
-2
@@ -4,8 +4,8 @@
|
||||
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
|
||||
<%=
|
||||
render(
|
||||
Filters::FilterForm.new(
|
||||
f,
|
||||
Filters::FilterFormComponent.new(
|
||||
builder: f,
|
||||
query: query,
|
||||
wrap_with_controller: true
|
||||
)
|
||||
|
||||
+2
-2
@@ -9,8 +9,8 @@
|
||||
<%= primer_form_with(url: "/foo", method: :post) do |f| %>
|
||||
<%=
|
||||
render(
|
||||
Filters::FilterForm.new(
|
||||
f,
|
||||
Filters::FilterFormComponent.new(
|
||||
builder: f,
|
||||
query: query,
|
||||
wrap_with_controller: true,
|
||||
hidden_input_name: "filters",
|
||||
|
||||
+3
-6
@@ -30,7 +30,7 @@
|
||||
|
||||
require "spec_helper"
|
||||
|
||||
RSpec.describe Filters::FilterForm, type: :forms do
|
||||
RSpec.describe Filters::FilterFormComponent, type: :component do
|
||||
include ViewComponent::TestHelpers
|
||||
|
||||
let(:query) { UserQuery.new }
|
||||
@@ -44,7 +44,7 @@ RSpec.describe Filters::FilterForm, type: :forms do
|
||||
def render_form(form_options = options)
|
||||
render_in_view_context(form_options) do |form_options|
|
||||
primer_form_with(url: "/foo", method: :post) do |f|
|
||||
render(Filters::FilterForm.new(f, **form_options))
|
||||
render(Filters::FilterFormComponent.new(builder: f, **form_options))
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -147,11 +147,8 @@ RSpec.describe Filters::FilterForm, type: :forms do
|
||||
end
|
||||
|
||||
it "raises on unknown values" do
|
||||
# `Primer::Forms::Base.new` accepts a nil builder without complaint,
|
||||
# which lets us exercise the keyword validation without spinning up
|
||||
# a real form context.
|
||||
expect do
|
||||
described_class.new(nil, query:, output_format: :bogus)
|
||||
described_class.new(builder: nil, query:, output_format: :bogus)
|
||||
end.to raise_error(ArgumentError, /Unknown output_format/)
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user