Make multi input a form control

.changeset/fair-ties-boil.md

@@ -0,0 +1,5 @@
+---
+'@primer/view-components': patch
+---
+
+Make multi input a form control

app/components/primer/primer.ts

@@ -11,4 +11,5 @@ import './beta/clipboard_copy'
 import './local_time'
 import './tab_container_component'
 import './time_ago_component'
+import '../../../lib/primer/forms/primer_multi_input'
 import '../../../lib/primer/forms/primer_text_field'

app/forms/multi_input_form.rb

@@ -9,17 +9,19 @@ class MultiInputForm < ApplicationForm
     end
 
     my_form.multi(name: :region, label: "Region") do |region|
-      region.select_list(label: "State") do |state_list|
+      region.select_list(name: :states) do |state_list|
         state_list.option(label: "California", value: "CA")
         state_list.option(label: "Washington", value: "WA")
         state_list.option(label: "Oregon", value: "OR")
       end
 
-      region.select_list(label: "Province", hidden: true) do |province_list|
+      region.select_list(hidden: true, name: :provinces) do |province_list|
         province_list.option(label: "British Columbia", value: "BC")
         province_list.option(label: "Alberta", value: "AB")
         province_list.option(label: "Saskatchewan", value: "SK")
       end
     end
+
+    my_form.submit(name: :submit, label: "Submit", scheme: :primary)
   end
 end

demo/app/controllers/multi_controller.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+# works with app/forms/multi_input_form.rb
+class MultiController < ApplicationController
+  def create
+    respond_to do |format|
+      format.json { render json: params.permit(:country, :region).to_h }
+    end
+  end
+end

demo/config/routes.rb

@@ -13,6 +13,7 @@
     get "/auto_complete", to: "auto_complete_test#index"
     resources :toggle_switch, only: [:create]
     resources :nav_list_items, only: [:index]
+    resources :multi, only: [:create]
     post "/example_check/ok", to: "auto_check#ok", as: :example_check_ok
     post "/example_check/error", to: "auto_check#error", as: :example_check_error
     post "/example_check/random", to: "auto_check#random", as: :example_check_random

demo/kuby.rb

@@ -111,6 +111,7 @@ def install_from_image(image, dockerfile)
           postcss.config.js
           lib/
           app/
+          lib/
           package.json
           package-lock.json
           previews

lib/primer/forms/builder.rb

@@ -2,6 +2,29 @@
 
 require "primer/classify"
 
+# See: https://github.com/rails/rails/pull/46666
+ActionView::Helpers::Tags::Base.prepend(
+  Module.new do
+    def initialize(*args, **kwargs, &block)
+      super
+
+      return if defined?(@generate_error_markup)
+
+      @generate_error_markup = @options.delete(:generate_error_markup) { true }
+    end
+
+    private
+
+    def error_wrapping(html_tag)
+      return html_tag unless @generate_error_markup
+
+      # :nocov:
+      super
+      # :nocov:
+    end
+  end
+)
+
 module Primer
   module Forms
     # :nodoc:
@@ -10,32 +33,49 @@ class Builder < ActionView::Helpers::FormBuilder
 
       UTILITY_KEYS = Primer::Classify::Utilities::UTILITIES.keys.freeze
 
-      def label(*args, **options, &block)
-        super(*args, classify(options), &block)
+      def label(method, text = nil, **options, &block)
+        super(method, text, classify(options).merge(generate_error_markup: false), &block)
       end
 
       def check_box(method, options = {}, checked_value = 1, unchecked_value = 0, &block)
-        super(method, classify(options), checked_value, unchecked_value, &block)
+        super(
+          method,
+          classify(options).merge(generate_error_markup: false),
+          checked_value,
+          unchecked_value,
+          &block
+        )
       end
 
       def radio_button(*args, **options, &block)
-        super(*args, classify(options), &block)
+        super(*args, classify(options).merge(generate_error_markup: false), &block)
       end
 
