Merge pull request #779 from lcreid/763-collection-controls-not-aria-accessible

Make Collection Controls Aria Accessible

Author: lcreid

.yarnrc

@@ -120,6 +120,8 @@ This generates the following HTML:
 </form>
 ```
 
+Note: All examples in this README are generated with the configuration option `group_around_collections` set to `true`. See the [Configuration](#configuration) section.
+
 ### bootstrap_form_tag
 
 If your form is not backed by a model, use the `bootstrap_form_tag`. Usage of this helper is the same as `bootstrap_form_for`, except no model object is passed in as the first argument. Here's an example:
@@ -233,6 +235,7 @@ The current configuration options are:
 | Option | Default value | Description |
 |---------------------------|------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `default_form_attributes` | {} | `bootstrap_form` versions 3 and 4 added a role="form" attribute to all forms. The W3C validator will raise a **warning** on forms with a role="form" attribute. `bootstrap_form` version 5 drops this attribute by default. Set this option to `{ role: "form" }` to make forms non-compliant with W3C, but generate the `role="form"` attribute like `bootstrap_form` versions 3 and 4. |
+| `group_around_collections` | false | Historically, `bootstrap_form` generated a wrapper around `collection_checkboxes` and `collection_radio_buttons` using the same `form_group` as individual controls used. This markup caused accessibility problems. Setting `group_around_collections = true` will generate collections of checkboxes and radio buttons wrapper in a `<fieldset>` with the text as a `<legend>` (https://www.w3.org/WAI/tutorials/forms/grouping/). This _will_ make visible changes to pages that use the collection methods.<br/><br/>The default for this option will be changed to `true` in a future version. |
 
 Example:
 
@@ -781,8 +784,8 @@ This generates:
 This generates:
 
 ```html
-<div class="mb-3">
-  <label class="form-label" for="user_skill_level">Skill level</label>
+<div aria-labelledby="user_skill_level" class="mb-3" role="group">
+  <div class="form-label" id="user_skill_level">Skill level</div>
   <div class="form-check">
     <input class="form-check-input" id="user_skill_level_1" name="user[skill_level]" type="radio" value="1">
     <label class="form-check-label" for="user_skill_level_1">Mind reading</label>
@@ -793,8 +796,8 @@ This generates:
   </div>
 </div>
 <input id="user_skills" name="user[skills][]" type="hidden" value="">
-<div class="mb-3">
-  <label class="form-label" for="user_skills">Skills</label>
+<div aria-labelledby="user_skills" class="mb-3" role="group">
+  <div class="form-label" id="user_skills">Skills</div>
   <div class="form-check">
     <input class="form-check-input" id="user_skills_1" name="user[skills][]" type="checkbox" value="1">
     <label class="form-check-label" for="user_skills_1">Mind reading</label>
@@ -829,8 +832,8 @@ To add `data-` attributes to a collection of radio buttons, map your models to a
 This generates:
 
 ```html
-<div class="mb-3">
-  <label class="form-label" for="user_misc">Misc</label>
+<div aria-labelledby="user_misc" class="mb-3" role="group">
+  <div class="form-label" id="user_misc">Misc</div>
   <div class="form-check">
     <input class="form-check-input" id="user_misc_1" name="user[misc]" type="radio" value="1">
     <label class="form-check-label" for="user_misc_1">Foo</label>
@@ -1417,7 +1420,7 @@ This generates:
 </form>
 ```
 
-A form-level `layout: :inline` can't be overridden because of the way Bootstrap 4 implements in-line layouts. One possible work-around is to leave the form-level layout as default, and specify the individual fields as `layout: :inline`, except for the fields(s) that should be other than in-line.
+A form-level `layout: :inline` can't be overridden because of the way Bootstrap implements in-line layouts. One possible work-around is to leave the form-level layout as default, and specify the individual fields as `layout: :inline`, except for the fields(s) that should be other than in-line.
 
 ### Floating Labels
 
@@ -1493,8 +1496,8 @@ Generated HTML:
     <input class="form-control is-invalid" id="user_email" name="user[email]" required="required" type="email" value="steve.example.com">
     <div class="invalid-feedback">is invalid</div>
   </div>
