From 3dcad2116d94bfa09ae6b441bc465ba6a16a2d01 Mon Sep 17 00:00:00 2001 From: Josh Frankel Date: Thu, 2 Apr 2026 15:34:04 -0400 Subject: [PATCH 1/3] Ensure disabled state works as expected by changing links to buttons * Use WeakSet to avoid attaching multiple event handlers to the same element from multiple init() calls * Append hidden input within the button_to form to contain all selected batch_action_ids * Set Button disabled as default state * Ensure data-confirm is pulled into confirmation dialog (Rails 7+ doesn't confirm with UJS) --- .../administrate_batch_actions/script.js | 127 +++++++++++++----- .../_button.html.erb | 8 +- .../_checkbox.html.erb | 2 +- 3 files changed, 98 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/administrate_batch_actions/script.js b/app/assets/javascripts/administrate_batch_actions/script.js index 74508a1..45b3fb1 100644 --- a/app/assets/javascripts/administrate_batch_actions/script.js +++ b/app/assets/javascripts/administrate_batch_actions/script.js @@ -1,13 +1,25 @@ -const init = function() { - var buttons = document.querySelectorAll("[data-batch-action-option='button']"); - var checkboxes = document.querySelectorAll("[data-batch-action-option='checkbox']"); - var selectAllCheckboxes = document.querySelector("[data-batch-action-option='select_all']"); +const boundForms = new WeakSet(); +const boundCheckboxes = new WeakSet(); +const boundSelectAllCheckboxes = new WeakSet(); - if (selectAllCheckboxes && checkboxes && buttons) { +const init = function () { + var buttons = document.querySelectorAll( + "[data-batch-action-option='button']", + ); + const forms = document.querySelectorAll( + "form:has([data-batch-action-option='button'])", + ); + var checkboxes = document.querySelectorAll( + "[data-batch-action-option='checkbox']", + ); + var selectAllCheckboxes = document.querySelector( + "[data-batch-action-option='select_all']", + ); - window.onpageshow = function(event) { - if (selectedItemIds()) { - checkboxes.forEach(function(checkbox) { + if (selectAllCheckboxes && checkboxes && buttons) { + window.onpageshow = function (event) { + if (selectedItemIds() && selectedItemIds().length === 0) { + checkboxes.forEach(function (checkbox) { checkbox.checked = false; }); @@ -15,57 +27,98 @@ const init = function() { } }; - selectAllCheckboxes.addEventListener('click', function(){ - checkboxes.forEach(function(checkbox) { - checkbox.checked = selectAllCheckboxes.checked; + if (!isBoundElement(boundSelectAllCheckboxes, selectAllCheckboxes)) { + boundSelectAllCheckboxes.add(selectAllCheckboxes); + + selectAllCheckboxes.addEventListener("click", function () { + checkboxes.forEach(function (checkbox) { + checkbox.checked = selectAllCheckboxes.checked; + }); + + checkAndToggleActionButtons(); }); + } - checkAndToggleActionButtons(); - }); + forms.forEach(function (form) { + if (isBoundElement(boundForms, form)) return; + boundForms.add(form); + + const confirmMessage = + form.querySelector("[data-confirm]")?.dataset?.confirm || + "Are you sure you want to submit this form?"; - buttons.forEach(function(button){ - button.addEventListener('click', function(event){ - button.href += '?' + selectedItemIds() + form.addEventListener("submit", function (event) { + event.preventDefault(); + + if (confirm(confirmMessage) === true) { + selectedItemIds().forEach(function (id) { + const hiddenInput = document.createElement("input"); + hiddenInput.type = "hidden"; + hiddenInput.name = "batch_action_ids[]"; + hiddenInput.value = id; + form.appendChild(hiddenInput); + }); + form.submit(); + } else { + return; + } }); }); - checkboxes.forEach(function(checkbox){ - checkbox.closest('td').addEventListener('click', function(event){ + checkboxes.forEach(function (checkbox) { + if (isBoundElement(boundCheckboxes, checkbox)) { + return; + } + boundCheckboxes.add(checkbox); + + checkbox.closest("td").addEventListener("click", function (event) { event.stopImmediatePropagation(); - }) + }); - checkbox.addEventListener('click', function(event) { + checkbox.addEventListener("click", function (event) { event.stopImmediatePropagation(); checkAndToggleActionButtons(); - }) - }) + }); + }); } function selectedItemIds() { - var ids = Array.prototype.filter.call(checkboxes, function(checkbox) { - if (checkbox.checked) { return checkbox } - }).map(function(checkbox) { - return 'batch_action_ids[]=' + checkbox.value - }).join('&'); + var ids = Array.prototype.filter + .call(checkboxes, function (checkbox) { + if (checkbox.checked) { + return checkbox; + } + }) + .map(function (checkbox) { + return checkbox.value; + }); return ids; } function checkAndToggleActionButtons() { - if (selectedItemIds()) { - buttons.forEach(function(button){ - button.classList.remove('disabled'); - button.removeAttribute('disabled'); + if (selectedItemIds() && selectedItemIds().length > 0) { + buttons.forEach(function (button) { + button.disabled = false; }); } else { - buttons.forEach(function(button){ - button.classList.add('disabled'); - button.setAttribute('disabled', 'disabled'); + buttons.forEach(function (button) { + button.disabled = true; }); } } + + function isBoundElement(boundElement, incomingElement) { + return boundElement.has(incomingElement); + } }; -document.addEventListener("DOMContentLoaded", function() { init() }); -document.addEventListener("turbolinks:load", function() { init() }); -document.addEventListener("turbo:load", function() { init() }); +document.addEventListener("DOMContentLoaded", function () { + init(); +}); +document.addEventListener("turbolinks:load", function () { + init(); +}); +document.addEventListener("turbo:load", function () { + init(); +}); diff --git a/app/views/shared/administrate_batch_actions/_button.html.erb b/app/views/shared/administrate_batch_actions/_button.html.erb index f9eea45..de55928 100644 --- a/app/views/shared/administrate_batch_actions/_button.html.erb +++ b/app/views/shared/administrate_batch_actions/_button.html.erb @@ -1 +1,7 @@ -<%= link_to name, path, class: "btn disabled #{html_options[:class]}", data: { batch_action_option: 'button', confirm: html_options[:confirm] }, method: :post %> +<%= button_to path, + class: "btn #{html_options[:class]}", + data: { batch_action_option: 'button', confirm: html_options[:confirm] }, + method: :post, + disabled: true do %> + <%= name %> +<% end%> diff --git a/app/views/shared/administrate_batch_actions/_checkbox.html.erb b/app/views/shared/administrate_batch_actions/_checkbox.html.erb index accd3d4..2bd4e5a 100644 --- a/app/views/shared/administrate_batch_actions/_checkbox.html.erb +++ b/app/views/shared/administrate_batch_actions/_checkbox.html.erb @@ -1,3 +1,3 @@ - + From 5f993713ca5c3fc1f694737174ac6d0599f861bb Mon Sep 17 00:00:00 2001 From: Josh Frankel Date: Thu, 2 Apr 2026 16:17:45 -0400 Subject: [PATCH 2/3] Fix breaking specs --- spec/helpers_spec.rb | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/spec/helpers_spec.rb b/spec/helpers_spec.rb index 58471cc..d8ace46 100644 --- a/spec/helpers_spec.rb +++ b/spec/helpers_spec.rb @@ -2,15 +2,33 @@ RSpec.describe 'Helpers', type: :helper do it 'renders action button (default)' do - expect(helper.administrate_batch_actions_button('Delete All', '/')).to include 'Delete All' + expected_html = <<~HTML +
+ HTML + + expect(helper.administrate_batch_actions_button('Delete All', '/').squish).to include(expected_html.squish) end it 'renders action button (with class)' do - expect(helper.administrate_batch_actions_button('Delete All', '/', class: 'button text-danger')).to include 'Delete All' + expected_html = <<~HTML +
+ HTML + + expect(helper.administrate_batch_actions_button('Delete All', '/', class: 'button text-danger').squish).to include(expected_html.squish) end it 'renders action button (with confirm window)' do - expect(helper.administrate_batch_actions_button('Delete All', '/', class: 'button', confirm: 'Are you sure to do this?')).to include 'Delete All' + expected_html = <<~HTML +
+ HTML + + expect(helper.administrate_batch_actions_button('Delete All', '/', class: 'button', confirm: 'Are you sure to do this?').squish).to include(expected_html.squish) end it 'renders select all' do @@ -18,6 +36,10 @@ end it 'renders checkbox' do - expect(helper.administrate_batch_actions_checkbox(1)).to include '' + expected_html = <<~HTML + + HTML + + expect(helper.administrate_batch_actions_checkbox(1).squish).to include(expected_html.squish) end end From 2a71591376a47840120d8d23b7c8bacab3157805 Mon Sep 17 00:00:00 2001 From: Andrei Bondarev Date: Thu, 22 Oct 2026 19:01:12 -0400 Subject: [PATCH 3/3] Address review feedback on batch action buttons - Only show a confirm dialog when a confirm message is given, and use data-batch-action-confirm so rails-ujs doesn't prompt a second time - Let the native form submission proceed instead of form.submit(), only preventing it when the confirm is cancelled - Replace previously appended hidden ids so a resubmit doesn't duplicate them - Fix inverted back/forward cache reset and re-sync button state - Opt the form out of Turbo to keep the previous full-page POST behavior - Find each button's form via button.form instead of form:has() - Tidy partial formatting --- CHANGELOG.md | 1 + .../administrate_batch_actions/script.js | 70 +++++++++---------- .../_button.html.erb | 5 +- .../_checkbox.html.erb | 2 +- spec/helpers_spec.rb | 10 ++- 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e375a6..3235233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. - Clarified in the README that the button label is a string (#12) ### Changed +- Batch action buttons are now `button_to` forms that are truly disabled until rows are selected (#48) - Batch action routes are now `POST //administrate_batch_actions//`, named `__`, e.g. `admin_delete_batch_action_users_path` ## [0.4.0] - 2026-03-26 diff --git a/app/assets/javascripts/administrate_batch_actions/script.js b/app/assets/javascripts/administrate_batch_actions/script.js index 45b3fb1..6b7a465 100644 --- a/app/assets/javascripts/administrate_batch_actions/script.js +++ b/app/assets/javascripts/administrate_batch_actions/script.js @@ -6,9 +6,6 @@ const init = function () { var buttons = document.querySelectorAll( "[data-batch-action-option='button']", ); - const forms = document.querySelectorAll( - "form:has([data-batch-action-option='button'])", - ); var checkboxes = document.querySelectorAll( "[data-batch-action-option='checkbox']", ); @@ -17,14 +14,17 @@ const init = function () { ); if (selectAllCheckboxes && checkboxes && buttons) { + // Reset selections restored by the browser's back/forward cache window.onpageshow = function (event) { - if (selectedItemIds() && selectedItemIds().length === 0) { + if (selectedItemIds().length > 0) { checkboxes.forEach(function (checkbox) { checkbox.checked = false; }); selectAllCheckboxes.checked = false; } + + checkAndToggleActionButtons(); }; if (!isBoundElement(boundSelectAllCheckboxes, selectAllCheckboxes)) { @@ -39,29 +39,34 @@ const init = function () { }); } - forms.forEach(function (form) { - if (isBoundElement(boundForms, form)) return; + buttons.forEach(function (button) { + var form = button.form; + if (!form || isBoundElement(boundForms, form)) return; boundForms.add(form); - const confirmMessage = - form.querySelector("[data-confirm]")?.dataset?.confirm || - "Are you sure you want to submit this form?"; - form.addEventListener("submit", function (event) { - event.preventDefault(); - - if (confirm(confirmMessage) === true) { - selectedItemIds().forEach(function (id) { - const hiddenInput = document.createElement("input"); - hiddenInput.type = "hidden"; - hiddenInput.name = "batch_action_ids[]"; - hiddenInput.value = id; - form.appendChild(hiddenInput); - }); - form.submit(); - } else { + var confirmMessage = button.dataset.batchActionConfirm; + + if (confirmMessage && confirm(confirmMessage) !== true) { + event.preventDefault(); return; } + + // Replace ids from a previous submission (e.g. after navigating back) + form + .querySelectorAll("[data-batch-action-option='hidden_id']") + .forEach(function (hiddenInput) { + hiddenInput.remove(); + }); + + selectedItemIds().forEach(function (id) { + var hiddenInput = document.createElement("input"); + hiddenInput.type = "hidden"; + hiddenInput.name = "batch_action_ids[]"; + hiddenInput.value = id; + hiddenInput.dataset.batchActionOption = "hidden_id"; + form.appendChild(hiddenInput); + }); }); }); @@ -84,28 +89,21 @@ const init = function () { } function selectedItemIds() { - var ids = Array.prototype.filter + return Array.prototype.filter .call(checkboxes, function (checkbox) { - if (checkbox.checked) { - return checkbox; - } + return checkbox.checked; }) .map(function (checkbox) { return checkbox.value; }); - return ids; } function checkAndToggleActionButtons() { - if (selectedItemIds() && selectedItemIds().length > 0) { - buttons.forEach(function (button) { - button.disabled = false; - }); - } else { - buttons.forEach(function (button) { - button.disabled = true; - }); - } + var disabled = selectedItemIds().length === 0; + + buttons.forEach(function (button) { + button.disabled = disabled; + }); } function isBoundElement(boundElement, incomingElement) { diff --git a/app/views/shared/administrate_batch_actions/_button.html.erb b/app/views/shared/administrate_batch_actions/_button.html.erb index de55928..517eaba 100644 --- a/app/views/shared/administrate_batch_actions/_button.html.erb +++ b/app/views/shared/administrate_batch_actions/_button.html.erb @@ -1,7 +1,8 @@ <%= button_to path, class: "btn #{html_options[:class]}", - data: { batch_action_option: 'button', confirm: html_options[:confirm] }, + data: { batch_action_option: 'button', batch_action_confirm: html_options[:confirm].presence }, + form: { data: { turbo: false } }, method: :post, disabled: true do %> <%= name %> -<% end%> +<% end %> diff --git a/app/views/shared/administrate_batch_actions/_checkbox.html.erb b/app/views/shared/administrate_batch_actions/_checkbox.html.erb index 2bd4e5a..f39c71f 100644 --- a/app/views/shared/administrate_batch_actions/_checkbox.html.erb +++ b/app/views/shared/administrate_batch_actions/_checkbox.html.erb @@ -1,3 +1,3 @@ - + diff --git a/spec/helpers_spec.rb b/spec/helpers_spec.rb index d8ace46..15dadc1 100644 --- a/spec/helpers_spec.rb +++ b/spec/helpers_spec.rb @@ -3,7 +3,7 @@ RSpec.describe 'Helpers', type: :helper do it 'renders action button (default)' do expected_html = <<~HTML -
HTML @@ -13,7 +13,7 @@ it 'renders action button (with class)' do expected_html = <<~HTML -
HTML @@ -23,7 +23,7 @@ it 'renders action button (with confirm window)' do expected_html = <<~HTML -
HTML @@ -31,6 +31,10 @@ expect(helper.administrate_batch_actions_button('Delete All', '/', class: 'button', confirm: 'Are you sure to do this?').squish).to include(expected_html.squish) end + it 'renders action button (keeps query params in the path)' do + expect(helper.administrate_batch_actions_button('Delete All', '/?scope=active')).to include 'action="/?scope=active"' + end + it 'renders select all' do expect(helper.administrate_batch_actions_select_all).to include '' end