-      def select(*args, **options, &block)
-        super(*args, classify(options), &block)
+      def select(method, choices = nil, options = {}, html_options = {}, &block)
+        super(method, choices, options.merge(generate_error_markup: false), classify(html_options), &block)
       end
 
       def text_field(*args, **options, &block)
-        super(*args, classify(options), &block)
+        super(*args, classify(options).merge(generate_error_markup: false), &block)
       end
 
       def text_area(*args, **options, &block)
-        super(*args, classify(options), &block)
+        super(*args, classify(options).merge(generate_error_markup: false), &block)
       end
 
       private
 
+      # This method does the following:
+      #
+      # 1. Runs Primer's classify routine to convert entries like mb: 1 to mb-1.
+      # 2. Runs classify on both options[:class] and options[:classes]. The first
+      #    is expected by Rails/HTML while the second is specific to Primer.
+      # 3. Combines options[:class] and options[:classes] into options[:class]
+      #    so the options hash can be easily passed to Rails form builder methods.
+      # 4. Sets generate_error_markup: false, which, in combination with the
+      #    monkeypatch at the top of this file, skips rendering any markup around
+      #    invalid fields.
+      #
       def classify(options)
         options[:classes] = class_names(options.delete(:class), options[:classes])
         options.merge!(Primer::Classify.call(options))

lib/primer/forms/check_box.html.erb

@@ -4,7 +4,9 @@
     <%= builder.label(@input.name, **@input.label_arguments) do %>
       <%= @input.label %>
     <% end %>
-    <%= render(Caption.new(input: @input)) %>
+    <% if @input.form_control? %>
+      <%= render(Caption.new(input: @input)) %>
+    <% end %>
   </span>
 <% end %>
 <% if @input.nested_form_block %>

lib/primer/forms/dsl/input.rb

@@ -16,7 +16,9 @@ class Input
 
         include Primer::ClassNameHelper
 
-        attr_reader :builder, :form, :input_arguments, :label_arguments, :caption, :validation_message, :ids
+        attr_reader :builder, :form, :input_arguments, :label_arguments, :caption, :validation_message, :ids, :form_control
+
+        alias form_control? form_control
 
         def initialize(builder:, form:, **system_arguments)
           @builder = builder
@@ -41,6 +43,10 @@ def initialize(builder:, form:, **system_arguments)
           @full_width = @input_arguments.delete(:full_width)
           @size = @input_arguments.delete(:size)
 
+          # Whether or not to wrap the component in a FormControl, which renders a
+          # label above and validation message beneath the input.
+          @form_control = @input_arguments.delete(:form_control) { true }
+
           @input_arguments[:invalid] = "true" if invalid?
 
           base_id = SecureRandom.hex[0..5]

lib/primer/forms/dsl/multi_input.rb

@@ -34,19 +34,16 @@ def add_input(input)
           check_one_input_visible!
         end
 
-        def decorate_options(name: nil, **options)
-          check_name!(name) if name
-          new_options = { name: name || @name, label: nil, **options }
+        def decorate_options(name:, **options)
+          new_options = { name: @name, label: nil, form_control: false, **options }
+          new_options[:data] ||= {}
+          new_options[:data][:name] = name
+          new_options[:data][:targets] = "primer-multi-input.fields"
           new_options[:id] = nil if options[:hidden]
+          new_options[:disabled] = true if options[:hidden] # disable to avoid submitting to server
           new_options
         end
 
-        def check_name!(name)
-          return if name == @name
-
-          raise ArgumentError, "Inputs inside a `multi' block must all have the same name. Expected '#{@name}', got '#{name}'."
-        end
-
         def check_one_input_visible!
           return if inputs.count { |input| !input.hidden? } <= 1
 

lib/primer/forms/dsl/select_list_input.rb

@@ -5,7 +5,7 @@ module Forms
     module Dsl
       # :nodoc:
       class SelectListInput < Input
