diff --git a/app/controllers/units_controller.rb b/app/controllers/units_controller.rb index 95efe11..27ca5ac 100644 --- a/app/controllers/units_controller.rb +++ b/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 diff --git a/app/javascript/application.js b/app/javascript/application.js index 264c2c0..68b6814 100644 --- a/app/javascript/application.js +++ b/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() }) } diff --git a/app/views/units/_form.html.erb b/app/views/units/_form.html.erb index db95dbc..e2013a8 100644 --- a/app/views/units/_form.html.erb +++ b/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 %> - <%= 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" %> - <%= 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" %> <% 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 %> - <%= 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}) %> <% end %> diff --git a/app/views/units/_form_close.html.erb b/app/views/units/_form_close.html.erb index d8cdffe..e79ea3c 100644 --- a/app/views/units/_form_close.html.erb +++ b/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 %> diff --git a/app/views/units/_unit.html.erb b/app/views/units/_unit.html.erb index dda3faa..150dfa9 100644 --- a/app/views/units/_unit.html.erb +++ b/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 %> - <%= 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} %> <%= unit.description %> <%= unit.multiplier.to_html %> @@ -14,9 +13,8 @@ <% if current_user.at_least(:active) %> <% 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), diff --git a/app/views/units/create.turbo_stream.erb b/app/views/units/create.turbo_stream.erb index 21265d5..98beca6 100644 --- a/app/views/units/create.turbo_stream.erb +++ b/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) %> diff --git a/app/views/units/edit.turbo_stream.erb b/app/views/units/edit.turbo_stream.erb index d3c8bee..e154cca 100644 --- a/app/views/units/edit.turbo_stream.erb +++ b/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 -%> diff --git a/app/views/units/index.html.erb b/app/views/units/index.html.erb index e2b8258..249bac3 100644 --- a/app/views/units/index.html.erb +++ b/app/views/units/index.html.erb @@ -1,7 +1,7 @@
<% 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' %>
diff --git a/app/views/units/new.turbo_stream.erb b/app/views/units/new.turbo_stream.erb index 8cb7080..4df54de 100644 --- a/app/views/units/new.turbo_stream.erb +++ b/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 %> diff --git a/config/initializers/core_ext.rb b/config/initializers/core_ext.rb index fe6bd62..90b09b8 100644 --- a/config/initializers/core_ext.rb +++ b/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 diff --git a/config/initializers/turbo_streams_tag_builder.rb b/config/initializers/turbo_streams_tag_builder.rb index 4e09d55..6de180a 100644 --- a/config/initializers/turbo_streams_tag_builder.rb +++ b/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) diff --git a/config/locales/en.yml b/config/locales/en.yml index 1fa61c1..ae07695 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/lib/core_ext/action_view/record_identifier_with_suffix.rb b/lib/core_ext/action_view/record_identifier_with_suffix.rb new file mode 100644 index 0000000..2e5138b --- /dev/null +++ b/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 diff --git a/test/system/units_test.rb b/test/system/units_test.rb index ab30709..f4b9574 100644 --- a/test/system/units_test.rb +++ b/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