diff --git a/app/components/filters/filter_form_component.html.erb b/app/components/filters/filter_form_component.html.erb index 2c5408a8285..10fc8108366 100644 --- a/app/components/filters/filter_form_component.html.erb +++ b/app/components/filters/filter_form_component.html.erb @@ -1,9 +1,11 @@ -<% 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 %> +<%= + render( + Primer::ConditionalWrapper.new( + condition: @wrap_with_controller, + **@wrapper_arguments + ) + ) do +%> <%= hidden_filters_input if @hidden_input_name %> <%= render(form_list) %> <% end %> diff --git a/app/components/filters/filter_form_component.rb b/app/components/filters/filter_form_component.rb index 7d19a594cbd..f7ad8e58d1f 100644 --- a/app/components/filters/filter_form_component.rb +++ b/app/components/filters/filter_form_component.rb @@ -74,6 +74,9 @@ # in a Primer dialog or another container that clips overflow, so the dropdown # portal renders outside that container instead of being clipped. class Filters::FilterFormComponent < ApplicationComponent + include Primer::AttributesHelper + include Primer::FetchOrFallbackHelper + OUTPUT_FORMATS = %i[params json].freeze def initialize(builder:, @@ -82,15 +85,31 @@ class Filters::FilterFormComponent < ApplicationComponent wrap_with_controller: false, hidden_input_name: nil, output_format: nil, - autocomplete_append_to: nil) + autocomplete_append_to: nil, + **wrapper_arguments) super() @builder = builder @query = query @allowed_filters = allowed_filters || query.available_advanced_filters @wrap_with_controller = wrap_with_controller @hidden_input_name = hidden_input_name - @output_format = validate_output_format(output_format) + @output_format = fetch_or_fallback(OUTPUT_FORMATS, output_format.to_sym) if output_format @autocomplete_append_to = autocomplete_append_to + @wrapper_arguments = wrapper_arguments + @wrapper_arguments[:tag] ||= :div + @wrapper_arguments[:classes] = class_names( + "op-filters-form -expanded", + @wrapper_arguments[:classes] + ) + @wrapper_arguments[:data] = merge_data( + @wrapper_arguments, + { + data: { + controller: "filter--filters-form", + filter__filters_form_output_format_value: @output_format&.to_s + } + } + ) end private @@ -101,28 +120,11 @@ class Filters::FilterFormComponent < ApplicationComponent 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 - attrs - end - - def validate_output_format(format) - return nil if format.nil? - - sym = format.to_sym - unless OUTPUT_FORMATS.include?(sym) - raise ArgumentError, - "Unknown output_format #{format.inspect}; expected one of #{OUTPUT_FORMATS.inspect}" - end - sym - end - def hidden_filters_input hidden_field_tag( @hidden_input_name, "", - data: { "filter--filters-form-target": "filtersInput" } + data: { filter__filters_form_target: "filtersInput" } ) end diff --git a/lookbook/docs/patterns/11-filter-forms.md.erb b/lookbook/docs/patterns/11-filter-forms.md.erb index fdbf97fe6a1..8d730bc2970 100644 --- a/lookbook/docs/patterns/11-filter-forms.md.erb +++ b/lookbook/docs/patterns/11-filter-forms.md.erb @@ -98,9 +98,10 @@ The host server receives the canonical string in ### Combining with non-filter inputs -`FilterFormComponent` composes with other forms via -`Primer::Forms::FormList`. All children share the same builder and -therefore submit through the same `
`. +`FilterFormComponent` is rendered next to normal Primer form objects, not +inside `Primer::Forms::FormList`. Render any regular form objects through +a `FormList`, then render `FilterFormComponent` with the same builder. All +fields still submit through the same surrounding ``. <%= embed OpenProject::Filter::FilterFormPreview, :combined_with_other_inputs, panels: %i[preview source] %> diff --git a/lookbook/previews/open_project/filter/filter_form_preview/combined_with_other_inputs.html.erb b/lookbook/previews/open_project/filter/filter_form_preview/combined_with_other_inputs.html.erb index b7c0dad25d8..de82afa3e87 100644 --- a/lookbook/previews/open_project/filter/filter_form_preview/combined_with_other_inputs.html.erb +++ b/lookbook/previews/open_project/filter/filter_form_preview/combined_with_other_inputs.html.erb @@ -1,6 +1,7 @@ -<%# `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. %> +<%# `Filters::FilterFormComponent` is a ViewComponent that shares the %> +<%# surrounding Primer form builder with other form objects. Render normal %> +<%# Primer form objects through FormList, then render the filter component %> +<%# next to that list with the same builder. %> <% note_form = Class.new(ApplicationForm) do form do |f| @@ -9,17 +10,13 @@ end %> <%= primer_form_with(url: "/foo", method: :post) do |f| %> - <%= - render( - Primer::Forms::FormList.new( - note_form.new(f), + <%= render(Primer::Forms::FormList.new(note_form.new(f))) %> + <%= render( Filters::FilterFormComponent.new( builder: f, query: query, wrap_with_controller: true, hidden_input_name: "filters" ) - ) - ) - %> + ) %> <% end %> diff --git a/spec/components/filters/filter_form_component_spec.rb b/spec/components/filters/filter_form_component_spec.rb index ac453594625..b68afbcbfef 100644 --- a/spec/components/filters/filter_form_component_spec.rb +++ b/spec/components/filters/filter_form_component_spec.rb @@ -140,7 +140,6 @@ RSpec.describe Filters::FilterFormComponent, type: :component do it "omits the data attribute by default" do render_form(query:, wrap_with_controller: true) - # The controller wrapper exists, but without the output-format attribute. expect(page).to have_element "data-controller": "filter--filters-form" do |wrapper| expect(wrapper["data-filter--filters-form-output-format-value"]).to be_nil end @@ -149,7 +148,41 @@ RSpec.describe Filters::FilterFormComponent, type: :component do it "raises on unknown values" do expect do described_class.new(builder: nil, query:, output_format: :bogus) - end.to raise_error(ArgumentError, /Unknown output_format/) + end.to raise_error(Primer::FetchOrFallbackHelper::InvalidValueError, /Expected one of/) + end + end + + describe "wrapper system arguments" do + it "forwards standard system arguments to the controller wrapper" do + render_form( + query:, + wrap_with_controller: true, + id: "custom-filter-wrapper", + aria: { label: "Filters" }, + data: { test_selector: "filters-wrapper" } + ) + + expect(page).to have_element :div, + id: "custom-filter-wrapper", + "aria-label": "Filters", + "data-test-selector": "filters-wrapper" + end + + it "merges caller classes and data with the required wrapper data" do + render_form( + query:, + wrap_with_controller: true, + classes: "custom-class", + data: { + controller: "custom-controller", + action: "keydown->custom#close" + } + ) + + expect(page).to have_element :div, + class: %w[op-filters-form -expanded custom-class], + "data-controller": "filter--filters-form", + "data-action": "keydown->custom#close" end end