-        SELECT_ARGUMENTS = %i[multiple disabled include_blank prompt].freeze
+        SELECT_ARGUMENTS = %i[multiple include_blank prompt].freeze
 
         # :nodoc:
         class Option

lib/primer/forms/form_control.html.erb

@@ -1,18 +1,22 @@
-<%= content_tag(:div, **@form_group_arguments) do %>
-  <% if @input.label %>
-    <%= builder.label(@input.name, **@input.label_arguments) do %>
-      <%= @input.label %>
-      <% if @input.required? %>
-        <span aria-hidden="true">*</span>
+<% if @input.form_control? %>
+  <%= content_tag(:div, **@form_group_arguments) do %>
+    <% if @input.label %>
+      <%= builder.label(@input.name, **@input.label_arguments) do %>
+        <%= @input.label %>
+        <% if @input.required? %>
+          <span aria-hidden="true">*</span>
+        <% end %>
       <% end %>
     <% end %>
-  <% end %>
-  <%= content %>
-  <% if @input.need_validation_element? %>
-    <%= content_tag(:div, **@input.validation_arguments) do %>
-      <%= render(Primer::Beta::Octicon.new(icon: :"alert-fill", size: :xsmall, aria: { hidden: true })) %>
-      <%= content_tag(:span, @input.validation_messages.first, **@input.validation_message_arguments) %>
+    <%= content %>
+    <% if @input.need_validation_element? %>
+      <%= content_tag(:div, **@input.validation_arguments) do %>
+        <%= render(Primer::Beta::Octicon.new(icon: :"alert-fill", size: :xsmall, aria: { hidden: true })) %>
+        <%= content_tag(:span, @input.validation_messages.first, **@input.validation_message_arguments) %>
+      <% end %>
     <% end %>
+    <%= render(Caption.new(input: @input)) %>
   <% end %>
-  <%= render(Caption.new(input: @input)) %>
+<% else %>
+  <%= content %>
 <% end %>

lib/primer/forms/multi.html.erb

@@ -1,3 +1,7 @@
-<% @input.inputs.each do |child_input| %>
-  <%= render(child_input.to_component) %>
+<%= render(FormControl.new(input: @input)) do %>
+  <primer-multi-input data-name="<%= @input.name %>">
+    <% @input.inputs.each do |child_input| %>
+      <%= render(child_input.to_component) %>
+    <% end %>
+  </primer-multi-input>
 <% end %>

lib/primer/forms/primer_multi_input.ts

@@ -0,0 +1,46 @@
+/* eslint-disable custom-elements/expose-class-on-global */
+import {controller, targets} from '@github/catalyst'
+
+@controller
+export class PrimerMultiInputElement extends HTMLElement {
+  @targets fields: HTMLInputElement[]
+
+  activateField(name: string) {
+    const fieldWithName = this.findField(name)
+    if (!fieldWithName) return
+
+    for (const field of this.fields) {
+      if (field === fieldWithName) continue
+
+      field.setAttribute('disabled', 'disabled')
+      field.setAttribute('hidden', 'hidden')
+
+      field.parentElement?.setAttribute('hidden', 'hidden')
+    }
+
+    fieldWithName.removeAttribute('disabled')
+    fieldWithName.removeAttribute('hidden')
+    fieldWithName.parentElement?.removeAttribute('hidden')
+  }
+
+  private findField(name: string): HTMLElement | null {
+    for (const field of this.fields) {
+      if (field.getAttribute('data-name') === name) {
+        return field
+      }
+    }
+
+    return null
+  }
+}
+
+declare global {
+  interface Window {
+    PrimerMultiInputElement: typeof PrimerMultiInputElement
+  }
+}
+
+if (!window.customElements.get('primer-multi-input')) {
+  Object.assign(window, {PrimerMultiInputElement})
+  window.customElements.define('primer-multi-input', PrimerMultiInputElement)
+}

lib/primer/forms/radio_button.html.erb

@@ -4,7 +4,9 @@
     <%= builder.label(@input.name, value: @input.value, **@input.label_arguments) do %>
       <%= @input.label %>
     <% end %>
