Ensure disabled state works as expected by changing links to buttons by joshmfrankel · Pull Request #48 · SourceLabsLLC/administrate_batch_actions

joshmfrankel · 2026-04-02T19:35:26Z

Why

The batch action buttons currently don't disable when checkboxes are toggled.

What

Convert anchor tag to button tag for better disable support
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)

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

Copilot

Pull request overview

This PR updates the Administrate Batch Actions UI to make “batch action” controls truly disable/enable based on checkbox selection by switching from links to real form buttons and adjusting the supporting JS to submit selected IDs.

Changes:

Replaced the batch action anchor link with a button_to form button (default disabled).
Updated the checkbox partial and JS to collect selected IDs and submit them via dynamically appended hidden inputs.
Added WeakSet-based guards to avoid rebinding event handlers across repeated init() calls.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`app/views/shared/administrate_batch_actions/_button.html.erb`	Swaps `link_to` for `button_to` with `disabled: true` and retains `data-confirm`.
`app/views/shared/administrate_batch_actions/_checkbox.html.erb`	Removes the checkbox `name` attribute and keeps only value + data hook for JS selection tracking.
`app/assets/javascripts/administrate_batch_actions/script.js`	Reworks selection tracking, toggles `button.disabled`, and submits selected IDs via hidden inputs + custom confirm handling.
`spec/helpers_spec.rb`	Updates helper specs to expect the new button/form and checkbox markup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-02T23:14:33Z

app/assets/javascripts/administrate_batch_actions/script.js

+    window.onpageshow = function (event) {
+      if (selectedItemIds() && selectedItemIds().length === 0) {
+        checkboxes.forEach(function (checkbox) {
          checkbox.checked = false;
        });

        selectAllCheckboxes.checked = false;
      }


window.onpageshow is currently clearing checkboxes only when selectedItemIds().length === 0. Since selectedItemIds() returns an array, this condition is the inverse of the previous behavior (it won’t clear when there are selected IDs, which is when BFCache/back-navigation tends to preserve checked boxes). Consider checking for > 0 (and re-running checkAndToggleActionButtons() after resetting) to keep button state consistent when navigating back.

Copilot · 2026-04-02T23:14:34Z

app/assets/javascripts/administrate_batch_actions/script.js

+      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) {


confirmMessage falls back to a default string when no element in the form has data-confirm, which means every batch action submission will now show a confirmation dialog. If confirmation is intended to be opt-in (only when confirm:/data-confirm is provided), gate the confirm() call on the presence of data-confirm instead of defaulting to a message.

Copilot · 2026-04-02T23:14:34Z

app/assets/javascripts/administrate_batch_actions/script.js

+        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;
+        }


The submit handler always preventDefault() and then calls form.submit(). Programmatic form.submit() bypasses normal submit event processing (and may bypass Turbo/UJS interception), changing navigation/remote-submit behavior. Prefer letting the original submission proceed after injecting the hidden inputs (only preventDefault on cancel), or use a submission method that preserves the normal submit pipeline.

Suggested change

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;

}

if (confirm(confirmMessage) !== true) {

event.preventDefault();

return;

}

form

.querySelectorAll("[data-batch-action-option='hidden_id']")

.forEach(function (hiddenInput) {

hiddenInput.remove();

});

selectedItemIds().forEach(function (id) {

const hiddenInput = document.createElement("input");

hiddenInput.type = "hidden";

hiddenInput.name = "batch_action_ids[]";

hiddenInput.value = id;

hiddenInput.dataset.batchActionOption = "hidden_id";

form.appendChild(hiddenInput);

});

Copilot · 2026-04-02T23:14:34Z

app/views/shared/administrate_batch_actions/_checkbox.html.erb

@@ -1,3 +1,3 @@
 <td>
-  <input type="checkbox" value="<%= value %>" name="batch_action_ids[]" data-batch-action-option="checkbox" />
+  <input type="checkbox" value="<%= value %>"  data-batch-action-option="checkbox" />


There’s an extra space between attributes (value=..." data-batch...). This is harmless but adds noise to generated HTML and specs; consider normalizing spacing.

Suggested change

<input type="checkbox" value="<%= value %>" data-batch-action-option="checkbox" />

<input type="checkbox" value="<%= value %>" data-batch-action-option="checkbox" />

Copilot · 2026-04-02T23:14:35Z

app/views/shared/administrate_batch_actions/_button.html.erb

+  method: :post,
+  disabled: true do %>
+  <%= name %>
+<% end%>


Minor ERB formatting: <% end%> is missing a space before %>. Aligning with the rest of the codebase’s ERB style (e.g., <% end %>) improves readability and avoids inconsistent diffs.

Suggested change

<% end%>

<% end %>

joshmfrankel · 2026-04-03T16:38:13Z

app/views/shared/administrate_batch_actions/_button.html.erb

+<%= button_to path,
+  class: "btn #{html_options[:class]}",
+  data: { batch_action_option: 'button', confirm: html_options[:confirm] },
+  method: :post,


TODO: This should also set turbo_method: :post

joshmfrankel changed the title ~~Ensure disabled state works as expected by changing links to buttons~~ WIP Ensure disabled state works as expected by changing links to buttons Apr 2, 2026

Fix breaking specs

5f99371

joshmfrankel marked this pull request as ready for review April 2, 2026 20:21

joshmfrankel changed the title ~~WIP Ensure disabled state works as expected by changing links to buttons~~ Ensure disabled state works as expected by changing links to buttons Apr 2, 2026

andreibondarev requested a review from Copilot April 2, 2026 23:10

Copilot started reviewing on behalf of andreibondarev April 2, 2026 23:10 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

joshmfrankel commented Apr 3, 2026

View reviewed changes

joshmfrankel marked this pull request as draft April 3, 2026 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure disabled state works as expected by changing links to buttons#48

Ensure disabled state works as expected by changing links to buttons#48
joshmfrankel wants to merge 2 commits intoSourceLabsLLC:mainfrom
joshmfrankel:joshmfrankel/fix-button-disable-state

joshmfrankel commented Apr 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

Copilot AI Apr 2, 2026

Uh oh!

joshmfrankel Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<input type="checkbox" value="<%= value %>" data-batch-action-option="checkbox" />
	<input type="checkbox" value="<%= value %>" data-batch-action-option="checkbox" />

Conversation

joshmfrankel commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

joshmfrankel Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joshmfrankel commented Apr 2, 2026 •

edited

Loading