-  <div class="mb-3">
-    <label class="form-label" for="user_misc">Misc</label>
+  <div aria-labelledby="user_misc" class="mb-3" role="group">
+    <div class="form-label" id="user_misc">Misc</div>
     <div class="form-check">
       <input checked class="form-check-input is-invalid" id="user_misc_1" name="user[misc]" type="radio" value="1">
       <label class="form-check-label" for="user_misc_1">Mind reading</label>
@@ -1506,8 +1509,8 @@ Generated HTML:
     </div>
   </div>
   <input id="user_preferences" name="user[preferences][]" type="hidden" value="">
-  <div class="mb-3">
-    <label class="form-label" for="user_preferences">Preferences</label>
+  <div aria-labelledby="user_preferences" class="mb-3" role="group">
+    <div class="form-label" id="user_preferences">Preferences</div>
     <div class="form-check">
       <input checked class="form-check-input is-invalid" id="user_preferences_1" name="user[preferences][]" type="checkbox" value="1">
       <label class="form-check-label" for="user_preferences_1">Good</label>

README.md

@@ -0,0 +1,3 @@
+BootstrapForm.configure do |config|
+  config.group_around_collections = true
+end

demo/config/initializers/bootstrap_form.rb

@@ -4,7 +4,14 @@
 require "capybara_screenshot_diff/minitest"
 
 class BootstrapTest < ApplicationSystemTestCase
-  setup { screenshot_section :bootstrap }
+  setup do
+    screenshot_section :bootstrap
+    Rails.application.config.bootstrap_form.group_around_collections = true
+  end
+
+  teardown do
+    Rails.application.config.bootstrap_form.group_around_collections = false
+  end
 
   test "visiting the index" do
     screenshot_group :index

demo/test/system/bootstrap_test.rb

@@ -10,16 +10,8 @@ module Labels
       def generate_label(id, name, options, custom_label_col, group_layout)
         return if options.blank?
 
-        # id is the caller's options[:id] at the only place this method is called.
-        # The options argument is a small subset of the options that might have
-        # been passed to generate_label's caller, and definitely doesn't include
-        # :id.
-        options[:for] = id if acts_like_form_tag
-
-        options[:class] = label_classes(name, options, custom_label_col, group_layout)
-        options.delete(:class) if options[:class].none?
-
-        label(name, label_text(name, options), options.except(:text))
+        prepare_label_options(id, name, options, custom_label_col, group_layout)
+        label(name, label_text(name, options[:text]), options.except(:text))
       end
 
       def label_classes(name, options, custom_label_col, group_layout)
@@ -42,14 +34,25 @@ def label_layout_classes(custom_label_col, group_layout)
         end
       end
 
-      def label_text(name, options)
-        label = options[:text] || object&.class&.try(:human_attribute_name, name)&.html_safe # rubocop:disable Rails/OutputSafety, Style/SafeNavigationChainLength
+      def label_text(name, text)
+        label = text || object&.class&.try(:human_attribute_name, name)&.html_safe # rubocop:disable Rails/OutputSafety, Style/SafeNavigationChainLength
         if label_errors && error?(name)
           (" ".html_safe + get_error_messages(name)).prepend(label)
         else
           label
         end
       end
+
+      def prepare_label_options(id, name, options, custom_label_col, group_layout)
+        # id is the caller's options[:id] at the only place this method is called.
+        # The options argument is a small subset of the options that might have
+        # been passed to generate_label's caller, and definitely doesn't include
+        # :id.
+        options[:for] = id if acts_like_form_tag
+
+        options[:class] = label_classes(name, options, custom_label_col, group_layout)
+        options.delete(:class) if options[:class].none?
+      end
     end
   end
 end

lib/bootstrap_form/components/labels.rb

@@ -9,6 +9,7 @@ class Engine < Rails::Engine
 
     config.bootstrap_form = BootstrapForm.config
     config.bootstrap_form.default_form_attributes ||= {}
+    config.bootstrap_form.group_around_collections = Rails.env.development? if config.bootstrap_form.group_around_collections.nil?
 
     initializer "bootstrap_form.configure" do |app|
       BootstrapForm.config = app.config.bootstrap_form

lib/bootstrap_form/engine.rb

@@ -7,6 +7,16 @@ module FormGroupBuilder
     private
 
     def form_group_builder(method, options, html_options=nil, &)
