From 7ca255198b060e84de4a2af944ef8389a7158f62 Mon Sep 17 00:00:00 2001 From: Jose Farias <31393016+josefarias@users.noreply.github.com> Date: Wed, 7 Feb 2024 09:47:03 -0600 Subject: [PATCH] Use provided id for combobox, not hidden field (#20) --- .github/release.yml | 3 + app/presenters/hotwire_combobox/component.rb | 21 ++- .../app/views/comboboxes/new_options.html.erb | 4 +- test/helpers/hotwire_combobox/helper_test.rb | 3 +- test/system/hotwire_combobox_test.rb | 148 +++++++++--------- 5 files changed, 94 insertions(+), 85 deletions(-) diff --git a/.github/release.yml b/.github/release.yml index bf8d362..ed5deb0 100644 --- a/.github/release.yml +++ b/.github/release.yml @@ -1,5 +1,8 @@ changelog: categories: + - title: Breaking Changes 🛠 + labels: + - breaking - title: 📦 Features labels: - '*' diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index ad4fdbc..ad2a830 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/app/presenters/hotwire_combobox/component.rb @@ -137,7 +137,7 @@ def paginated? end def pagination_attrs - { for_id: hidden_field_id, src: async_src } + { for_id: canonical_id, src: async_src } end private @@ -153,7 +153,7 @@ def infer_association_name def fieldset_data data.reverse_merge \ - async_id: hidden_field_id, + async_id: canonical_id, controller: view.token_list("hw-combobox", data[:controller]), hw_combobox_expanded_value: open, hw_combobox_name_when_new_value: name_when_new, @@ -186,10 +186,15 @@ def association_exists? end - def hidden_field_id + def canonical_id id || form&.field_id(name) end + + def hidden_field_id + "#{canonical_id}-hw-hidden-field" + end + def hidden_field_name form&.field_name(name) || name end @@ -204,7 +209,7 @@ def hidden_field_value def input_id - "#{hidden_field_id}-hw-combobox" + canonical_id end def input_type @@ -220,7 +225,7 @@ def input_data click@window->hw-combobox#closeOnClickOutside focusin@window->hw-combobox#closeOnFocusOutside".squish, hw_combobox_target: "combobox", - async_id: hidden_field_id + async_id: canonical_id end def input_aria @@ -241,7 +246,7 @@ def handle_data def listbox_id - "#{hidden_field_id}-hw-listbox" + "#{canonical_id}-hw-listbox" end def listbox_data @@ -262,7 +267,7 @@ def dialog_data end def dialog_input_id - "#{hidden_field_id}-hw-dialog-combobox" + "#{canonical_id}-hw-dialog-combobox" end def dialog_input_data @@ -284,7 +289,7 @@ def dialog_input_aria end def dialog_listbox_id - "#{hidden_field_id}-hw-dialog-listbox" + "#{canonical_id}-hw-dialog-listbox" end def dialog_listbox_data diff --git a/test/dummy/app/views/comboboxes/new_options.html.erb b/test/dummy/app/views/comboboxes/new_options.html.erb index 1b4e146..b759aa1 100644 --- a/test/dummy/app/views/comboboxes/new_options.html.erb +++ b/test/dummy/app/views/comboboxes/new_options.html.erb @@ -7,10 +7,10 @@ <% end %> <%= form_with model: User.new, url: new_options_form_path, method: :post do |form| %> - <%= form.label "Favorite State", for: "allow-new-hw-combobox" %> + <%= form.label "Favorite State", for: "allow-new" %> <%= form.combobox :favorite_state_id, State.all, id: "allow-new", name_when_new: "user[favorite_state_attributes][name]" %> - <%= form.label "Home State", for: "disallow-new-hw-combobox" %> + <%= form.label "Home State", for: "disallow-new" %> <%= form.combobox :home_state_id, State.all, id: "disallow-new" %> <%= form.submit %> diff --git a/test/helpers/hotwire_combobox/helper_test.rb b/test/helpers/hotwire_combobox/helper_test.rb index d052550..0e74bd0 100644 --- a/test/helpers/hotwire_combobox/helper_test.rb +++ b/test/helpers/hotwire_combobox/helper_test.rb @@ -29,7 +29,8 @@ class HotwireCombobox::HelperTest < ApplicationViewTestCase form = ActionView::Helpers::FormBuilder.new :foo, nil, self, {} tag = combobox_tag :bar, form: form - assert_attrs tag, name: "foo[bar]", id: "foo_bar" # name is not "bar" + assert_attrs tag, type: "hidden", name: "foo[bar]" # name is not "bar" + assert_attrs tag, id: "foo_bar", role: "combobox" # id is determined by the form builder end test "passing a form builder object overrides value" do diff --git a/test/system/hotwire_combobox_test.rb b/test/system/hotwire_combobox_test.rb index 2965dfe..92c8f8d 100644 --- a/test/system/hotwire_combobox_test.rb +++ b/test/system/hotwire_combobox_test.rb @@ -73,7 +73,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("Flo") + find("#state-field").send_keys("Flo") assert_selector "li[role=option]", text: "Florida" assert_no_selector "li[role=option]", text: "Alabama" end @@ -83,14 +83,14 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("Flor") - assert_field "state-field-hw-combobox", with: "Florida" - assert_field "state-field", type: "hidden", with: "FL" + find("#state-field").send_keys("Flor") + assert_field "state-field", with: "Florida" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" assert_selector "li[role=option].hw-combobox__option--selected", text: "Florida" - find("#state-field-hw-combobox").send_keys(:backspace) - assert_field "state-field-hw-combobox", with: "Flor" - assert_field "state-field", type: "hidden", with: nil + find("#state-field").send_keys(:backspace) + assert_field "state-field", with: "Flor" + assert_field "state-field-hw-hidden-field", type: "hidden", with: nil assert_no_selector "li[role=option].hw-combobox__option--selected" end @@ -99,9 +99,9 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("lor") - assert_field "state-field-hw-combobox", with: "lor" - assert_field "state-field", type: "hidden", with: "FL" + find("#state-field").send_keys("lor") + assert_field "state-field", with: "lor" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" assert_selector "li[role=option].hw-combobox__option--selected", text: "Florida" end @@ -110,16 +110,16 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("lor", :enter) + find("#state-field").send_keys("lor", :enter) assert_selector "input[aria-expanded=false]" - assert_field "state-field-hw-combobox", with: "Florida" - assert_field "state-field", type: "hidden", with: "FL" + assert_field "state-field", with: "Florida" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" assert_no_selector "li[role=option].hw-combobox__option--selected", text: "Florida" # because the list is closed - find("#state-field-hw-combobox").send_keys(:backspace) + find("#state-field").send_keys(:backspace) assert_selector "input[aria-expanded=true]" - assert_field "state-field-hw-combobox", with: "Florid" - assert_field "state-field", type: "hidden", with: nil + assert_field "state-field", with: "Florid" + assert_field "state-field-hw-hidden-field", type: "hidden", with: nil assert_no_selector "li[role=option].hw-combobox__option--selected" end @@ -128,11 +128,11 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("lor") + find("#state-field").send_keys("lor") click_on_edge assert_selector "input[aria-expanded=false]" - assert_field "state-field-hw-combobox", with: "Florida" - assert_field "state-field", type: "hidden", with: "FL" + assert_field "state-field", with: "Florida" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" end test "focusing away locks in the current selection" do @@ -140,11 +140,11 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("lor") + find("#state-field").send_keys("lor") find("body").send_keys(:tab) assert_selector "input[aria-expanded=false]" - assert_field "state-field-hw-combobox", with: "Florida" - assert_field "state-field", type: "hidden", with: "FL" + assert_field "state-field", with: "Florida" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" end test "navigating with the arrow keys" do @@ -152,33 +152,33 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys(:down) + find("#state-field").send_keys(:down) assert_selector "li[role=option].hw-combobox__option--selected", text: "Alabama" - find("#state-field-hw-combobox").send_keys(:down) + find("#state-field").send_keys(:down) assert_selector "li[role=option].hw-combobox__option--selected", text: "Florida" - find("#state-field-hw-combobox").send_keys(:down) + find("#state-field").send_keys(:down) assert_selector "li[role=option].hw-combobox__option--selected", text: "Michigan" - find("#state-field-hw-combobox").send_keys(:up) + find("#state-field").send_keys(:up) assert_selector "li[role=option].hw-combobox__option--selected", text: "Florida" - find("#state-field-hw-combobox").send_keys(:up) + find("#state-field").send_keys(:up) assert_selector "li[role=option].hw-combobox__option--selected", text: "Alabama" # wrap around - find("#state-field-hw-combobox").send_keys(:up) + find("#state-field").send_keys(:up) assert_selector "li[role=option].hw-combobox__option--selected", text: "Missouri" - find("#state-field-hw-combobox").send_keys(:down) + find("#state-field").send_keys(:down) assert_selector "li[role=option].hw-combobox__option--selected", text: "Alabama" # home and end keys - find("#state-field-hw-combobox").send_keys(:end) + find("#state-field").send_keys(:end) assert_selector "li[role=option].hw-combobox__option--selected", text: "Missouri" - find("#state-field-hw-combobox").send_keys(:home) + find("#state-field").send_keys(:home) assert_selector "li[role=option].hw-combobox__option--selected", text: "Alabama" end @@ -187,21 +187,21 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - assert_field "state-field-hw-combobox", with: nil - assert_field "state-field", type: "hidden", with: nil + assert_field "state-field", with: nil + assert_field "state-field-hw-hidden-field", type: "hidden", with: nil find("li[role=option]", text: "Florida").click assert_selector "input[aria-expanded=false]" - assert_field "state-field-hw-combobox", with: "Florida" - assert_field "state-field", type: "hidden", with: "FL" + assert_field "state-field", with: "Florida" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" end test "combobox with prefilled value" do visit prefilled_combobox_path assert_selector "input[aria-expanded=false]" - assert_field "state-field-hw-combobox", with: "Michigan" - assert_field "state-field", type: "hidden", with: "MI" + assert_field "state-field", with: "Michigan" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MI" open_combobox @@ -212,8 +212,8 @@ class HotwireComboboxTest < ApplicationSystemTestCase visit prefilled_form_combobox_path assert_selector "input[aria-expanded=false]" - assert_field "user_favorite_state_id-hw-combobox", with: "Michigan" - assert_field "user_favorite_state_id", type: "hidden", with: states(:mi).id + assert_field "user_favorite_state_id", with: "Michigan" + assert_field "user_favorite_state_id-hw-hidden-field", type: "hidden", with: states(:mi).id open_combobox "user_favorite_state_id" @@ -224,8 +224,8 @@ class HotwireComboboxTest < ApplicationSystemTestCase visit prefilled_async_combobox_path assert_selector "input[aria-expanded=false]" - assert_field "user_home_state_id-hw-combobox", with: "Florida" - assert_field "user_home_state_id", type: "hidden", with: states(:fl).id + assert_field "user_home_state_id", with: "Florida" + assert_field "user_home_state_id-hw-hidden-field", type: "hidden", with: states(:fl).id open_combobox "user_home_state_id" @@ -238,7 +238,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox assert_no_selector "input[aria-invalid=true]" - find("#state-field-hw-combobox").send_keys("Flor", :backspace, :enter) + find("#state-field").send_keys("Flor", :backspace, :enter) assert_selector "input[aria-invalid=true]" end @@ -248,7 +248,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox assert_no_selector "input[aria-invalid=true]" - find("#state-field-hw-combobox").send_keys("Flor", :backspace, :enter) + find("#state-field").send_keys("Flor", :backspace, :enter) assert_no_selector "input[aria-invalid=true]" end @@ -262,12 +262,12 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_difference -> { State.count }, +1 do visit new_options_combobox_path - find("#allow-new-hw-combobox").then do |combobox| + find("#allow-new").then do |combobox| combobox.click combobox.send_keys "Alaska", :enter end - find("#disallow-new-hw-combobox").then do |combobox| + find("#disallow-new").then do |combobox| combobox.click combobox.send_keys "Alabama", :enter end @@ -286,17 +286,17 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_difference -> { State.count }, +1 do visit new_options_combobox_path - find("#allow-new-hw-combobox").then do |combobox| + find("#allow-new").then do |combobox| combobox.click combobox.send_keys "Ala" - assert_field "allow-new", type: "hidden", with: states(:al).id + assert_field "allow-new-hw-hidden-field", type: "hidden", with: states(:al).id assert_selector "li[role=option][aria-selected=true]", text: "Alabama" assert_selector "input[name='user[favorite_state_id]']", visible: :hidden assert_no_selector "input[name='user[favorite_state_attributes][name]']", visible: :hidden combobox.send_keys :backspace - assert_field "allow-new", type: "hidden", with: "Ala" # backspace removes the autocompleted part, not the typed part + assert_field "allow-new-hw-hidden-field", type: "hidden", with: "Ala" # backspace removes the autocompleted part, not the typed part assert_no_selector "li[role=option][aria-selected=true]" assert_no_selector "input[name='user[favorite_state_id]']", visible: :hidden assert_selector "input[name='user[favorite_state_attributes][name]']", visible: :hidden @@ -317,7 +317,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase assert_no_difference -> { State.count } do visit new_options_combobox_path - find("#disallow-new-hw-combobox").then do |combobox| + find("#disallow-new").then do |combobox| combobox.click combobox.send_keys "Alaska", :enter end @@ -336,19 +336,19 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("mi") - assert_field "state-field", type: "hidden", with: "MI" - find("#state-field-hw-combobox").send_keys(:down, :down) - assert_field "state-field", type: "hidden", with: "MI" + find("#state-field").send_keys("mi") + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MI" + find("#state-field").send_keys(:down, :down) + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MI" - find("#state-field-hw-combobox").then do |input| + find("#state-field").then do |input| "Michigan".chars.each { input.send_keys(:backspace) } end - find("#state-field-hw-combobox").send_keys("mi") - assert_field "state-field", type: "hidden", with: "MI" - find("#state-field-hw-combobox").send_keys("n") - assert_field "state-field", type: "hidden", with: "MN" + find("#state-field").send_keys("mi") + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MI" + find("#state-field").send_keys("n") + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MN" end test "list-only autocomplete" do @@ -356,29 +356,29 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox - find("#state-field-hw-combobox").send_keys("mi") - assert_field "state-field", type: "hidden", with: "MI" - assert_field "state-field-hw-combobox", with: "mi" + find("#state-field").send_keys("mi") + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MI" + assert_field "state-field", with: "mi" - find("#state-field-hw-combobox").send_keys(:down, :down) - assert_field "state-field", type: "hidden", with: "MS" - assert_field "state-field-hw-combobox", with: "Mississippi" + find("#state-field").send_keys(:down, :down) + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MS" + assert_field "state-field", with: "Mississippi" assert_selector "li[role=option]", text: "Michigan" assert_selector "li[role=option]", text: "Minnesota" assert_selector "li[role=option]", text: "Mississippi" assert_selector "li[role=option]", text: "Missouri" - find("#state-field-hw-combobox").then do |input| + find("#state-field").then do |input| "Mississippi".chars.each { input.send_keys(:backspace) } end - find("#state-field-hw-combobox").send_keys("mi") + find("#state-field").send_keys("mi") click_on_edge - assert_field "state-field", type: "hidden", with: "MI" - assert_field "state-field-hw-combobox", with: "Michigan" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "MI" + assert_field "state-field", with: "Michigan" end test "dialog" do @@ -396,7 +396,7 @@ class HotwireComboboxTest < ApplicationSystemTestCase end click_on_edge - assert_field "state-field", type: "hidden", with: "FL" + assert_field "state-field-hw-hidden-field", type: "hidden", with: "FL" assert_no_selector "dialog[open]" end end @@ -410,12 +410,12 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "movie-field" - find("#movie-field-hw-combobox").then do |input| + find("#movie-field").then do |input| assert_text "12 Angry Men" input.send_keys("wh") - assert_field "movie-field-hw-combobox", with: "Whiplash" + assert_field "movie-field", with: "Whiplash" assert_selector "li[role=option]", count: 2 input.send_keys(:backspace) # clear autocompleted portion @@ -437,13 +437,13 @@ class HotwireComboboxTest < ApplicationSystemTestCase open_combobox "movie-field" - find("#movie-field-hw-combobox").send_keys("sn") - assert_field "movie-field-hw-combobox", with: "Snow White and the Seven Dwarfs" + find("#movie-field").send_keys("sn") + assert_field "movie-field", with: "Snow White and the Seven Dwarfs" end private def open_combobox(name = "state-field") - find("##{name}-hw-combobox").click + find("##{name}").click end def on_small_screen