Skip to content

Commit fec60e0

Browse files
fix(components): select-all draws a dash for a partial selection (#27814)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgDFH4S5pR1C8YvYCWWsZfXx)_ ## 维护者速读 - 改了什么:列表(data-table,ObjectGrid 也经它渲染)表头全选框在「部分选中」时画一条横杠,不再画对勾;全选仍是对勾,未选为空。 - 为什么改:部分选中时表头看起来和「全部选中」一模一样,用户可能误以为整页都选上了再去跑批量操作。 - 风险与回滚:只换了表头全选这一处的复选框组件,行内复选框与上游 Shadcn `ui/checkbox.tsx` 未动;回滚即还原 data-table 那一行 import 与标签。 - 先看:`ui/packages/components/src/custom/tri-state-checkbox.tsx`(新的内部包装),`data-table.tsx` 表头全选那一处。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/components/src/renderers/complex/__tests__/data-table-selectAllMixed-27807.test.tsx`,预期 3/3 通过。 - 需要你做的:无,席位落地。 Fixes #27807 Clause-②: no Metadata-protocol: none ## What changes - New package-internal `TriStateCheckbox` in `ui/packages/components/src/custom/tri-state-checkbox.tsx`, built on the Radix checkbox primitive with the same classes as the Shadcn `Checkbox`: `checked="indeterminate"` draws a lucide `Minus` (dash) and takes the checked fill; `true` draws the check; `false` draws nothing. It is **not** re-exported from `custom/index.ts`, so the package's public surface is unchanged (`Clause-②: no`). - `data-table.tsx`: the header select-all (the only call site in `ui/packages/**` that passes `'indeterminate'`) uses `TriStateCheckbox`. Row checkboxes keep the Shadcn `Checkbox`. - ⛔ No edit under `ui/packages/components/src/ui/**` (ui/AGENTS.md #7 No-Touch zone). ### Measured repro / call sites (on `origin/main` at 86aaff3) - The Tasks → Bulk Actions view is an object grid; `ObjectGrid` builds a `type: 'data-table'` schema and renders it through `SchemaRenderer`, so the header checkbox is the components data-table's. - `git grep -n indeterminate` over `ui/packages/*/src` (non-test, excluding `src/ui/`): exactly one checkbox call site, the data-table header select-all (`somePageRowsSelected ? 'indeterminate' : false`). The other hits are progress bars and comments. - Mechanism confirmed in `ui/checkbox.tsx`: the Radix `Indicator` renders for `checked` and `indeterminate`, and its only child is `Check`. ## Tests New `ui/packages/components/src/renderers/complex/__tests__/data-table-selectAllMixed-27807.test.tsx`: - one of three rows selected → header `aria-checked="mixed"`, dash glyph present, check glyph absent; - control: all selected → `aria-checked="true"`, check present, dash absent; - control: none selected → `aria-checked="false"`, no glyph. ## Acceptance notes - Ablation (via `scripts/ablation-replace.mjs`, the header select-all's opening tag `TriStateCheckbox` swapped back to `Checkbox` in `data-table.tsx`, landed 1 → 0, restored to the HEAD blob with `git diff HEAD` empty): the partial-selection test went red, as did the all-selected control (the upstream check carries no `data-glyph` marker); the none-selected control stayed green. Tests 2 failed | 1 passed. - The wrapper mirrors the Shadcn checkbox classes by copy. If a future shadcn sync changes `ui/checkbox.tsx` styling, the header select-all will not follow automatically. ## Verification See the report comment on #27807 for the gate list (`dispatch-gates --commands` derived, each `command :: exit code`) and the test runs. --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CX72iRaaWgbDcBSUdZ)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0596115 commit fec60e0

4 files changed

Lines changed: 137 additions & 1 deletion

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@object-ui/components': patch
3+
---
4+
5+
fix(components): a grid's select-all draws a dash for a partial selection
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
With some but not all rows of the page selected, the data table's header checkbox was already announced as mixed (`aria-checked="mixed"`) but drew the same check mark as "all selected", so a user could read two selected rows as the whole page. It now draws a dash, filled like the checked state; all selected still draws the check and none selected draws nothing. The upstream Shadcn `Checkbox` is unchanged: the header uses a package-internal tri-state wrapper.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* TriStateCheckbox
11+
*
12+
* The Shadcn checkbox with a distinct mixed state: `checked="indeterminate"`
13+
* draws a dash, `true` draws the check, `false` draws nothing. Radix's
14+
* Indicator renders for both checked and indeterminate, and the upstream
15+
* `ui/checkbox.tsx` hard-codes a check mark inside it, so a partial selection
16+
* would read as "all selected".
17+
*
18+
* This lives in `custom/` to avoid modifying the Shadcn-synced
19+
* `ui/checkbox.tsx`; its classes mirror that file, plus the indeterminate fill.
20+
* Internal to this package: not re-exported from `custom/index.ts`.
21+
*/
22+
23+
import * as React from 'react';
24+
import * as CheckboxPrimitive from '@radix-ui/react-checkbox';
25+
import { Check, Minus } from 'lucide-react';
26+
import { cn } from '../lib/utils';
27+
28+
const TriStateCheckbox = React.forwardRef<
29+
React.ElementRef<typeof CheckboxPrimitive.Root>,
30+
React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root>
31+
>(({ className, checked, ...props }, ref) => (
32+
<CheckboxPrimitive.Root
33+
ref={ref}
34+
checked={checked}
35+
className={cn(
36+
'grid place-content-center peer h-4 w-4 shrink-0 rounded-sm border border-primary ring-offset-background focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground data-[state=indeterminate]:bg-primary data-[state=indeterminate]:text-primary-foreground',
37+
className,
38+
)}
39+
{...props}
40+
>
41+
<CheckboxPrimitive.Indicator className={cn('grid place-content-center text-current')}>
42+
{checked === 'indeterminate'
43+
? <Minus className="h-4 w-4" data-glyph="dash" />
44+
: <Check className="h-4 w-4" data-glyph="check" />}
45+
</CheckboxPrimitive.Indicator>
46+
</CheckboxPrimitive.Root>
47+
));
48+
TriStateCheckbox.displayName = 'TriStateCheckbox';
49+
50+
export { TriStateCheckbox };
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectstack#27807 — with some rows selected, the header's select-all is
11+
* announced as mixed AND drawn as a dash, never as the check mark that means
12+
* "all selected". Controls: all selected draws the check; none draws nothing.
13+
*/
14+
import { describe, it, expect } from 'vitest';
15+
import { render, fireEvent } from '@testing-library/react';
16+
import '@testing-library/jest-dom';
17+
import React from 'react';
18+
import { ComponentRegistry } from '@object-ui/core';
19+
import '../data-table';
20+
21+
function renderTable() {
22+
const DataTable = ComponentRegistry.get('data-table') as any;
23+
return render(
24+
<DataTable
25+
schema={{
26+
data: [
27+
{ id: 'a', name: 'Ada' },
28+
{ id: 'b', name: 'Grace' },
29+
{ id: 'c', name: 'Linus' },
30+
],
31+
pagination: false,
32+
searchable: false,
33+
selectable: true,
34+
columns: [{ header: 'Name', accessorKey: 'name' }],
35+
}}
36+
/>,
37+
);
38+
}
39+
40+
function selectAllOf(container: HTMLElement) {
41+
return container.querySelector('thead [data-slot="select-all"] [role="checkbox"]') as HTMLElement;
42+
}
43+
44+
function rowCheckboxes(container: HTMLElement) {
45+
return Array.from(container.querySelectorAll('tbody [data-slot="row-select"] [role="checkbox"]')) as HTMLElement[];
46+
}
47+
48+
describe('data-table select-all mixed state (objectstack#27807)', () => {
49+
it('a partial selection draws the dash, not the check, and stays aria-checked="mixed"', () => {
50+
const { container } = renderTable();
51+
const rows = rowCheckboxes(container);
52+
expect(rows).toHaveLength(3);
53+
fireEvent.click(rows[0]);
54+
const header = selectAllOf(container);
55+
expect(header).toHaveAttribute('aria-checked', 'mixed');
56+
expect(header.querySelector('[data-glyph="dash"]')).not.toBeNull();
57+
expect(header.querySelector('[data-glyph="check"]')).toBeNull();
58+
});
59+
60+
it('control: all rows selected draws the check', () => {
61+
const { container } = renderTable();
62+
for (const row of rowCheckboxes(container)) fireEvent.click(row);
63+
const header = selectAllOf(container);
64+
expect(header).toHaveAttribute('aria-checked', 'true');
65+
expect(header.querySelector('[data-glyph="check"]')).not.toBeNull();
66+
expect(header.querySelector('[data-glyph="dash"]')).toBeNull();
67+
});
68+
69+
it('control: no rows selected draws nothing', () => {
70+
const { container } = renderTable();
71+
const header = selectAllOf(container);
72+
expect(header).toHaveAttribute('aria-checked', 'false');
73+
expect(header.querySelector('svg')).toBeNull();
74+
});
75+
});

‎ui/packages/components/src/renderers/complex/data-table.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import { Button } from '../../ui/button';
3232
import { Input } from '../../ui/input';
3333
import { Checkbox } from '../../ui/checkbox';
34+
import { TriStateCheckbox } from '../../custom/tri-state-checkbox';
3435
import {
3536
Select,
3637
SelectContent,
@@ -3142,7 +3143,7 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
31423143
absolutely placed checkbox. */}
31433144
{selectionMode === 'multiple' && (
31443145
<div data-slot="select-all" className={cn("flex items-center", showRowNumbers && "justify-center")}>
3145-
<Checkbox
3146+
<TriStateCheckbox
31463147
checked={allPageRowsSelected ? true : somePageRowsSelected ? 'indeterminate' : false}
31473148
onCheckedChange={handleSelectAll}
31483149
aria-label={t('table.selectAllRows')}

0 commit comments

Comments
 (0)