Focus forms properly on open and close

Closes #44

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

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")))

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
+    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
-    within('tbody tr:has(input[type=text])') { assert_selector ':focus' }
+    end
+    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
+    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
-    within('tbody tr:has(input[type=text])') { assert_selector ':focus' }
+    end
+    assert_not_equal form, find('tbody tr:has(input:focus)')
   end
 
   #test "edit" do