diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index acc4022..28aa7c5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -65,7 +65,9 @@ module ApplicationHelper end def tabular_fields_for(record_name, record_object = nil, options = {}, &block) - options.merge! builder: TabularFormBuilder + # skip_default_ids causes turbo to generate unique ID for element with [autofocus]. + # Otherwise IDs are not unique when multiple forms are open and the first input gets focus. + options.merge! builder: TabularFormBuilder, skip_default_ids: true render_errors(record_name) fields_for(record_name, record_object, **options, &block) end diff --git a/app/javascript/application.js b/app/javascript/application.js index 68b6814..8680aae 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -34,6 +34,13 @@ Turbo.StreamActions.hide = function() { Turbo.StreamActions.close_form = function() { this.targetElements.forEach((e) => { + /* Move focus if there's no focus or focus inside form being closed */ + const focused = document.activeElement + if (!focused || (focused == document.body) || e.contains(focused)) { + let nextForm = e.parentElement.querySelector(`#${e.id} ~ tr:has([autofocus])`) + nextForm ??= e.parentElement.querySelector("tr:has([autofocus])") + nextForm?.querySelector("[autofocus]").focus() + } document.getElementById(e.getAttribute("data-form")).remove() if (e.hasAttribute("data-link")) { this.enableElement(document.getElementById(e.getAttribute("data-link"))) diff --git a/test/system/units_test.rb b/test/system/units_test.rb index 6c53c48..b005f02 100644 --- a/test/system/units_test.rb +++ b/test/system/units_test.rb @@ -101,9 +101,7 @@ class UnitsTest < ApplicationSystemTestCase end end - # TODO: add non-empty form closing warning - test "new and edit disallow opening multiple forms" do - # Once new/edit form is open, attempt to open another one will close it + test "new and edit allow opening multiple forms" do links = {} targets = {} LINK_LABELS.each_pair do |type, labels| @@ -111,14 +109,16 @@ class UnitsTest < ApplicationSystemTestCase targets[type] = links[type].sample end # Define tr count change depending on link clicked - row_change = {new_unit: 1, new_subunit: 1, edit: 0} + tr_diff = {new_unit: 1, new_subunit: 1, edit: 0} type, link = targets.assoc(targets.keys.sample).tap { |t, _| targets.delete(t) } - rows = row_change[type] - assert_difference ->{ all('tbody tr').count }, rows do - link.click + assert_difference ->{ all('tbody tr').count }, tr_diff[type] do + assert_difference ->{ all('tbody tr:has(input[type=text])').count }, 1 do + link.click + end end - within('tbody tr:has(input[type=text])') { assert_selector ':focus' } + form = find('tbody tr:has(input:focus)') + if type == :edit assert !link.visible? [:new_subunit, :edit].each do |t| @@ -130,10 +130,12 @@ class UnitsTest < ApplicationSystemTestCase targets.merge([:new_subunit, :edit].map { |t| [t, links[t].sample] }.to_h) type, link = targets.assoc(targets.keys.sample) - assert_difference ->{ all('tbody tr').count }, row_change[type] - rows do - link.click + assert_difference ->{ all('tbody tr').count }, tr_diff[type] do + assert_difference ->{ all('tbody tr:has(input[type=text])').count }, 1 do + link.click + end end - within('tbody tr:has(input[type=text])') { assert_selector ':focus' } + assert_not_equal form, find('tbody tr:has(input:focus)') end #test "edit" do