Fix w3 validator warning for label[for] not corresponding to any inpu…

…t[id]
bogdan · Nov 13, 2024 · 61b0b50 · 61b0b50
1 parent a171ba7
commit 61b0b50
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 26 deletions.
diff --git a/app/views/datagrid/_range_filter.html.erb b/app/views/datagrid/_range_filter.html.erb
@@ -1,3 +1,5 @@
 <%= form.datagrid_filter_input(filter, class: 'datagrid-range-from', **from_options) %>
 <span class="datagrid-range-separator"><%= I18n.t('datagrid.filters.range.separator') %></span>
-<%= form.datagrid_filter_input(filter, class: 'datagrid-range-to', **to_options) %>
+<%# Generating id only for "from" input to make sure 
+  # there is no duplicate id in DOM and click on label focuses the first input -%>
+<%= form.datagrid_filter_input(filter, class: 'datagrid-range-to', **to_options, id: nil) %>
diff --git a/lib/datagrid/form_builder.rb b/lib/datagrid/form_builder.rb
@@ -190,17 +190,6 @@ def datagrid_range_filter_options(object, filter, type, **options)
       type_method_map = { from: :begin, to: :end }
       options[:value] = object[filter.name]&.public_send(type_method_map[type])
       options[:name] = @template.field_name(object_name, filter.name, type)
-      # In case of datagrid ranged filter
-      # from and to input will have same id
-      if !options.key?(:id)
-        # Rails provides it's own default id for all inputs
-        # In order to prevent that we assign no id by default
-        options[:id] = nil
-      elsif options[:id].present?
-        # If the id was given we prefix it
-        # with from_ and to_ accordingly
-        options[:id] = [type, options[:id]].join("_")
-      end
       options
     end
 

diff --git a/spec/datagrid/form_builder_spec.rb b/spec/datagrid/form_builder_spec.rb
@@ -189,10 +189,10 @@ class MyTemplate
         let(:_range) { [1, 2] }
         it {
           should equal_to_dom(
-            '<input value="1" id="from_hello" class="datagrid-range-from"
+            '<input value="1" id="hello" class="datagrid-range-from"
                 type="number" step="1" name="report[group_id][from]"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
-            '<input value="2" id="to_hello" class="datagrid-range-to"
+            '<input value="2" class="datagrid-range-to"
                 type="number" step="1" name="report[group_id][to]"/>',
           )
         }
