Allow opening multiple new/edit forms

Closes #30

app/controllers/units_controller.rb

@@ -48,6 +48,8 @@ class UnitsController < ApplicationController
       flash.now[:notice] = t(".multiplier_reset", unit: @unit)
     end
   ensure
+    # TODO: Avoid double table width change by first un-hiding table header,
+    # then displaying index, e.g. by re-displaying header in index
     run_and_render :index
   end
 

app/javascript/application.js

@@ -15,20 +15,6 @@ Turbo.StreamElement.prototype.enableElement = function(element) {
     element.removeAttribute("tabindex")
 }
 
-Turbo.StreamElement.prototype.removePreviousForm = function(form) {
-  const id = form.id
-  const row = document.getElementById(id + "_cached")
-  form.remove()
-  if (row) {
-    row.id = id
-    row.removeAttribute("style")
-  }
-  if (form.hasAttribute("data-link-id")) {
-    const link = document.getElementById(form.getAttribute("data-link-id"))
-    this.enableElement(link)
-  }
-}
-
 
 Turbo.StreamActions.disable = function() {
   this.targetElements.forEach((e) => {
@@ -42,48 +28,19 @@ Turbo.StreamActions.enable = function() {
   this.targetElements.forEach((e) => { this.enableElement(e) })
 }
 
-Turbo.StreamActions.blur = function() {
-  blur()
-}
-
-Turbo.StreamActions.focus = function() {
-  // NOTE: call blur() before setting focus?
-  this.targetElements[0].focus({focusVisible: true})
-}
-
-Turbo.StreamActions.prepend_form = function() {
-  this.targetElements.forEach((e) => {
-    [...e.getElementsByClassName("form")].forEach((f) => {
-      this.removePreviousForm(f)
-    })
-    e.prepend(this.templateContent)
-  })
-}
-
-Turbo.StreamActions.after_form = function() {
-  this.targetElements.forEach((e) => {
-    [...e.parentElement?.getElementsByClassName("form")].forEach((f) => {
-      this.removePreviousForm(f)
-    })
-    e.parentElement?.insertBefore(this.templateContent, e.nextSibling)
-  })
-}
-
-Turbo.StreamActions.replace_form = function() {
-  this.targetElements.forEach((e) => {
-    const id = e.id;
-    [...e.parentElement?.getElementsByClassName("form")].forEach((f) => {
-      this.removePreviousForm(f)
-    })
-    e = document.getElementById(id)
-    e.style.display = "none"
-    e.id = e.id + "_cached"
-    e.parentElement?.insertBefore(this.templateContent, e.nextSibling)
-  })
+Turbo.StreamActions.hide = function() {
+  this.targetElements.forEach((e) => { e.style.display = "none" })
 }
 
 Turbo.StreamActions.close_form = function() {
   this.targetElements.forEach((e) => {
-    this.removePreviousForm(e.closest(".form"))
+    document.getElementById(e.getAttribute("data-form")).remove()
+    if (e.hasAttribute("data-link")) {
+      this.enableElement(document.getElementById(e.getAttribute("data-link")))
+    }
+    if (e.hasAttribute("data-hidden-row")) {
+      document.getElementById(e.getAttribute("data-hidden-row")).removeAttribute("style")
+    }
+    e.remove()
   })
 }

app/views/units/_form.html.erb

@@ -1,28 +1,28 @@
 <%= tabular_fields_for @unit do |form| %>
-  <%= tag.tr id: dom_id(@unit), class: "form", onkeydown: "processKey(event)",
-    data: {link_id: link_id} do %>
+  <%- tag.tr id: row, class: "form", onkeydown: "processKey(event)",
+    data: {link: link, form: form_tag, hidden_row: hidden_row} do %>
 
     <td class="<%= class_names({subunit: @unit.base}) %>">
-      <%= form.text_field :symbol, form: :unit_form, required: true, autofocus: true, size: 12,
+      <%= form.text_field :symbol, form: form_tag, required: true, autofocus: true, size: 12,
         maxlength: @unit.class.type_for_attribute(:symbol).limit, autocomplete: "off" %>
     </td>
     <td>
-      <%= form.text_area :description, form: :unit_form, cols: 30, rows: 1, escape: false,
+      <%= form.text_area :description, form: form_tag, cols: 30, rows: 1, escape: false,
         maxlength: @unit.class.type_for_attribute(:description).limit, autocomplete: "off" %>
     </td>
     <td>
       <% unless @unit.base.nil? %>
-        <%= form.hidden_field :base_id, form: :unit_form %>
-        <%= form.number_field :multiplier, form: :unit_form, required: true,
+        <%= form.hidden_field :base_id, form: form_tag %>
+        <%= form.number_field :multiplier, form: form_tag, required: true,
           size: 10, autocomplete: "off",
           **number_attributes(@unit.class.type_for_attribute(:multiplier)) %>
       <% end %>
     </td>
 
     <td class="actions">
-      <%= form.submit form: :unit_form %>
+      <%= form.submit form: form_tag %>
       <%= image_link_to t(:cancel), "close-outline", units_path, class: 'dangerous',
-        name: :cancel, onclick: render_turbo_stream('form_close') %>
+        name: :cancel, onclick: render_turbo_stream('form_close', {row: row}) %>
     </td>
     <td></td>
   <% end %>

app/views/units/_form_close.html.erb

@@ -1,2 +1,2 @@
-<%= turbo_stream.close_form @unit %>
+<%= turbo_stream.close_form row %>
 <%= turbo_stream.update :flashes %>

app/views/units/_unit.html.erb

@@ -5,8 +5,7 @@
            data: {drag_path: rebase_unit_path(unit), drop_id: dom_id(unit.base || unit)} do %>
 
   <td class="<%= class_names('link', {subunit: unit.base}) %>">
-    <%= link_to unit, edit_unit_path(unit), id: dom_id(unit, :edit),
-      onclick: 'this.blur();', data: {turbo_stream: true} %>
+    <%= link_to unit, edit_unit_path(unit), onclick: 'this.blur();', data: {turbo_stream: true} %>
   </td>
   <td><%= unit.description %></td>
   <td class="number"><%= unit.multiplier.to_html %></td>
@@ -14,9 +13,8 @@
   <% if current_user.at_least(:active) %>
     <td class="actions">
       <% if unit.base.nil? %>
-        <%= image_link_to t('.add_subunit'), 'plus-outline', new_unit_path(unit),
-          id: dom_id(unit, :add), onclick: 'this.blur();',
-          data: {turbo_stream: true} %>
+        <%= image_link_to t('.new_subunit'), 'plus-outline', new_unit_path(unit),
+          id: dom_id(unit, :new, :link), onclick: 'this.blur();', data: {turbo_stream: true} %>
       <% end %>
 
       <%= image_button_to t('.delete_unit'), 'delete-outline', unit_path(unit),

app/views/units/create.turbo_stream.erb

@@ -1,4 +1,4 @@
-<%= turbo_stream.close_form dom_id(Unit.new) %>
+<%= turbo_stream.close_form dom_id(@unit.base || Unit, :new) %>
 <%= turbo_stream.remove :no_items %>
 <%= turbo_stream.replace @unit.base unless @unit.base.nil? %>
 <%= @before.nil? ? turbo_stream.append(:units, @unit) : turbo_stream.before(@before, @unit) %>

app/views/units/edit.turbo_stream.erb

@@ -1,6 +1,13 @@
-<%= turbo_stream.replace :unit_form do %>
-  <%= form_with model: @unit, html: {id: :unit_form} do %>
+<% ids = {row: dom_id(@unit, :edit),
+          hidden_row: dom_id(@unit),
+          link: nil,
+          form_tag: dom_id(@unit, :edit, :form)} %>
+
+<%= turbo_stream.append :unit_form do %>
+  <%- form_with model: @unit, html: {id: ids[:form_tag]} do %>
   <% end %>
 <% end %>
 
-<%= turbo_stream.replace_form @unit, partial: 'form', locals: {link_id: dom_id(@unit, :edit)} %>
+<%= turbo_stream.hide ids[:hidden_row] %>
+<%= turbo_stream.remove ids[:row] %>
+<%= turbo_stream.after @unit, partial: 'form', locals: ids -%>

app/views/units/index.html.erb

@@ -1,7 +1,7 @@
 <div class="rightside buttongrid">
   <% if current_user.at_least(:active) %>
-    <%= image_link_to t('.add_unit'), 'plus-outline', new_unit_path, id: :add_unit,
-      onclick: 'this.blur();', data: {turbo_stream: true} %>
+    <%= image_link_to t('.new_unit'), 'plus-outline', new_unit_path,
+      id: dom_id(Unit, :new, :link), onclick: 'this.blur();', data: {turbo_stream: true} %>
   <% end %>
   <%= image_link_to t('.import_units'), 'download-outline', default_units_path, class: 'tools' %>
 </div>

app/views/units/new.turbo_stream.erb

@@ -1,9 +1,19 @@
-<% link_id = dom_id(@unit.base || @unit, :add) %>
+<% dom_obj = @unit.base || @unit %>
+<% ids = {row: dom_id(dom_obj, :new),
+          hidden_row: nil,
+          link: dom_id(dom_obj, :new, :link),
+          form_tag: dom_id(dom_obj, :new, :form)} %>
 
-<%= turbo_stream.replace :unit_form do %>
-  <%= form_with model: @unit, html: {id: :unit_form} do %>
+<%= turbo_stream.disable ids[:link] -%>
+
+<%= turbo_stream.append :unit_form do %>
+  <%- form_with model: @unit, html: {id: ids[:form_tag]} do %>
   <% end %>
 <% end %>
 
-<%= turbo_stream.insert_form (@unit.base || :units), partial: 'form', locals: {link_id: link_id} %>
-<%= turbo_stream.disable link_id -%>
+<% if @unit.base %>
+  <%= turbo_stream.remove ids[:row] %>
+  <%= turbo_stream.after @unit.base, partial: 'form', locals: ids %>
+<% else %>
+  <%= turbo_stream.prepend :units, partial: 'form', locals: ids %>
+<% end %>

config/initializers/core_ext.rb

@@ -1,9 +1,14 @@
 require 'core_ext/big_decimal_scientific_notation'
 
-ActiveSupport.on_load :active_record do
-  ActiveModel::Validations::NumericalityValidator.prepend CoreExt::ActiveModel::Validations::NumericalityValidatesPrecisionAndScale
-end
-
 ActiveSupport.on_load :action_dispatch_system_test_case do
   prepend CoreExt::ActionDispatch::SystemTesting::TestHelpers::ScreenshotHelperUniqueId
 end
+
+ActiveSupport.on_load :action_view do
+  ActionView::RecordIdentifier.prepend CoreExt::ActionView::RecordIdentifierWithSuffix
+end
+
+ActiveSupport.on_load :active_record do
+  ActiveModel::Validations::NumericalityValidator
+    .prepend CoreExt::ActiveModel::Validations::NumericalityValidatesPrecisionAndScale
+end

config/initializers/turbo_streams_tag_builder.rb

@@ -15,24 +15,8 @@ ActiveSupport.on_load :turbo_streams_tag_builder do
     action_all :enable, targets, allow_inferred_rendering: false
   end
 
-  def blur_all
-    action :blur, nil, allow_inferred_rendering: false
-  end
-
-  def focus(target)
-    action :focus, target, allow_inferred_rendering: false
-  end
-
-  def insert_form(target, content = nil, **rendering, &block)
-    if target.is_a? Symbol
-      action :prepend_form, target, content, **rendering, &block
-    else
-      action :after_form, target, content, **rendering, &block
-    end
-  end
-
-  def replace_form(target, content = nil, **rendering, &block)
-    action :replace_form, target, content, **rendering, &block
+  def hide(target)
+    action :hide, target, allow_inferred_rendering: false
   end
 
   def close_form(target)

config/locales/en.yml

@@ -57,10 +57,10 @@ en:
       users: Users
   units:
     unit:
-      add_subunit: Subunit
+      new_subunit: Subunit
       delete_unit: Delete
     index:
-      add_unit: Add unit
+      new_unit: Add unit
       import_units: Import
       no_items: There are no configured units. You can Add some or Import from defaults.
       top_level_drop: Drop here to reposition into top-level unit

lib/core_ext/action_view/record_identifier_with_suffix.rb

@@ -0,0 +1,9 @@
+module CoreExt::ActionView::RecordIdentifierWithSuffix
+  def dom_id(object, prefix = nil, suffix = nil)
+    if suffix
+      "#{super(object, prefix)}#{::ActionView::RecordIdentifier::JOIN}#{suffix}"
+    else
+      super(object, prefix)
+    end
+  end
+end

test/system/units_test.rb

@@ -7,8 +7,8 @@ require "application_system_test_case"
 
 class UnitsTest < ApplicationSystemTestCase
   LINK_LABELS = {
-    add_unit: t('units.index.add_unit'),
-    add_subunit: t('units.unit.add_subunit'),
+    new_unit: t('units.index.new_unit'),
+    new_subunit: t('units.unit.new_subunit'),
     edit: nil
   }
 
@@ -35,10 +35,10 @@ class UnitsTest < ApplicationSystemTestCase
   end
 
   test "new and create" do
-    type, label = LINK_LABELS.assoc([:add_unit, :add_subunit].sample)
-    add_link = all(:link, exact_text: label).sample
-    add_link.click
-    assert_equal 'disabled', add_link[:disabled]
+    type, label = LINK_LABELS.assoc([:new_unit, :new_subunit].sample)
+    new_link = all(:link, exact_text: label).sample
+    new_link.click
+    assert_equal 'disabled', new_link[:disabled]
 
     values = nil
     within 'tbody > tr:has(input[type=text], textarea)' do
@@ -52,7 +52,7 @@ class UnitsTest < ApplicationSystemTestCase
       }.with_indifferent_access
       within :field, 'unit[multiplier]' do |field|
         values[:multiplier] = random_number(field[:max], field[:step])
-      end if type == :add_subunit
+      end if type == :new_subunit
 
       values.each_pair { |name, value| fill_in "unit[#{name}]", with: value }
 
@@ -67,7 +67,7 @@ class UnitsTest < ApplicationSystemTestCase
       assert_selector 'tr', count: @user.units.count
     end
     assert_no_selector :element, :a, 'disabled': 'disabled',
-      exact_text: Regexp.union(LINK_LABELS.fetch_values(:add_unit, :add_subunit))
+      exact_text: Regexp.union(LINK_LABELS.fetch_values(:new_unit, :new_subunit))
     assert_equal values, Unit.last.attributes.slice(*values.keys)
   end
 
@@ -111,7 +111,7 @@ class UnitsTest < ApplicationSystemTestCase
       targets[type] = links[type].sample
     end
     # Define tr count change depending on link clicked
-    row_change = {add_unit: 1, add_subunit: 1, edit: 0}
+    row_change = {new_unit: 1, new_subunit: 1, edit: 0}
 
     type, link = targets.assoc(targets.keys.sample).tap { |t, _| targets.delete(t) }
     rows = row_change[type]
@@ -121,14 +121,14 @@ class UnitsTest < ApplicationSystemTestCase
     within('tbody tr:has(input[type=text])') { assert_selector ':focus' }
     if type == :edit
       assert !link.visible?
-      [:add_subunit, :edit].each do |t|
+      [:new_subunit, :edit].each do |t|
         assert_difference(->{ links[t].length }, -1) { links[t].select!(&:visible?) }
       end
     else
       assert link[:disabled]
     end
 
-    targets.merge([:add_subunit, :edit].map { |t| [t, links[t].sample] }.to_h)
+    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
@@ -136,9 +136,13 @@ class UnitsTest < ApplicationSystemTestCase
     within('tbody tr:has(input[type=text])') { assert_selector ':focus' }
   end
 
+  test "edit" do
+    # NOTE: Check if displayed attributes match record
+  end
+
   # NOTE: extend with any add/edit link
   test "close new unit form with escape key" do
-    click_on t('units.index.add_unit')
+    click_on t('units.index.new_unit')
     first('tbody > tr').all(:field).sample.send_keys :escape
     within 'tbody' do
       assert_no_selector :fillable_field
@@ -147,12 +151,15 @@ class UnitsTest < ApplicationSystemTestCase
 
   # NOTE: extend with any add/edit link
   test "close and reopen new unit form" do
-    click_on t('units.index.add_unit')
+    click_on t('units.index.new_unit')
     within 'tbody' do
       find(:link_or_button, exact_text: t(:cancel)).click
       assert_no_selector :fillable_field
     end
-    click_on t('units.index.add_unit')
+    click_on t('units.index.new_unit')
     assert_selector 'tbody > tr:has(input, textarea)'
   end
+
+  test "destroy" do
+  end
 end