fix(grid): bulk-bar Clear resets the row checkboxes, not just the toolbar

.changeset/grid-bulk-bar-clear-resets-checkboxes-4140.md

@@ -0,0 +1,9 @@
+---
+'@object-ui/plugin-grid': patch
+---
+
+ObjectGrid's bulk-bar **Clear** now unticks the row checkboxes, instead of only removing the toolbar
+
+Selecting rows and pressing Clear emptied the bulk-actions bar but left every row checkbox at `data-state="checked"` (the header checkbox stuck at `indeterminate` on a partial pick). The user was stranded on a page of ticked rows with no toolbar left to act on them, and the only way out was a reload or re-selecting and clearing through some other path.
+
+The selection lives in two places: `selectedRows`, which is the grid's own state and drives the toolbar, and the row checkboxes, which live inside the embedded data-table and only clear when `selectionResetKey` moves. `resetSelection()` writes all three, and the delete / dispatch / dialog-close paths have gone through it since the reset-key mechanism was introduced. Both `BulkActionBar` mount sites, however, hand-wrote their `onClearSelection` as `setSelectedRows([]); setSelectAllMatching(false);` — exactly `resetSelection()` minus the key bump — so Clear updated one source and left the other ticked. Both sites now call `resetSelection()`, so there is one reset for every path that clears a selection rather than three hand-copied ones, and the cross-page "all matching" state drops with it.

packages/plugin-grid/src/ObjectGrid.tsx

@@ -1935,11 +1935,18 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
   // the consumer-provided onBulkDelete (which already knows about confirm +
   // refresh). Other actions fall through to the generic action runner.
   //
-  // [#3056] Both branches must clear BOTH selection sources. `selectedRows` is
-  // ours and drives the toolbar; the row checkboxes live inside the data-table
-  // and only clear when `selectionResetKey` moves. Bumping one without the
-  // other strands the user on a page of ticked rows with no toolbar to act on
-  // them — the exact drift `handleBulkDialogClose` below already guards.
+  // [#3056] Every path that clears the selection must clear BOTH selection
+  // sources. `selectedRows` is ours and drives the toolbar; the row checkboxes
+  // live inside the data-table and only clear when `selectionResetKey` moves.
+  // Bumping one without the other strands the user on a page of ticked rows
+  // with no toolbar to act on them — the exact drift `handleBulkDialogClose`
+  // below already guards.
+  //
+  // [#4140] So this is the ONE reset — dispatch, delete, dialog-close AND the
+  // bulk bar's own `onClearSelection`. Both `BulkActionBar` sites previously
+  // hand-wrote `setSelectedRows([]); setSelectAllMatching(false);`, which is
+  // this function minus the key bump: Clear emptied the toolbar and left every
+  // checkbox ticked. Never re-implement the reset inline — call this.
   const resetSelection = () => {
     setSelectedRows([]);
     setSelectAllMatching(false);
@@ -2953,7 +2960,7 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
                 objectFields={objectSchema?.fields}
                 onAction={dispatchBulkAction}
                 onActionDef={dispatchBulkActionDef}
-                onClearSelection={() => { setSelectedRows([]); setSelectAllMatching(false); }}
+                onClearSelection={resetSelection}
                 pageSize={data.length}
                 totalMatching={singleSelection ? undefined : totalMatching}
                 allMatchingSelected={selectAllMatching}
@@ -2991,7 +2998,7 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
         objectFields={objectSchema?.fields}
         onAction={dispatchBulkAction}
         onActionDef={dispatchBulkActionDef}
-        onClearSelection={() => { setSelectedRows([]); setSelectAllMatching(false); }}
+        onClearSelection={resetSelection}
         pageSize={data.length}
         totalMatching={singleSelection ? undefined : totalMatching}
         allMatchingSelected={selectAllMatching}

packages/plugin-grid/src/__tests__/bulkActionRefresh.test.tsx

@@ -21,7 +21,7 @@
  * the missing refresh.
  */
 import { describe, it, expect, vi, beforeAll } from 'vitest';
-import { render, screen, waitFor, fireEvent } from '@testing-library/react';
+import { render, screen, waitFor, fireEvent, within } from '@testing-library/react';
 import '@testing-library/jest-dom';
 import React from 'react';
 
@@ -373,3 +373,119 @@ describe('ObjectGrid — dispatchBulkAction keeps selection in sync (#3056)', ()
     await waitFor(() => expect(headerChecked()).toBe('unchecked'));
   });
 });
