Skip to content

Commit 679fc7c

Browse files
feat(layout): a collapsed sidebar nav group shows its reachable child count (#26103)
Fixes #25876 Clause-②: no Metadata-protocol: none ## What changes A collapsed sidebar navigation group now shows how many entries it holds, as a small count badge between its label and its chevron (`ui/packages/layout/src/NavigationRenderer.tsx`). Before, a closed group drew a label and a chevron and nothing else, so a closed group holding one entry looked exactly like one holding ten. - **What is counted.** The entries that opening the group would reveal: every leaf that passes `passesNavItemGuards` (the `visible` expression, `requiredPermissions`, the `requiresObject` / `requiresService` capability gates, a `doc` entry's readability check), counted through nested groups. Separators and the groups themselves are not entries, and an `action` entry counts only when the host wires `onAction`. - **One walk, not a copy.** The walk that `hasVisibleNavigationItems` ran is now a module-local generator, `visibleNavigationEntries`. `hasVisibleNavigationItems` takes its first entry (still stopping at the first hit) and the new module-local `countVisibleNavigationItems` counts them all. The count therefore cannot disagree with whether the group renders, or with what it shows once opened. No export is added or changed. - **When it shows.** Only while the group is collapsed (`!isOpen`). An open group's entries are in view. A group holding the current route is forced open, so it never shows a count. - **Accessibility.** The badge is the already-imported `Badge` with `aria-hidden="true"`, so the disclosure's accessible name stays the group label (`NavigationRenderer.groupDefaultOpen-13336.test.tsx` uses exact button names and passes unmodified). The disclosure already announces collapsed or expanded. - **Icon rail.** The badge carries `HIDDEN_ICON_LABEL_NO_POINTER` and `HIDDEN_ICON_LABEL_NOT_FOCUSABLE`, the same classes the group label carries. The chevron's `ml-auto` moves to the badge while it shows, so the badge and chevron sit together at the right edge. - No new i18n key. No navigation key or value changes. ## The card's second bullet is void The card also asked that "when the console replaces a report group's children with one index link, draw the group as that single entry". The seat's claim voided this bullet after checking `origin/main`: the console never folds a group's children. 「全部报表」 is a static sibling `SidebarGroup` at `UnifiedSidebar.tsx:595-619`. Flattening single-child groups would be a new product rule for every app, and the card does not ask for one. This PR adds only the count badge. No single-child flattening is implemented. ## Tests New: `ui/packages/layout/src/__tests__/NavigationRenderer.groupChildCount-25876.test.tsx`, 6 cases. - Red first against `main`'s renderer (commit 3e21743, the test alone): **4 failed, 2 passed**. The failures were the guarded count, the collapse/expand toggle, the nested group's own count, and the icon-mode classes. The two that passed are preservation pins: the accessible name is unchanged, and a group holding the route is forced open and shows no count. - The guarded count fixture holds seven nodes and four reachable entries. A `visible`-hidden entry, a permission-denied entry, a separator, an `action` with no dispatcher, a nested `visible`-hidden entry and the nested group itself are not counted. The two nested leaves are. - Ablation, one-off and not kept: `node scripts/ablation-replace.mjs` replaced `countVisibleNavigationItems(children, guardOptions)` with `countVisibleNavigationItems(children, { hasActionHandler: !!onAction })`, which drops the visibility and permission guards. The anchor went 1 → 0 and the blob went 71eca7a → c44ada02. Result: **3 failed / 3 passed**. The restore was verified as blob == HEAD (71eca7a) with `git diff HEAD` empty. The tests import `../NavigationRenderer` from `src`, so there is no `dist` leg. Run at head `279ce1d9` (after merging `origin/main`), under the verify lock: - `pnpm --filter @object-ui/layout test`: Test Files 46 passed (46), Tests 384 passed (384). - `pnpm --filter @object-ui/layout type-check`: exit 0. - `vitest run packages/app-shell/src/layout/__tests__/UnifiedSidebar` (from `ui/`): Test Files 16 passed (16), Tests 70 passed (70). This includes `UnifiedSidebar.iconModeLabelFocus-12795`. `NavigationRenderer.iconModeLabelHitTest-12744` is in the layout run above. - `node scripts/check-sdui-manifest.mjs`: exit 0. The record is untouched, as expected: `@object-ui/layout` is outside the registry closure. Gates, local subset, at head `279ce1d9`: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 commands. All 34 ran, and each exited 0 (`:: exit 0`, captured before any pipe). `dispatch-gates --ran` reported: "34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED (a DERIVED zero — all 34 recorded an exit code and none of them is 3)". ESLint on the three touched files found 0 errors; its 21 warnings were already there. Every other family is left to CI. ## Acceptance notes - **File-surface breach, declared.** One existing test outside the dispatched surface needed a one-line lookup change: `ui/packages/layout/src/__tests__/AppSchemaRenderer.mobileTabOrder-11395.test.tsx`, case "`expanded: false` … a collapsed group's children stay tabs". It found the sidebar group trigger by `el.textContent === 'Sales'`. A closed trigger now also contains the aria-hidden count, so its text is `Sales2`. The lookup now matches a closed trigger (`aria-expanded="false"`) whose first child, the label text node, reads `Sales`. The assertion still checks the same things: the trigger exists outside the bar, and the bar's tabs are `Deals`, `Leads`. Only the test changed. `AppSchemaRenderer.tsx` itself is untouched. The change is in its own commit (c809cd2, together with the changeset) and is easy to drop if the seat prefers another route. - The count badge uses `variant="secondary"`, so it reads as information, not as the `default` (primary) colour that an authored leaf `badge` takes by default. --- _Generated by [Claude Code](https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9c79640 commit 679fc7c

4 files changed

Lines changed: 251 additions & 6 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@object-ui/layout': patch
3+
---
4+
5+
feat(layout): a collapsed sidebar navigation group shows how many entries it holds
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **Before.** A closed navigation group drew its label and a chevron and nothing else, so a closed group holding one entry looked exactly like one holding ten, and users did not open it to find what was inside.
11+
- **Now.** While a group is collapsed, `NavigationRenderer` draws a small count badge between its label and its chevron. The count is what opening the group would reveal: the entries that pass the same per-item guards the renderer applies (the `visible` expression, `requiredPermissions`, the `requiresObject` / `requiresService` capability gates, a `doc` entry's readability check, and an `action` entry only when the host wires `onAction`), counted through nested groups. Separators and the groups themselves are not entries. It is the same walk that decides whether a group renders at all (`hasVisibleNavigationItems`), so the count cannot disagree with the opened group.
12+
- **When it does not show.** An open group shows no count, because its entries are in view. A group that holds the current route is forced open, so it never shows one. In the icon rail the badge is hidden under the same classes as the group label.
13+
- **Accessibility.** The badge is `aria-hidden`: the disclosure's accessible name stays the group label, and the disclosure already announces collapsed or expanded.
14+
15+
What to do: nothing. No navigation key or value changes, and no translation key is added.

‎ui/packages/layout/src/NavigationRenderer.tsx‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,35 @@ export function hasVisibleNavigationItems(
736736
items: NavigationItem[],
737737
options: NavigationVisibilityOptions = {},
738738
): boolean {
739+
return !visibleNavigationEntries(items, options).next().done;
740+
}
741+
742+
/**
743+
* How many entries a navigation tree would actually render under the given
744+
* guards — the count a collapsed group shows beside its label
745+
* (objectstack#25876), so a closed group holding one entry no longer looks
746+
* like one holding ten.
747+
*
748+
* The very walk {@link hasVisibleNavigationItems} stops at the first hit of:
749+
* separators and groups are not entries, a group contributes its surviving
750+
* descendants, and an `action` counts only with a dispatcher. Sharing the walk
751+
* keeps the count equal to what opening the group reveals.
752+
*/
753+
function countVisibleNavigationItems(
754+
items: NavigationItem[],
755+
options: NavigationVisibilityOptions,
756+
): number {
757+
const walk = visibleNavigationEntries(items, options);
758+
let count = 0;
759+
while (!walk.next().done) count += 1;
760+
return count;
761+
}
762+
763+
/** Every entry of `items` that would render, depth-first — the walk behind the two functions above. */
764+
function* visibleNavigationEntries(
765+
items: NavigationItem[],
766+
options: NavigationVisibilityOptions,
767+
): Generator<NavigationItem, void, undefined> {
739768
const { hasActionHandler = false } = options;
740769

741770
for (const item of items) {
@@ -744,14 +773,13 @@ export function hasVisibleNavigationItems(
744773

745774
if (item.type === 'separator') continue;
746775
if (item.type === 'group') {
747-
if (hasVisibleNavigationItems(item.children ?? [], options)) return true;
776+
yield* visibleNavigationEntries(item.children ?? [], options);
748777
continue;
749778
}
750779
if (item.type === 'action' && !hasActionHandler) continue;
751780

752-
return true;
781+
yield item;
753782
}
754-
return false;
755783
}
756784

757785
// ---------------------------------------------------------------------------
@@ -1599,8 +1627,27 @@ function NavigationItemRenderer({
15991627
<SidebarGroupLabel asChild className={cn(HIDDEN_ICON_LABEL_NO_POINTER, HIDDEN_ICON_LABEL_NOT_FOCUSABLE)}>
16001628
<CollapsibleTrigger className={cn('flex w-full items-center justify-between', isMobile && 'min-h-[44px] text-[15px] rounded-xl')}>
16011629
{groupLabel}
1630+
{/* objectstack#25876 — a collapsed group says how much it holds.
1631+
Open, its entries are in view and the count is noise; a group
1632+
holding the current route is forced open, so it never shows
1633+
one. `aria-hidden`: the disclosure's name stays the label, and
1634+
it already announces collapsed. In icon mode it hides under
1635+
the very classes the label does. */}
1636+
{!isOpen && (
1637+
<Badge
1638+
variant="secondary"
1639+
aria-hidden="true"
1640+
className={cn(
1641+
'ml-auto px-1.5 py-0 text-[10px] font-medium tabular-nums',
1642+
HIDDEN_ICON_LABEL_NO_POINTER,
1643+
HIDDEN_ICON_LABEL_NOT_FOCUSABLE,
1644+
)}
1645+
>
1646+
{countVisibleNavigationItems(children, guardOptions)}
1647+
</Badge>
1648+
)}
16021649
<ChevronRight
1603-
className={cn('ml-auto transition-transform', isMobile ? 'h-5 w-5' : 'h-4 w-4', isOpen && 'rotate-90')}
1650+
className={cn(isOpen ? 'ml-auto' : 'ml-1', 'transition-transform', isMobile ? 'h-5 w-5' : 'h-4 w-4', isOpen && 'rotate-90')}
16041651
/>
16051652
</CollapsibleTrigger>
16061653
</SidebarGroupLabel>

‎ui/packages/layout/src/__tests__/AppSchemaRenderer.mobileTabOrder-11395.test.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,12 @@ describe('objectui#11395 — the enumeration: inputs that are deliberately bar-s
288288
{ id: 'grp_sales', type: 'group', label: 'Sales', expanded: false, children: [entry('Deals'), entry('Leads')] },
289289
];
290290
const { tabs, sidebarRow, container, bar } = renderShell(navigation);
291-
// The sidebar holds them behind its closed disclosure, one tap away.
291+
// The sidebar holds them behind its closed disclosure, one tap away. A
292+
// closed disclosure also draws its aria-hidden child count after the label
293+
// (objectstack#25876), so the trigger is matched on its label text node.
292294
expect(sidebarRow('Deals')).toBeNull();
293295
const groupTrigger = Array.from(container.querySelectorAll('button')).find(
294-
(el) => !bar()?.contains(el) && el.textContent === 'Sales',
296+
(el) => !bar()?.contains(el) && el.getAttribute('aria-expanded') === 'false' && el.firstChild?.textContent === 'Sales',
295297
);
296298
expect(groupTrigger).toBeDefined();
297299
expect(tabs()).toEqual(['Deals', 'Leads']);
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
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#25876 — a collapsed navigation group shows how many entries it
11+
* holds.
12+
*
13+
* Before: a closed group drew its label and a chevron and nothing else, so a
14+
* closed group holding one entry looked exactly like one holding ten, and new
15+
* users never opened it. Now, while the group is collapsed, a count badge sits
16+
* beside the label.
17+
*
18+
* The count is what opening the group would reveal: the entries that pass the
19+
* same per-item guards the renderer applies (`visible`, `requiredPermissions`,
20+
* the capability gates, an `action` needing a dispatcher), recursing into
21+
* nested groups. Separators and the groups themselves are not entries. The
22+
* badge is `aria-hidden`, so the disclosure's accessible name stays the label
23+
* alone (the disclosure already announces collapsed / expanded).
24+
*/
25+
26+
import '@testing-library/jest-dom/vitest';
27+
import { afterEach, describe, expect, it } from 'vitest';
28+
import React from 'react';
29+
import { cleanup, render, screen, within } from '@testing-library/react';
30+
import userEvent from '@testing-library/user-event';
31+
import { MemoryRouter } from 'react-router-dom';
32+
import type { NavigationItem } from '@object-ui/types';
33+
import { Sidebar, SidebarContent, SidebarProvider } from '@object-ui/components';
34+
import { NavigationRenderer } from '../NavigationRenderer';
35+
36+
const BASE = '/apps/crm';
37+
38+
const entry = (id: string, extra: Partial<NavigationItem> = {}): NavigationItem =>
39+
({ id, type: 'object', objectName: id, label: id.toUpperCase(), ...extra }) as NavigationItem;
40+
41+
/**
42+
* Seven nodes, four of them reachable: two plain entries and the two inside
43+
* the nested group. Not counted: an entry hidden by its `visible` expression,
44+
* an entry whose permission is denied, a separator, an `action` with no
45+
* dispatcher, a nested entry hidden by `visible`, and the nested group itself.
46+
*/
47+
const children = (): NavigationItem[] => [
48+
entry('nav_campaigns'),
49+
entry('nav_leads'),
50+
entry('nav_hidden', { visible: 'hide' } as Partial<NavigationItem>),
51+
entry('nav_denied', { requiredPermissions: ['marketing.admin'] } as Partial<NavigationItem>),
52+
{ id: 'sep_1', type: 'separator' } as NavigationItem,
53+
{ id: 'act_import', type: 'action', label: 'Import', actionDef: { actionName: 'import' } } as unknown as NavigationItem,
54+
{
55+
id: 'grp_nested',
56+
type: 'group',
57+
label: 'Forecasts',
58+
children: [entry('nav_forecast'), entry('nav_quota'), entry('nav_nested_hidden', { visible: 'hide' } as Partial<NavigationItem>)],
59+
} as NavigationItem,
60+
];
61+
62+
const group = (id: string, label: string, extra: Partial<NavigationItem> = {}): NavigationItem =>
63+
({ id, type: 'group', label, children: children(), ...extra }) as NavigationItem;
64+
65+
const evaluateVisibility = (expr: unknown) => expr !== 'hide';
66+
const checkPermission = (perms: string[]) => !perms.includes('marketing.admin');
67+
68+
function renderNav(items: NavigationItem[], path = BASE) {
69+
return render(
70+
<MemoryRouter initialEntries={[path]}>
71+
<SidebarProvider defaultOpen>
72+
<NavigationRenderer
73+
items={items}
74+
basePath={BASE}
75+
evaluateVisibility={evaluateVisibility}
76+
checkPermission={checkPermission}
77+
/>
78+
</SidebarProvider>
79+
</MemoryRouter>,
80+
);
81+
}
82+
83+
const disclosure = (label: string) => screen.getByRole('button', { name: label });
84+
/** Any count drawn inside a disclosure: a run of digits as an element's whole text. */
85+
const countIn = (button: HTMLElement) => within(button).queryByText(/^\d+$/);
86+
87+
afterEach(cleanup);
88+
89+
describe('NavigationRenderer — a collapsed group shows its reachable child count (objectstack#25876)', () => {
90+
it('a collapsed group shows the guarded count: hidden, denied, separator, dispatcher-less action and the nested group itself are not counted; nested entries are', () => {
91+
renderNav([group('grp_marketing', 'Marketing', { expanded: false })]);
92+
const button = disclosure('Marketing');
93+
expect(button).toHaveAttribute('aria-expanded', 'false');
94+
95+
const count = countIn(button);
96+
expect(count, 'a collapsed group draws a count').not.toBeNull();
97+
expect(count).toHaveTextContent(/^4$/);
98+
expect(count).toHaveAttribute('aria-hidden', 'true');
99+
});
100+
101+
it('the count is the badge, not part of the disclosure\'s accessible name', () => {
102+
renderNav([group('grp_marketing', 'Marketing', { expanded: false })]);
103+
// Exact name: the label alone, the badge's digits are not read into it.
104+
expect(screen.getByRole('button', { name: 'Marketing' })).toBeInTheDocument();
105+
expect(screen.queryByRole('button', { name: /4/ })).not.toBeInTheDocument();
106+
});
107+
108+
it('an expanded group shows no count; collapsing it shows the count, expanding hides it again', async () => {
109+
const user = userEvent.setup();
110+
renderNav([group('grp_marketing', 'Marketing')]);
111+
const button = disclosure('Marketing');
112+
expect(button).toHaveAttribute('aria-expanded', 'true');
113+
expect(countIn(button)).toBeNull();
114+
115+
await user.click(button);
116+
expect(button).toHaveAttribute('aria-expanded', 'false');
117+
expect(countIn(button)).toHaveTextContent(/^4$/);
118+
119+
await user.click(button);
120+
expect(button).toHaveAttribute('aria-expanded', 'true');
121+
expect(countIn(button)).toBeNull();
122+
});
123+
124+
it('a nested group collapsed inside an open one shows its own count', async () => {
125+
const user = userEvent.setup();
126+
renderNav([group('grp_marketing', 'Marketing')]);
127+
const nested = disclosure('Forecasts');
128+
expect(countIn(nested)).toBeNull();
129+
await user.click(nested);
130+
expect(countIn(nested)).toHaveTextContent(/^2$/);
131+
});
132+
133+
it('a group declared closed that holds the current route is forced open, so it shows no count', () => {
134+
renderNav([group('grp_marketing', 'Marketing', { expanded: false })], `${BASE}/nav_leads`);
135+
const button = disclosure('Marketing');
136+
expect(button).toHaveAttribute('aria-expanded', 'true');
137+
expect(countIn(button)).toBeNull();
138+
});
139+
140+
it('collapsed to icons: the count carries the very icon-mode hiding classes the group label carries', () => {
141+
const { container } = render(
142+
<MemoryRouter initialEntries={[BASE]}>
143+
<SidebarProvider defaultOpen={false}>
144+
<Sidebar collapsible="icon">
145+
<SidebarContent>
146+
<NavigationRenderer
147+
items={[group('grp_marketing', 'Marketing', { expanded: false })]}
148+
basePath={BASE}
149+
evaluateVisibility={evaluateVisibility}
150+
checkPermission={checkPermission}
151+
/>
152+
</SidebarContent>
153+
</Sidebar>
154+
</SidebarProvider>
155+
</MemoryRouter>,
156+
);
157+
const peer = container.querySelector('.group.peer[data-state]');
158+
expect(peer).toHaveAttribute('data-state', 'collapsed');
159+
expect(peer).toHaveAttribute('data-collapsible', 'icon');
160+
161+
const label = disclosure('Marketing');
162+
const tokens = (el: Element) => el.className.split(/\s+/).filter(Boolean);
163+
// The tokens this file adds to the label to hide it in icon mode (pointer,
164+
// focus / accessibility tree, visibility transition), read off the label.
165+
// The primitive's own `-mt-8` / `opacity-0` move and fade the label's box,
166+
// which already carries the count; they are the primitive's, not the file's.
167+
const prefix = 'group-data-[state=collapsed]:group-data-[collapsible=icon]:';
168+
const hiding = tokens(label).filter(
169+
(t) => t.startsWith(prefix) && t !== `${prefix}-mt-8` && t !== `${prefix}opacity-0`,
170+
);
171+
expect(hiding).toHaveLength(3);
172+
expect(hiding).toEqual(expect.arrayContaining([
173+
expect.stringMatching(/:invisible$/),
174+
expect.stringMatching(/:pointer-events-none$/),
175+
]));
176+
177+
const count = countIn(label);
178+
expect(count).not.toBeNull();
179+
for (const t of hiding) expect(tokens(count!)).toContain(t);
180+
});
181+
});

0 commit comments

Comments
 (0)