@@ -201,7 +201,7 @@ class MyTemplate
         let(:_range) { [10, nil] }
         it {
           should equal_to_dom(
-            '<input value="10" class="datagrid-range-from"
+            '<input value="10" class="datagrid-range-from" id="report_group_id"
                 type="number" step="1" name="report[group_id][from]"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input class="datagrid-range-to"
@@ -214,7 +214,7 @@ class MyTemplate
         let(:_range) { [nil, 10] }
         it {
           should equal_to_dom(
-            '<input class="datagrid-range-from" type="number" step="1" name="report[group_id][from]"/>' \
+            '<input class="datagrid-range-from" type="number" step="1" name="report[group_id][from]" id="report_group_id"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input value="10" class="datagrid-range-to" type="number" step="1" name="report[group_id][to]"/>',
           )
@@ -226,7 +226,7 @@ class MyTemplate
         let(:_range) { 2..1 }
         it {
           should equal_to_dom(
-            '<input value="1" class="datagrid-range-from" type="number" step="1" name="report[group_id][from]"/>' \
+            '<input value="1" class="datagrid-range-from" type="number" step="1" name="report[group_id][from]" id="report_group_id"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input value="2" class="datagrid-range-to" type="number" step="1" name="report[group_id][to]"/>',
           )
@@ -248,7 +248,7 @@ class MyTemplate
         let(:_range) { nil }
         it {
           should equal_to_dom(
-            '<input class="datagrid-range-from" type="number" step="1" name="report[group_id][from]">
+            '<input class="datagrid-range-from" type="number" step="1" name="report[group_id][from]" id="report_group_id">
             <span class="datagrid-range-separator"> - </span>
             <input class="datagrid-range-to" type="number" step="1" name="report[group_id][to]">',
           )
@@ -267,7 +267,7 @@ class MyTemplate
       let(:_range) { [1.5, 2.5] }
       it {
         should equal_to_dom(
-          '<input value="1.5" class="datagrid-range-from"
+          '<input value="1.5" class="datagrid-range-from" id="report_rating"
               type="number" step="any" name="report[rating][from]"/>' \
           '<span class="datagrid-range-separator"> - </span>' \
           '<input value="2.5" class="datagrid-range-to"
@@ -288,7 +288,7 @@ class MyTemplate
         let(:_range) { ["2012-01-03", nil] }
         it {
           should equal_to_dom(
-            '<input value="2012-01-03" class="datagrid-range-from" type="date" name="report[created_at][from]"/>' \
+            '<input value="2012-01-03" class="datagrid-range-from" type="date" name="report[created_at][from]" id="report_created_at"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input class="datagrid-range-to" type="date" name="report[created_at][to]" value=""/>',
           )
@@ -304,7 +304,7 @@ class MyTemplate
         let(:_range) { ["2013/01/01", "2013/02/02"] }
         it {
           should equal_to_dom(
-            '<input value="2013-01-01" class="datagrid-range-from"
+            '<input value="2013-01-01" class="datagrid-range-from" id="report_created_at"
                 type="date" name="report[created_at][from]"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input value="2013-02-02" class="datagrid-range-to"
@@ -316,11 +316,11 @@ class MyTemplate
         let(:_range) { [nil, "2012-01-03"] }
         it {
           should equal_to_dom(
-            '<input class="datagrid-range-from"
+            '<input class="datagrid-range-from" id="report_created_at"
                 type="date" value="" name="report[created_at][from]"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input value="2012-01-03" class="datagrid-range-to"
-                type="date"  name="report[created_at][to]"/>',
+                type="date" name="report[created_at][to]"/>',
           )
         }
         it { should be_html_safe }
@@ -330,7 +330,7 @@ class MyTemplate
         let(:_range) { Date.parse("2012-01-02")..Date.parse("2012-01-01") }
         it {
           should equal_to_dom(
-            '<input value="2012-01-01" class="datagrid-range-from"
+            '<input value="2012-01-01" class="datagrid-range-from" id="report_created_at"
                 type="date" name="report[created_at][from]"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input value="2012-01-02" class="datagrid-range-to"
@@ -347,7 +347,7 @@ class MyTemplate
         let(:_range) { [nil, nil] }
         it {
           should equal_to_dom(
-            '<input class="datagrid-range-from" type="date" value="" name="report[created_at][from]"/>' \
+            '<input class="datagrid-range-from" type="date" value="" name="report[created_at][from]" id="report_created_at"/>' \
             '<span class="datagrid-range-separator"> - </span>' \
             '<input class="datagrid-range-to" type="date" value="" name="report[created_at][to]"/>',
           )

diff --git a/version-2/Readme.markdown b/version-2/Readme.markdown
@@ -18,6 +18,7 @@ List of things introduces:
 1. Native Rails Engines.
    * while supported, the library was not initially designed for it.
 1. HTML5 data attributes
+1. Consistent id attribute for range filter inputs
 1. Inherit `Datagrid::Base` instead of `include Datagrid`
 1. `ApplicationGrid` is recommended base class instead of `BaseGrid`
 1. Introduced `datagrid.filters.range.separator` localization
@@ -373,6 +374,31 @@ Renders:
 [Modify built-in partials](https://github.com/bogdan/datagrid/wiki/Frontend#modifying-built-in-partials)
 if you want to change this behavior completely.
 
+## id attribute for range filter inputs
+
+[W3 validator](https://validator.w3.org/) complains when 
+a `label[for]` attribute doesn't correspond to any `input[id]` in the same form.
+Version 1 generated no id attribute for range filter inputs by default which resulted in a warning:
+
+``` html
+<label for="musics_grid_year">Year</label>
+<input class="year integer_fiilter from" multiple name="musics_grid[year][]" type="text">
+<span class="separator integer"> - </span>
+<input class="year integer_filter to" multiple name="musics_grid[year][]" type="text">
+```
+
+Version 2 generates id attribute only for the first input, so that a click on label focuses the first input:
+
+``` html
+<label for="musics_grid_year">Year</label>
+<input id="musics_grid_year" step="1" class="datagrid-range-from" name="musics_grid[year][from]" type="number">
+<span class="datagrid-range-separator"> - </span>
+<input step="1" class="datagrid-range-to" name="musics_grid[year][to]" type="number">
+```
+
+The behavior can be changed by modifying 
+[built-in view](https://github.com/bogdan/datagrid/blob/version-2/app/views/datagrid/_range_filter.html.erb#L3).
+
 ## ApplicationGrid base class
 
 Previously recommended base class `BaseGrid` is incosistent
@@ -404,7 +430,7 @@ end
 A separator symbol between range filter inputs is now a part of localizations to avoid hardcore.
 Add `datagrid.filters.range.separator` to your localization file.
 
-[See commit for details](https://github.com/bogdan/datagrid/commit/2bd914a39a5f8367758ad697d7ccf8d98379fff7#diff-0e78e11f3e693a6523052bc71095ec539fa390cf04b50d74c35c9af5260f50f3L2)
+[See related view](https://github.com/bogdan/datagrid/blob/version-2/app/views/datagrid/_range_filter.html.erb#L2)
 
 
 ## Remove SASS dependency