-    <%= render(Caption.new(input: @input)) %>
+    <% if @input.form_control? %>
+      <%= render(Caption.new(input: @input)) %>
+    <% end %>
   </span>
 <% end %>
 <% if @input.nested_form_block %>

lib/primer/forms/text_field.html.erb

@@ -1,6 +1,6 @@
 <%= render Primer::ConditionalWrapper.new(condition: @input.auto_check_src, tag: "primer-text-field") do %>
   <%= render(FormControl.new(input: @input)) do %>
-    <%= render Primer::ConditionalWrapper.new(tag: :div, class: @input.field_wrap_classes, condition: @input.leading_visual || @input.show_clear_button?) do %>
+    <%= content_tag(:div, **@field_wrap_arguments) do %>
       <% if @input.leading_visual %>
         <span class="FormControl-input-leadingVisualWrap">
           <%= render(Primer::Beta::Octicon.new(**@input.leading_visual)) %>
@@ -10,7 +10,7 @@
         <%= builder.text_field(@input.name, **@input.input_arguments) %>
       <% end %>
       <% if @input.show_clear_button? %>
-        <button id="<%= @input.clear_button_id %>" class="FormControl-input-trailingAction" aria-label="Clear">
+        <button type="button" id="<%= @input.clear_button_id %>" class="FormControl-input-trailingAction" aria-label="Clear">
           <%= render(Primer::Beta::Octicon.new(icon: :"x-circle-fill")) %>
         </button>
       <% end %>

previews/primer/forms/forms_preview/multi_input_form.html.erb

@@ -1,3 +1,14 @@
-<%= primer_form_with(url: "/foo") do |f| %>
+<script type="text/javascript">
+  window.addEventListener('load', function () {
+    document.querySelector('input#country_us').onclick = function() {
+      document.querySelector('primer-multi-input').activateField('states');
+    };
+
+    document.querySelector('input#country_ca').onclick = function() {
+      document.querySelector('primer-multi-input').activateField('provinces');
+    };
+  }, false);
+</script>
+<%= primer_form_with(url: multi_index_path(format: :json)) do |f| %>
   <%= render(MultiInputForm.new(f)) %>
 <% end %>

test/lib/primer/forms/checkbox_group_input_test.rb

@@ -20,7 +20,7 @@ def test_hidden_checkbox_group
       end
     end
 
-    assert_selector "fieldset", visible: false
-    assert_selector ".FormControl-checkbox-wrap", visible: false
+    assert_selector "fieldset", visible: :hidden
+    assert_selector ".FormControl-checkbox-wrap", visible: :hidden
   end
 end

test/lib/primer/forms/checkbox_input_test.rb

@@ -49,8 +49,8 @@ def test_hidden_checkbox
       end
     end
 
-    assert_selector "input[name=foo]", visible: false
-    assert_selector "input[name=bar]", visible: false
+    assert_selector "input[name=foo]", visible: :hidden
+    assert_selector "input[name=bar]", visible: :hidden
   end
 
   class CheckboxWithHiddenNestedForm < ApplicationForm
@@ -71,6 +71,6 @@ def test_nested_form_can_be_hidden_independently
     end
 
     assert_selector "input[name=foo]"
-    assert_selector "input[name=bar]", visible: false
+    assert_selector "input[name=bar]", visible: :hidden
   end
 end

test/lib/primer/forms/forms_test.rb

@@ -167,12 +167,14 @@ def test_renders_horizontal_group
   def test_renders_multi_input
     render_preview :multi_input_form
 
+    assert_selector "label", text: "Region"
+
     assert_selector ".FormControl select[name=region]" do
-      assert_selector "label", text: "State"
+      assert_selector "select option[value=WA]"
     end
 
     assert_selector ".FormControl select[name=region]", visible: false do
-      assert_selector "label", text: "Province", visible: false
+      assert_selector "select option[value=BC]", visible: false
     end
   end