+
+/**
+ * The SAME desync, the third selection-clearing site (#4140) — the bulk bar's
+ * own **Clear** button. `resetSelection()` above upholds the #3056 invariant for
+ * every dispatch path, but both `onClearSelection` props were hand-written as
+ * `setSelectedRows([]); setSelectAllMatching(false);` — the two state writes
+ * without the `selectionResetKey` bump. Clicking Clear therefore removed the
+ * toolbar and left every row `data-state="checked"` (header `indeterminate` on a
+ * partial pick): stranded ticks with no toolbar to act on them, which is the
+ * literal failure the reset-key mechanism exists to prevent.
+ *
+ * Found by the records-forms QA run (objectstack#7439, `bulk-select-all-matching`
+ * FAIL, reproduced 2x). These pin the button on BOTH selection sources and on
+ * both bar instances' shared handler, so the next hand-copy cannot drift again.
+ */
+describe('ObjectGrid — bulk bar Clear resets both selection sources (#4140)', () => {
+  const headerChecked = () =>
+    (document.querySelector('thead [role="checkbox"]') as HTMLElement)?.getAttribute('data-state');
+  const rowStates = () =>
+    Array.from(document.querySelectorAll('tbody [role="checkbox"]')).map((el) =>
+      el.getAttribute('data-state'),
+    );
+  const clickClear = () =>
+    fireEvent.click(
+      within(screen.getByTestId('bulk-actions-bar')).getByRole('button', { name: 'Clear' }),
+    );
+
+  it('unticks the row checkboxes (not just the toolbar) on a partial selection', async () => {
+    const ds = makeDataSource();
+    renderGrid(ds, { approve: vi.fn(async () => ({ success: true })) });
+
+    await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
+
+    // Tick ONE of the two rows — the header goes `indeterminate`, which is the
+    // exact state the QA repro reported as stuck after Clear.
+    const firstRow = document.querySelectorAll('tbody [role="checkbox"]')[0] as HTMLElement;
+    expect(firstRow).toBeTruthy();
+    fireEvent.click(firstRow);
+    await waitFor(() => expect(headerChecked()).toBe('indeterminate'));
+    expect(rowStates()).toEqual(['checked', 'unchecked']);
+    expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument();
+
+    clickClear();
+
+    // Both sources reset in lockstep: toolbar gone AND the boxes untick.
+    await waitFor(() =>
+      expect(screen.queryByTestId('bulk-actions-bar')).not.toBeInTheDocument(),
+    );
+    await waitFor(() => expect(rowStates()).toEqual(['unchecked', 'unchecked']));
+    expect(headerChecked()).toBe('unchecked');
+  });
+
+  it('unticks the row checkboxes when the whole page was selected', async () => {
+    const ds = makeDataSource();
+    renderGrid(ds, { approve: vi.fn(async () => ({ success: true })) });
+
+    await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
+    fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement);
+    await waitFor(() => expect(headerChecked()).toBe('checked'));
+
+    clickClear();
+
+    await waitFor(() =>
+      expect(screen.queryByTestId('bulk-actions-bar')).not.toBeInTheDocument(),
+    );
+    await waitFor(() => expect(headerChecked()).toBe('unchecked'));
+    expect(rowStates()).toEqual(['unchecked', 'unchecked']);
+  });
+
+  it('drops the cross-page "all matching" state too, so a fresh pick starts clean', async () => {
+    // `total` exceeds the returned page, so the bar offers "Select all N matching".
+    const ds = makeDataSource();
+    ds.find = vi.fn(async () => {
+      const data = Object.values(ds.store).map((r: any) => ({ ...r }));
+      return { data, total: 5, hasMore: true, pageSize: 50 };
+    });
+    renderGrid(ds, { approve: vi.fn(async () => ({ success: true })) });
+
+    await waitFor(() => expect(screen.getByText('Plan A')).toBeInTheDocument());
+    fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement);
+    await waitFor(() => expect(headerChecked()).toBe('checked'));
+
+    // Escalate to the cross-page selection, then Clear it.
+    //
+    // The banner's two branches are told apart *structurally*, by whether the
+    // escalation button is still offered — not by its copy. These grid tests
+    // wrap in `ActionProvider` only, with no `I18nProvider`, so `{{count}}`
+    // interpolation never runs and count-bearing strings reach the DOM as the
+    // raw template ("All {{count}} matching records are selected."). See
+    // BulkActionBar.test.tsx, which asserts on that copy and therefore *does*
+    // wrap in an English `I18nProvider`. Asserting the rendered count here
+    // would pin i18n plumbing rather than the reset, and its negation would
+    // pass for the wrong reason — the interpolated text is absent either side
+    // of the fix. Structure is the right probe for a selection-state question.
+    fireEvent.click(await screen.findByTestId('bulk-select-all-matching'));
+    await waitFor(() =>
+      expect(screen.queryByTestId('bulk-select-all-matching')).not.toBeInTheDocument(),
+    );
+    expect(screen.getByTestId('bulk-cross-page-banner')).toBeInTheDocument();
+
+    clickClear();
+
+    await waitFor(() =>
+      expect(screen.queryByTestId('bulk-actions-bar')).not.toBeInTheDocument(),
+    );
+    await waitFor(() => expect(headerChecked()).toBe('unchecked'));
+
+    // Re-picking the page must start from scratch: the header checkbox is live
+    // again (it would toggle the *other* way if the table still held the old
+    // selection) and the bar offers the escalation rather than remembering it —
+    // the button being back is exactly `selectAllMatching === false`.
+    fireEvent.click(document.querySelector('thead [role="checkbox"]') as HTMLElement);
+    await waitFor(() => expect(headerChecked()).toBe('checked'));
+    expect(await screen.findByTestId('bulk-select-all-matching')).toBeInTheDocument();
+  });
+});