[OP-19415] Primer patterns in FilterFormComponent

Uses `ConditionalWrapper` to eliminate template duplication in the
`wrap_with_controller` branch, and `merge_data` from
`Primer::AttributesHelper` for Stimulus data attributes.

https://community.openproject.org/wp/OP-19415

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 %>

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
 

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 `<form>`.
+`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 `<form>`.
 
 <%= embed OpenProject::Filter::FilterFormPreview, :combined_with_other_inputs, panels: %i[preview source] %>
 

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 %>

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