+      form_group_builder_wrapper(method, options, html_options) do |form_group_options, no_wrapper|
+        if no_wrapper
+          yield
+        else
+          form_group(method, form_group_options, &)
+        end
+      end
+    end
+
+    def form_group_builder_wrapper(method, options, html_options=nil)
       no_wrapper = options[:wrapper] == false
 
       options = form_group_builder_options(options, method)
@@ -18,11 +28,7 @@ def form_group_builder(method, options, html_options=nil, &)
         :hide_label, :skip_required, :label_as_placeholder, :wrapper_class, :wrapper
       )
 
-      if no_wrapper
-        yield
-      else
-        form_group(method, form_group_options, &)
-      end
+      yield(form_group_options, no_wrapper)
     end
 
     def form_group_builder_options(options, method)

lib/bootstrap_form/form_group_builder.rb

@@ -7,24 +7,20 @@ module InputsCollection
 
       private
 
-      def inputs_collection(name, collection, value, text, options={})
+      def inputs_collection(name, collection, value, text, options={}, &)
         options[:label] ||= { class: group_label_class(field_layout(options)) }
         options[:inline] ||= layout_inline?(options[:layout])
 
-        form_group_builder(name, options) do
-          inputs = ActiveSupport::SafeBuffer.new
-
-          collection.each_with_index do |obj, i|
-            input_value = value.respond_to?(:call) ? value.call(obj) : obj.send(value)
-            input_options = form_group_collection_input_options(options, text, obj, i, input_value, collection)
-            inputs << yield(name, input_value, input_options)
-          end
+        return group_inputs_collection(name, collection, value, text, options, &) if BootstrapForm.config.group_around_collections
 
-          inputs
+        form_group_builder(name, options) do
+          render_collection(name, collection, value, text, options, &)
         end
       end
 
-      def field_layout(options) = options[:layout] || (:inline if options[:inline] == true)
+      def field_layout(options)
+        (:inline if options[:inline] == true) || options[:layout]
+      end
 
       def group_label_class(field_layout)
         if layout_horizontal?(field_layout)
@@ -56,6 +52,63 @@ def form_group_collection_input_checked?(checked, obj, input_value)
         checked == input_value || Array(checked).try(:include?, input_value) ||
           checked == obj || Array(checked).try(:include?, obj)
       end
+
+      def group_inputs_collection(name, collection, value, text, options={}, &)
+        group_builder(name, options) do
+          render_collection(name, collection, value, text, options, &)
+        end
+      end
+
+      def render_collection(name, collection, value, text, options={}, &)
+        inputs = ActiveSupport::SafeBuffer.new
+
+        collection.each_with_index do |obj, i|
+          input_value = value.respond_to?(:call) ? value.call(obj) : obj.send(value)
+          input_options = form_group_collection_input_options(options, text, obj, i, input_value, collection)
+          inputs << yield(name, input_value, input_options)
+        end
+
+        inputs
+      end
+
+      def group_builder(method, options, html_options=nil, &)
+        form_group_builder_wrapper(method, options, html_options) do |form_group_options, no_wrapper|
+          if no_wrapper
+            yield
+          else
+            field_group(method, form_group_options, &)
+          end
+        end
+      end
+
+      def field_group(name, options, &)
+        options[:class] = form_group_classes(options)
+
+        tag.div(
+          **options.except(
+            :add_control_col_class, :append, :control_col, :floating, :help, :icon, :id,
+            :input_group_class, :label, :label_col, :layout, :prepend
+          ),
+          aria: { labelledby: options[:id] || default_id(name) },
+          role: :group
+        ) do
+          group_label_div = generate_group_label_div(name, options)
+          prepare_label_options(options[:id], name, options[:label], options[:label_col], options[:layout])
+          form_group_content(group_label_div, generate_help(name, options[:help]), options, &)
+        end
+      end
+
+      def generate_group_label_div(name, options)
+        group_label_div_class = options.dig(:label, :class) || "form-label"
+        id = options[:id] || default_id(name)
+
+        tag.div(
+          **{ class: group_label_div_class }.compact,
+          id:
+        ) { label_text(name, options.dig(:label, :text)) }
+      end
+
+      def default_id(name) = raw("#{object_name}_#{name}") # rubocop:disable Rails/OutputSafety
     end
   end
 end