From 56dc07b01e917ff923654bf124770d469485fac9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 04:59:20 +0000 Subject: [PATCH 1/9] fix(layout,app-shell): drag-to-reorder works within each level of a grouped sidebar (objectui#11626) The sortable path existed only in NavigationRenderer's group-free arm, so on every grouped menu (every stock app) `enableReorder` drew no grip. Each group's children, and each run of top-level entries between two groups, is now a sortable list of its own; nothing moves into or out of a group. The move is reported through the existing `onReorder(reorderedItems)`: the top-level list with the moved group's children reordered. The console's nav-order store keeps an order per group `id` beside `__root__`, writes only the level that moved, and applies every level on read. The group-free arm and its `__root__` record are unchanged. Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- .../app-shell/src/layout/UnifiedSidebar.tsx | 103 +++++-- packages/layout/src/NavigationRenderer.tsx | 251 +++++++++++++++--- ...tionRenderer.groupedReorder-11626.test.tsx | 227 ++++++++++++++++ 3 files changed, 534 insertions(+), 47 deletions(-) create mode 100644 packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index c792c5f273..06c761bca2 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -73,6 +73,64 @@ import { LocalizedSidebarTrigger } from './LocalizedSidebarTrigger.js'; // useNavOrder – localStorage-persisted drag-and-drop reorder for nav items // --------------------------------------------------------------------------- +/** + * The stored key of the top level's order. A group's order is stored beside it + * under the group's `id` (objectui#11626): the one key a group has that holds + * across reloads and locales (its label is translated). A spec-valid `id` is + * snake_case starting with a letter, so no group can be keyed `__root__`. + */ +const ROOT_ORDER_KEY = '__root__'; + +/** + * One level in its saved order: the saved ids first, in the saved order, then + * the entries the saved order does not name, in their own order. An id saved + * for an entry that is gone is skipped. + */ +function applyLevelOrder(items: NavigationItem[], saved: string[] | undefined): NavigationItem[] { + if (!saved) return items; + const byId = new Map(items.map(i => [i.id, i])); + const ordered: NavigationItem[] = []; + for (const id of saved) { + const item = byId.get(id); + if (item) { ordered.push(item); byId.delete(id); } + } + byId.forEach(item => ordered.push(item)); + return ordered; +} + +/** Every group's saved order applied to its children, at every depth. */ +function applyGroupOrders( + items: NavigationItem[], + orderMap: Record, +): NavigationItem[] { + let changed = false; + const next = items.map(item => { + if (item.type !== 'group' || !item.children?.length) return item; + const children = applyGroupOrders(applyLevelOrder(item.children, orderMap[item.id]), orderMap); + if (children === item.children) return item; + changed = true; + return { ...item, children }; + }); + return changed ? next : items; +} + +/** The ids of each group's children, in order, by group `id`, at every depth. */ +function groupChildIds( + items: NavigationItem[], + into: Map = new Map(), +): Map { + for (const item of items) { + if (item.type !== 'group') continue; + const children = item.children ?? []; + into.set(item.id, children.map(c => c.id)); + groupChildIds(children, into); + } + return into; +} + +const sameIds = (a: string[], b: string[]) => + a.length === b.length && a.every((id, i) => id === b[i]); + function useNavOrder(appName: string) { const storageKey = `objectui-nav-order-${appName}`; @@ -103,25 +161,34 @@ function useNavOrder(appName: string) { ); const applyOrder = React.useCallback( - (items: NavigationItem[]): NavigationItem[] => { - const saved = orderMap['__root__']; - if (!saved) return items; - const byId = new Map(items.map(i => [i.id, i])); - const ordered: NavigationItem[] = []; - for (const id of saved) { - const item = byId.get(id); - if (item) { ordered.push(item); byId.delete(id); } - } - byId.forEach(item => ordered.push(item)); - return ordered; - }, + (items: NavigationItem[]): NavigationItem[] => + applyGroupOrders(applyLevelOrder(items, orderMap[ROOT_ORDER_KEY]), orderMap), [orderMap], ); + /** + * `drawn` is the tree this sidebar handed the renderer. A move within a group + * comes back as the top-level list with that group's children reordered + * (objectui#11626), so the group whose children now differ from the drawn + * ones is the group that moved, and only its order is stored: the top level + * and every other group keep following the app until the user moves them. + * A report in which no group's children moved is a move among top-level + * entries, stored under `__root__` exactly as it was before groups had one. + */ const handleReorder = React.useCallback( - (reorderedItems: NavigationItem[]) => { + (reorderedItems: NavigationItem[], drawn: NavigationItem[]) => { + const before = groupChildIds(drawn); + const movedGroups: Record = {}; + groupChildIds(reorderedItems).forEach((ids, groupId) => { + const was = before.get(groupId); + if (was && !sameIds(was, ids)) movedGroups[groupId] = ids; + }); + if (Object.keys(movedGroups).length > 0) { + persist({ ...orderMap, ...movedGroups }); + return; + } const ids = reorderedItems.map(i => i.id); - persist({ ...orderMap, __root__: ids }); + persist({ ...orderMap, [ROOT_ORDER_KEY]: ids }); }, [orderMap, persist], ); @@ -456,6 +523,12 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { const ordered = applyOrder(studioNavigationItems); return applyPins(ordered); }, [studioNavigationItems, applyOrder, applyPins]); + // The drawn tree goes along with a report, so the store can tell which + // level moved (objectui#11626). + const handleNavReorder = React.useCallback( + (reorderedItems: NavigationItem[]) => handleReorder(reorderedItems, processedNavigation), + [handleReorder, processedNavigation], + ); // Recent section collapsed by default const [recentExpanded, setRecentExpanded] = React.useState(false); @@ -581,7 +654,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { enablePinning={!isMobile} onPinToggle={togglePin} enableReorder={!isMobile} - onReorder={handleReorder} + onReorder={handleNavReorder} resolveTargetLabel={resolveNavTargetLabel} locale={language} onAction={dispatchNavAction} diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index bc85fdaae6..f6f6841b6d 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -220,10 +220,26 @@ export interface NavigationRendererProps { basePath?: string, ) => void; - /** Enable drag-to-reorder for navigation items */ + /** + * Enable drag-to-reorder for navigation items. + * + * An entry moves within its own level only: among the top-level entries of a + * menu with no groups, among one group's children, or, in a grouped menu, + * among a run of top-level entries between two groups. It never moves into or + * out of a group (objectui#11626): which group an entry sits in is the app's + * structure, not a personal order. While `searchQuery` narrows a grouped + * menu, the menu offers no grip, because a narrowed group shows only some of + * its children. + */ enableReorder?: boolean; - /** Called when navigation items are reordered via drag */ + /** + * Called when navigation items are reordered via drag, always with the + * top-level list. After a move among top-level entries, that list is + * reordered. After a move within a group, the top-level list is as drawn and + * that group's `children` are reordered (objectui#11626). The moved level's + * entries carry their new positions as `order` (0, 1, 2, …). + */ onReorder?: (reorderedItems: NavigationItem[]) => void; // RETIRED (objectui#11299): `resolveObjectLabel` / `resolveDashboardLabel` / @@ -1108,6 +1124,121 @@ export function filterNavigationItems( /** Minimum drag distance in pixels to activate reorder */ const DRAG_ACTIVATION_DISTANCE = 5; +// --------------------------------------------------------------------------- +// Within-level reorder for a grouped menu (objectui#11626) +// --------------------------------------------------------------------------- + +/** + * One level moved: the entry `activeId` taken out and put where `overId` was, + * every entry of the level carrying its new position as `order`, which is how + * the group-free arm reports a move too. `null` when either id is not in + * `level`. + * + * `level` is the WHOLE level as the renderer orders it, gated-away entries + * included, so the entries a user cannot see keep their places relative to the + * ones the user moved, and the reported level loses none of them. + */ +function moveWithinLevel( + level: NavigationItem[], + activeId: string, + overId: string, +): NavigationItem[] | null { + const oldIndex = level.findIndex((i) => i.id === activeId); + const newIndex = level.findIndex((i) => i.id === overId); + if (oldIndex === -1 || newIndex === -1) return null; + return arrayMove(level, oldIndex, newIndex).map((item, idx) => ({ ...item, order: idx })); +} + +/** `items` with the children of the group `groupId` replaced, at any depth. */ +function withGroupChildren( + items: NavigationItem[], + groupId: string, + children: NavigationItem[], +): NavigationItem[] { + return items.map((item) => { + if (item.type !== 'group') return item; + if (item.id === groupId) return { ...item, children }; + if (!item.children?.length) return item; + return { ...item, children: withGroupChildren(item.children, groupId, children) }; + }); +} + +/** + * Reports a move within the group `groupId`: its children, already moved by + * {@link moveWithinLevel}. Provided by the grouped arm of + * {@link NavigationRenderer}. `null` means the menu offers no grip on a group's + * children: reorder is off, the menu has no groups, or a search narrows it. + */ +type GroupChildrenReorder = (groupId: string, reorderedChildren: NavigationItem[]) => void; +const GroupReorderContext = React.createContext(null); + +/** + * Whether `item` draws anything: the decisions `NavigationItemRenderer` takes + * before it returns `null`, asked through the same shared guard statement and + * predicate. A sortable wrapper is put only around an entry that draws, so a + * gated-away entry does not leave an empty, focusable drag wrapper behind. + */ +function drawsNavItem(item: NavigationItem, options: NavigationVisibilityOptions): boolean { + if (item.type === 'separator') return passesNavItemGuards(item, options); + return hasVisibleNavigationItems([item], options); +} + +/** The props every row renderer takes besides its `item`. */ +interface NavRowProps { + basePath: string; + evalVis: VisibilityEvaluator; + checkPerm: PermissionChecker; + checkCap: CapabilityChecker; + checkDocTarget?: DocTargetChecker; + onAction?: (item: NavigationItem) => void; + enablePinning?: boolean; + onPinToggle?: (itemId: string, pinned: boolean, item?: NavigationItem, basePath?: string) => void; + resolveTargetLabel?: NavTargetLabelResolver; + locale?: string; + t?: (key: string, options?: any) => string; + templateContext?: NavTemplateContext; +} + +/** + * One level of a grouped menu as a sortable list: its own `DndContext`, so a + * drag starts, moves and drops within this list only and no other list is a + * drop target. The rows are the group-free arm's `SortableNavigationItem`. + */ +function SortableNavigationList({ + contextId, + items, + onMove, + rowProps, +}: { + contextId: string; + items: NavigationItem[]; + onMove: (activeId: string, overId: string) => void; + rowProps: NavRowProps; +}) { + const sensors = useSensors( + useSensor(PointerSensor, { activationConstraint: { distance: DRAG_ACTIVATION_DISTANCE } }), + useSensor(KeyboardSensor), + ); + + const handleDragEnd = (event: DragEndEvent) => { + const { active, over } = event; + if (!over || active.id === over.id) return; + onMove(String(active.id), String(over.id)); + }; + + return ( + + i.id)} strategy={verticalListSortingStrategy}> + + {items.map((item) => ( + + ))} + + + + ); +} + // --------------------------------------------------------------------------- // SortableNavigationItem (drag-reorder wrapper) // --------------------------------------------------------------------------- @@ -1256,6 +1387,7 @@ function NavigationItemRenderer({ ? true : (explicitOpen ?? (childCount >= AUTO_COLLAPSE_THRESHOLD ? false : true)); const [isOpen, setIsOpen] = useState(initialOpen); + const reorderGroup = React.useContext(GroupReorderContext); // --- Per-item guards: `visible`, `requiredPermissions`, and the // runtime-capability gates (an entry whose required object/service is not @@ -1293,6 +1425,24 @@ function NavigationItemRenderer({ const groupLabel = resolveNavItemLabel(item, tProp, resolveTargetLabel, locale); + // objectui#11626: with reorder on, this group's children are one sortable + // list of their own. The move is taken over ALL of `children` (gated-away + // entries keep their places) and reported up as this group's new children. + const rowProps: NavRowProps = { + basePath, + evalVis, + checkPerm, + checkCap, + checkDocTarget, + onAction, + enablePinning, + onPinToggle, + resolveTargetLabel, + locale, + t: tProp, + templateContext, + }; + return ( @@ -1306,26 +1456,27 @@ function NavigationItemRenderer({ - - {children.map((child) => ( - - ))} - + {reorderGroup ? ( + drawsNavItem(child, guardOptions))} + onMove={(activeId, overId) => { + const moved = moveWithinLevel(children, activeId, overId); + if (moved) reorderGroup(item.id, moved); + }} + rowProps={rowProps} + /> + ) : ( + + {children.map((child) => ( + + ))} + + )} @@ -1657,6 +1808,31 @@ export function NavigationRenderer({ const fragments: React.ReactNode[] = []; let leafBuffer: NavigationItem[] = []; + // --- Grouped drag-reorder (objectui#11626) --- each group's children, and + // each run of top-level entries between two groups, is a sortable list of + // its own; nothing moves into or out of a group. Off while a search narrows + // the tree: a narrowed group shows only some of its children, and an order + // taken among some of them is not the group's order. + const groupedReorder = !!enableReorder && !searchQuery?.trim(); + const reorderGroup: GroupChildrenReorder | null = groupedReorder + ? (groupId, reorderedChildren) => { + if (!onReorder) return; + onReorder(withGroupChildren(sorted, groupId, reorderedChildren)); + } + : null; + const moveTopLevel = (activeId: string, overId: string) => { + if (!onReorder) return; + const moved = moveWithinLevel(sorted, activeId, overId); + if (moved) onReorder(moved); + }; + const itemGuards: NavigationVisibilityOptions = { + evaluateVisibility: evalVis, + checkPermission: checkPerm, + checkCapability: checkCap, + checkDocTarget, + hasActionHandler: !!onAction, + }; + const flushLeaves = (key: string) => { if (leafBuffer.length === 0) return; const leaves = leafBuffer; @@ -1664,15 +1840,24 @@ export function NavigationRenderer({ fragments.push( - - {leaves.map((item) => ( - - ))} - + {groupedReorder ? ( + drawsNavItem(item, itemGuards))} + onMove={moveTopLevel} + rowProps={itemProps} + /> + ) : ( + + {leaves.map((item) => ( + + ))} + + )} , ); @@ -1698,7 +1883,9 @@ export function NavigationRenderer({ return ( {favoritesSection} - {fragments} + + {fragments} + ); } diff --git a/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx b/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx new file mode 100644 index 0000000000..753f18e33f --- /dev/null +++ b/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx @@ -0,0 +1,227 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#11626 — drag-to-reorder works on a GROUPED menu, within a level. + * + * Before: `NavigationRenderer` mounted its sortable path only when the menu + * had no groups, so on every stock app (all grouped) `enableReorder` drew no + * grip at all. Triage's direction (`5986793209`): each group's children get the + * same sortable path, scoped to that group, and the move is reported through + * the existing `onReorder(reorderedItems)` with no signature change. A move + * into another group is out of scope: which group an entry sits in is the + * app's structure, not a personal order. + * + * ── The instrument ──────────────────────────────────────────────────────── + * The one `plugin-kanban` uses (`sameColumnDropIsNotClaimed-8826.test.tsx`): + * a module mock of `@dnd-kit/core` that renders the REAL `DndContext` and + * records the `onDragEnd` it was handed, keyed by the context's `id` (the + * group-free arm's context has none). dnd-kit's sensors need layout that + * happy-dom does not have, so a synthesized drop on the production handler is + * the closest honest reproduction; everything after that call is the real + * path. The live drag in a browser is measured on the pull request. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, act } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationItem } from '@object-ui/types'; +import { SidebarProvider } from '@object-ui/components'; +import { NavigationRenderer } from '../NavigationRenderer'; + +type DragEnd = (event: { active: { id: string }; over: { id: string } | null }) => void; + +// `vi.hoisted` so the hoisted mock factory can reach the box. +const dnd = vi.hoisted(() => ({ byContext: new Map() })); + +vi.mock('@dnd-kit/core', async (importOriginal) => { + const actual = await importOriginal(); + const ReactMod = await import('react'); + const CapturingDndContext = (props: Record) => { + dnd.byContext.set(String(props.id ?? '(no id)'), props.onDragEnd as DragEnd); + return ReactMod.createElement(actual.DndContext, props as never); + }; + return { ...actual, DndContext: CapturingDndContext }; +}); + +const BASE = '/apps/crm'; +const GRIP = '[aria-label="Drag to reorder"]'; + +const leaf = (id: string, extra: Partial = {}): NavigationItem => + ({ id, type: 'object', objectName: id, label: id.toUpperCase(), ...extra }) as NavigationItem; + +/** Two top-level entries, then two groups — the shape of the stock showcase. */ +const GROUPED: NavigationItem[] = [ + leaf('t1'), + leaf('t2'), + { id: 'grp_a', type: 'group', label: 'Group A', children: [leaf('a1'), leaf('a2'), leaf('a3')] }, + { id: 'grp_b', type: 'group', label: 'Group B', children: [leaf('b1'), leaf('b2')] }, +]; + +function renderNav(items: NavigationItem[], props: Record = {}) { + return render( + + + + + , + ); +} + +function drop(contextId: string, activeId: string, overId: string) { + const onDragEnd = dnd.byContext.get(contextId); + expect(onDragEnd, `a DndContext with id ${contextId} was mounted`).toBeTypeOf('function'); + act(() => onDragEnd!({ active: { id: activeId }, over: { id: overId } })); +} + +const ids = (items: NavigationItem[] | undefined) => (items ?? []).map((i) => i.id); +const orders = (items: NavigationItem[] | undefined) => (items ?? []).map((i) => i.order); +const childrenOf = (items: NavigationItem[], groupId: string) => + items.find((i) => i.id === groupId)?.children; + +beforeEach(() => { + dnd.byContext.clear(); +}); + +describe('objectui#11626 — a grouped menu offers drag-to-reorder within each level', () => { + it('draws a grip on every entry of a grouped menu, and none with reorder off', () => { + const on = renderNav(GROUPED, { enableReorder: true, onReorder: vi.fn() }); + // t1, t2, a1–a3, b1–b2: seven entries, seven grips; a group header has none. + expect(on.container.querySelectorAll(GRIP)).toHaveLength(7); + on.unmount(); + + const off = renderNav(GROUPED, { onReorder: vi.fn() }); + expect(off.container.querySelectorAll(GRIP)).toHaveLength(0); + }); + + it('each group, and each run of top-level entries, is a sortable list of its own', () => { + renderNav(GROUPED, { enableReorder: true, onReorder: vi.fn() }); + expect([...dnd.byContext.keys()].sort()).toEqual([ + 'nav-reorder-group-grp_a', + 'nav-reorder-group-grp_b', + 'nav-reorder-top-t1', + ]); + }); + + it('a move within a group reports the top-level list with that group’s children reordered', () => { + const onReorder = vi.fn(); + renderNav(GROUPED, { enableReorder: true, onReorder }); + + drop('nav-reorder-group-grp_a', 'a3', 'a1'); + + expect(onReorder).toHaveBeenCalledTimes(1); + const reported: NavigationItem[] = onReorder.mock.calls[0][0]; + // The top level is reported whole and as drawn. + expect(ids(reported)).toEqual(['t1', 't2', 'grp_a', 'grp_b']); + // The moved group: new order, positions carried as `order`. + expect(ids(childrenOf(reported, 'grp_a'))).toEqual(['a3', 'a1', 'a2']); + expect(orders(childrenOf(reported, 'grp_a'))).toEqual([0, 1, 2]); + // The other group and the top level are not restamped. + expect(ids(childrenOf(reported, 'grp_b'))).toEqual(['b1', 'b2']); + expect(orders(childrenOf(reported, 'grp_b'))).toEqual([undefined, undefined]); + expect(orders(reported)).toEqual([undefined, undefined, undefined, undefined]); + }); + + it('an entry of another group is no drop target: a cross-group drop reports nothing', () => { + const onReorder = vi.fn(); + renderNav(GROUPED, { enableReorder: true, onReorder }); + + drop('nav-reorder-group-grp_a', 'a1', 'b1'); + drop('nav-reorder-group-grp_b', 'b2', 't1'); + drop('nav-reorder-top-t1', 't2', 'a1'); + + expect(onReorder).not.toHaveBeenCalled(); + // Lit control: the same context does report a drop within its own group. + drop('nav-reorder-group-grp_a', 'a2', 'a1'); + expect(onReorder).toHaveBeenCalledTimes(1); + }); + + it('a move among the top-level entries of a grouped menu reorders the top level, groups in place', () => { + const onReorder = vi.fn(); + renderNav(GROUPED, { enableReorder: true, onReorder }); + + drop('nav-reorder-top-t1', 't2', 't1'); + + const reported: NavigationItem[] = onReorder.mock.calls[0][0]; + expect(ids(reported)).toEqual(['t2', 't1', 'grp_a', 'grp_b']); + expect(orders(reported)).toEqual([0, 1, 2, 3]); + expect(ids(childrenOf(reported, 'grp_a'))).toEqual(['a1', 'a2', 'a3']); + }); + + it('a gated-away child gets no drag wrapper, keeps its place, and is not dropped from the report', () => { + const onReorder = vi.fn(); + const items: NavigationItem[] = [ + { + id: 'grp_a', + type: 'group', + label: 'Group A', + children: [leaf('a1'), leaf('hidden', { visible: false } as Partial), leaf('a2')], + }, + ]; + const { container } = renderNav(items, { + enableReorder: true, + onReorder, + evaluateVisibility: (expr: unknown) => expr !== false, + }); + expect(container.querySelectorAll('[aria-roledescription="sortable"]')).toHaveLength(2); + + drop('nav-reorder-group-grp_a', 'a2', 'a1'); + + const reported: NavigationItem[] = onReorder.mock.calls[0][0]; + expect(ids(childrenOf(reported, 'grp_a'))).toEqual(['a2', 'a1', 'hidden']); + }); + + it('a nested group is a level of its own; its move is reported at its depth', () => { + const onReorder = vi.fn(); + const items: NavigationItem[] = [ + { + id: 'grp_outer', + type: 'group', + label: 'Outer', + children: [ + leaf('o1'), + { id: 'grp_inner', type: 'group', label: 'Inner', children: [leaf('i1'), leaf('i2')] }, + ], + }, + ]; + renderNav(items, { enableReorder: true, onReorder }); + + drop('nav-reorder-group-grp_inner', 'i2', 'i1'); + + const reported: NavigationItem[] = onReorder.mock.calls[0][0]; + const outer = childrenOf(reported, 'grp_outer'); + expect(ids(outer)).toEqual(['o1', 'grp_inner']); + expect(ids(childrenOf(outer!, 'grp_inner'))).toEqual(['i2', 'i1']); + }); + + it('while a search narrows a grouped menu, it offers no grip', () => { + const { container } = renderNav(GROUPED, { enableReorder: true, onReorder: vi.fn(), searchQuery: 'a' }); + // Control: the search matched entries, so rows are drawn. + expect(container.querySelectorAll('a[href]').length).toBeGreaterThan(0); + expect(container.querySelectorAll(GRIP)).toHaveLength(0); + expect(dnd.byContext.size).toBe(0); + }); +}); + +describe('objectui#11626 — the group-free menu is unchanged', () => { + const FLAT: NavigationItem[] = [leaf('f1'), leaf('f2'), leaf('f3')]; + + it('keeps its one context and reports the whole level moved, positions as `order`', () => { + const onReorder = vi.fn(); + const { container } = renderNav(FLAT, { enableReorder: true, onReorder }); + expect(container.querySelectorAll(GRIP)).toHaveLength(3); + expect([...dnd.byContext.keys()]).toEqual(['(no id)']); + + drop('(no id)', 'f3', 'f1'); + + const reported: NavigationItem[] = onReorder.mock.calls[0][0]; + expect(ids(reported)).toEqual(['f3', 'f1', 'f2']); + expect(orders(reported)).toEqual([0, 1, 2]); + }); +}); From 04660ddad93fcbddbacbba606da63fb5d48d14a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 05:00:34 +0000 Subject: [PATCH 2/9] test(app-shell): pin the per-group nav order store and the unchanged `__root__` record (objectui#11626) Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- ...fiedSidebar.groupedNavOrder-11626.test.tsx | 285 ++++++++++++++++++ 1 file changed, 285 insertions(+) create mode 100644 packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx new file mode 100644 index 0000000000..d16556ed3b --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx @@ -0,0 +1,285 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * UnifiedSidebar — a personal order made within a group of a grouped app + * menu is stored under that group's `id` and restored on the next load + * (objectui#11626). + * + * The store (`useNavOrder`, localStorage `objectui-nav-order-`) kept one + * order, `__root__`, and the renderer drew no grip on a grouped menu, so on + * every stock app the shell's `enableReorder` did nothing. Triage's direction + * (`5986793209`): an order per group key beside `__root__`; a group's key is its + * `id`, which holds across reloads and locales where its label does not. + * + * This pins the WIRING in the real sidebar: the real `NavigationRenderer` + * reports a move through `onReorder`, the real store writes it, and a fresh + * mount reads it back. The group-free app's `__root__` record is pinned + * byte-for-byte, because it must not change. + * + * Instrument: a module mock of `@dnd-kit/core` that renders the REAL + * `DndContext` and records its `onDragEnd` by context `id` (the one + * `NavigationRenderer.groupedReorder-11626.test.tsx` uses, after + * `plugin-kanban`'s `sameColumnDropIsNotClaimed-8826.test.tsx`). + * Harness: the provider / chrome mocks of + * `UnifiedSidebar.navLabelInheritsTarget-9868.test.tsx`; `@object-ui/layout` + * and `@object-ui/components` stay REAL. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen, act, within } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationItem } from '@object-ui/types'; + +type DragEnd = (event: { active: { id: string }; over: { id: string } | null }) => void; + +const dnd = vi.hoisted(() => ({ byContext: new Map() })); +// The nav ids the (mocked) pin store answers as pinned. +const pins = vi.hoisted(() => ({ ids: new Set() })); + +vi.mock('@dnd-kit/core', async (importOriginal) => { + const actual = await importOriginal(); + const ReactMod = await import('react'); + const CapturingDndContext = (props: Record) => { + dnd.byContext.set(String(props.id ?? '(no id)'), props.onDragEnd as DragEnd); + return ReactMod.createElement(actual.DndContext, props as never); + }; + return { ...actual, DndContext: CapturingDndContext }; +}); + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + language: 'en', + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + appLabel: ({ label }: { label?: string }) => label, + }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: null, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), +})); + +vi.mock('@object-ui/permissions', async (importOriginal) => ({ + ...(await importOriginal()), + usePermissions: () => ({ can: () => true, hasCapabilities: () => true }), +})); + +let metadataState: { apps: unknown[]; objects: unknown[] }; +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => metadataState, +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', +})); + +vi.mock('../../utils', () => ({ + resolveKeyedI18nLabel: (label: unknown) => (typeof label === 'string' ? label : ''), + matchAppBySegment: (apps: Array<{ name?: string }>, segment?: string) => + apps.find((a) => a?.name === segment), + appRouteSegment: (app: { name?: string }) => app?.name, +})); + +vi.mock('../../utils/getIcon', () => ({ getIcon: () => () => null })); +vi.mock('../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) })); +vi.mock('../../hooks/useFavorites', () => ({ + useFavorites: () => ({ favorites: [], removeFavorite: vi.fn() }), +})); +vi.mock('../../hooks/useNavPins', () => ({ + useNavPins: () => ({ + togglePin: vi.fn(), + applyPins: (items: NavigationItem[]) => { + const walk = (list: NavigationItem[]): NavigationItem[] => + list.map((item) => ({ + ...item, + ...(item.type !== 'separator' && pins.ids.has(item.id) ? { pinned: true } : {}), + ...(item.children ? { children: walk(item.children) } : {}), + }) as NavigationItem); + return walk(items); + }, + }), +})); +vi.mock('../../hooks/useNavActionDispatch', () => ({ + useNavActionDispatch: () => vi.fn(), +})); +vi.mock('../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ context: 'app', currentAppName: 'crm' }), +})); +vi.mock('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), + contextSelectorQueryKey: (id: string) => (id === 'active_package' ? 'package' : id), + STUDIO_PACKAGE_SELECTOR_ID: 'active_package', +})); +vi.mock('../LocalizedSidebarTrigger', () => ({ + LocalizedSidebarTrigger: () => null, +})); + +import { SidebarProvider } from '@object-ui/components'; +import { UnifiedSidebar } from '../UnifiedSidebar'; + +const STORE = 'objectui-nav-order-crm'; + +const entry = (id: string, label: string, extra: Partial = {}): NavigationItem => + ({ id, type: 'object', objectName: id, label, ...extra }) as NavigationItem; + +/** The stock showcase's shape: top-level entries, then groups keyed by `id`. */ +const GROUPED: NavigationItem[] = [ + entry('nav_map', 'Map'), + entry('nav_start', 'Start'), + { + id: 'grp_sales', + type: 'group', + // A translated label: the store must not key on it. + label: 'Sales', + children: [entry('nav_accounts', 'Accounts'), entry('nav_contacts', 'Contacts'), entry('nav_leads', 'Leads')], + }, + { + id: 'grp_ops', + type: 'group', + label: 'Operations', + children: [entry('nav_tasks', 'Tasks'), entry('nav_projects', 'Projects')], + }, +]; + +function sidebarUi(navigation: NavigationItem[]) { + metadataState = { + apps: [{ name: 'crm', label: 'CRM', active: true, navigation }], + objects: [], + }; + return ( + + + + + + ); +} + +function drop(contextId: string, activeId: string, overId: string) { + const onDragEnd = dnd.byContext.get(contextId); + expect(onDragEnd, `a DndContext with id ${contextId} was mounted`).toBeTypeOf('function'); + act(() => onDragEnd!({ active: { id: activeId }, over: { id: overId } })); +} + +const stored = () => JSON.parse(localStorage.getItem(STORE) ?? 'null'); +/** The link texts in document order, limited to the given names. */ +const linkOrder = (names: string[]) => + screen + .getAllByRole('link') + .map((a) => a.textContent ?? '') + .filter((text) => names.includes(text)); + +beforeEach(() => { + localStorage.clear(); + dnd.byContext.clear(); + pins.ids = new Set(); +}); + +describe('UnifiedSidebar — a grouped app menu keeps a personal order per group (objectui#11626)', () => { + it('the shell draws a grip on the grouped menu', () => { + const { container } = render(sidebarUi(GROUPED)); + expect(container.querySelectorAll('[aria-label="Drag to reorder"]')).toHaveLength(7); + }); + + it('a move within one group stores that group’s order under its id, and nothing else', () => { + render(sidebarUi(GROUPED)); + + drop('nav-reorder-group-grp_sales', 'nav_leads', 'nav_accounts'); + + expect(stored()).toEqual({ grp_sales: ['nav_leads', 'nav_accounts', 'nav_contacts'] }); + // Drawn in the new order at once. + expect(linkOrder(['Accounts', 'Contacts', 'Leads'])).toEqual(['Leads', 'Accounts', 'Contacts']); + }); + + it('the order is restored after a reload, and the rest of the menu follows the app', () => { + const first = render(sidebarUi(GROUPED)); + drop('nav-reorder-group-grp_sales', 'nav_leads', 'nav_accounts'); + first.unmount(); + + render(sidebarUi(GROUPED)); + expect(linkOrder(['Accounts', 'Contacts', 'Leads'])).toEqual(['Leads', 'Accounts', 'Contacts']); + expect(linkOrder(['Map', 'Start'])).toEqual(['Map', 'Start']); + expect(linkOrder(['Tasks', 'Projects'])).toEqual(['Tasks', 'Projects']); + }); + + it('a second group’s move is stored beside the first; a top-level move beside both', () => { + render(sidebarUi(GROUPED)); + + drop('nav-reorder-group-grp_sales', 'nav_leads', 'nav_accounts'); + drop('nav-reorder-group-grp_ops', 'nav_projects', 'nav_tasks'); + expect(stored()).toEqual({ + grp_sales: ['nav_leads', 'nav_accounts', 'nav_contacts'], + grp_ops: ['nav_projects', 'nav_tasks'], + }); + + drop('nav-reorder-top-nav_map', 'nav_start', 'nav_map'); + expect(stored()).toEqual({ + grp_sales: ['nav_leads', 'nav_accounts', 'nav_contacts'], + grp_ops: ['nav_projects', 'nav_tasks'], + __root__: ['nav_start', 'nav_map', 'grp_sales', 'grp_ops'], + }); + expect(linkOrder(['Map', 'Start'])).toEqual(['Start', 'Map']); + expect(linkOrder(['Tasks', 'Projects'])).toEqual(['Projects', 'Tasks']); + }); + + it('a pinned entry and a gated-away entry keep working through a move', () => { + pins.ids = new Set(['nav_contacts']); + const navigation: NavigationItem[] = [ + { + id: 'grp_sales', + type: 'group', + label: 'Sales', + children: [ + entry('nav_accounts', 'Accounts'), + entry('nav_hidden', 'Hidden', { visible: false } as Partial), + entry('nav_contacts', 'Contacts'), + ], + }, + ]; + const first = render(sidebarUi(navigation)); + const favorites = () => within(screen.getByText('Favorites').closest('[data-sidebar="group"]') as HTMLElement); + expect(favorites().getByRole('link', { name: 'Contacts' })).toBeInTheDocument(); + + drop('nav-reorder-group-grp_sales', 'nav_contacts', 'nav_accounts'); + + // The gated-away id keeps its place in the stored order; it stays hidden. + expect(stored()).toEqual({ grp_sales: ['nav_contacts', 'nav_accounts', 'nav_hidden'] }); + expect(screen.queryByRole('link', { name: 'Hidden' })).not.toBeInTheDocument(); + first.unmount(); + + render(sidebarUi(navigation)); + expect(favorites().getByRole('link', { name: 'Contacts' })).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: 'Hidden' })).not.toBeInTheDocument(); + }); +}); + +describe('UnifiedSidebar — a group-free app’s `__root__` record is unchanged (objectui#11626)', () => { + const FLAT: NavigationItem[] = [entry('nav_a', 'Alpha'), entry('nav_b', 'Beta'), entry('nav_c', 'Gamma')]; + + it('a move stores `__root__` byte-for-byte as before, and a reload applies it', () => { + const first = render(sidebarUi(FLAT)); + drop('(no id)', 'nav_c', 'nav_a'); + + expect(localStorage.getItem(STORE)).toBe('{"__root__":["nav_c","nav_a","nav_b"]}'); + first.unmount(); + + render(sidebarUi(FLAT)); + expect(linkOrder(['Alpha', 'Beta', 'Gamma'])).toEqual(['Gamma', 'Alpha', 'Beta']); + }); + + it('a stored `__root__` record written before this change still reads back', () => { + localStorage.setItem(STORE, '{"__root__":["nav_b","nav_c","nav_a"]}'); + render(sidebarUi(FLAT)); + expect(linkOrder(['Alpha', 'Beta', 'Gamma'])).toEqual(['Beta', 'Gamma', 'Alpha']); + }); +}); From a01d405dae0a5f03a517d5f565076f042a84726e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 05:02:02 +0000 Subject: [PATCH 3/9] chore(changeset): declare the grouped-sidebar reorder fix (objectui#11626) Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- .changeset/11626-grouped-nav-reorder.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/11626-grouped-nav-reorder.md diff --git a/.changeset/11626-grouped-nav-reorder.md b/.changeset/11626-grouped-nav-reorder.md new file mode 100644 index 0000000000..8dad749635 --- /dev/null +++ b/.changeset/11626-grouped-nav-reorder.md @@ -0,0 +1,12 @@ +--- +'@object-ui/layout': patch +'@object-ui/app-shell': patch +--- + +Drag-to-reorder works on a grouped sidebar menu, within each level (objectui#11626). `NavigationRenderer` had a sortable path only in its group-free arm. Every stock app's menu is grouped, so the console's `enableReorder` drew no grip anywhere a user could reach. + +**`@object-ui/layout`.** With `enableReorder` on, each group's children are now a sortable list of their own, and so is each run of top-level entries between two groups. An entry moves within its level only. It never moves into or out of a group, because which group an entry sits in is the app's structure, not a personal order. A move is reported through the existing `onReorder(reorderedItems)`, always with the top-level list. After a move within a group, that list is as drawn and the group's `children` are reordered. The moved level's entries carry their new positions as `order` (0, 1, 2, …), as a move in a group-free menu already did. While `searchQuery` narrows a grouped menu, the menu offers no grip, because a narrowed group shows only some of its children. A menu with no groups behaves exactly as before. + +**`@object-ui/app-shell`.** The sidebar's personal order store (localStorage `objectui-nav-order-APP`) keeps an order for each group under the group's `id`, beside the top level's `__root__`. The `id` is the one key a group has that holds across reloads and locales, because its label is translated. A move writes only the level that moved, so the rest of the menu keeps following the app. Every stored level is applied on load. A group-free app's `__root__` record is written and read exactly as before, so orders users already saved keep working. + +**Clause-②: no.** No export, prop, type member, callback parameter or i18n key is added or removed, and no signature changes. `NavigationRendererProps.enableReorder` and `onReorder` keep their types. Their doc comments, which ship in the published `.d.ts`, now state the within-level scope and what `onReorder` receives. From 9adb8d14d4349ace7123b3ab0ebb0127df34ebbd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 05:09:17 +0000 Subject: [PATCH 4/9] refactor(layout): type the sortable row props' `t` from the renderer prop (objectui#11626) Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- packages/layout/src/NavigationRenderer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index f6f6841b6d..6d4283a7d2 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -1195,7 +1195,7 @@ interface NavRowProps { onPinToggle?: (itemId: string, pinned: boolean, item?: NavigationItem, basePath?: string) => void; resolveTargetLabel?: NavTargetLabelResolver; locale?: string; - t?: (key: string, options?: any) => string; + t?: NavigationRendererProps['t']; templateContext?: NavTemplateContext; } From 75c86feb785fc67f8a4df4a5b609898b71d85abf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 06:33:05 +0000 Subject: [PATCH 5/9] test(layout): a row inside a sortable group still lights for the current route (objectui#11626) Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- ...avigationRenderer.groupedReorder-11626.test.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx b/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx index 753f18e33f..3e34517b33 100644 --- a/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx +++ b/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx @@ -200,6 +200,20 @@ describe('objectui#11626 — a grouped menu offers drag-to-reorder within each l expect(ids(childrenOf(outer!, 'grp_inner'))).toEqual(['i2', 'i1']); }); + it('a row inside a sortable group still lights for the current route, and opens its group', () => { + const { container } = render( + + + + + , + ); + const lit = [...container.querySelectorAll('[data-sidebar="menu-button"][data-active="true"]')]; + expect(lit.map((el) => el.textContent)).toEqual(['A2']); + // The lit row sits in a sortable row: its grip is drawn beside it. + expect(lit[0].closest('[data-sidebar="menu-item"]')?.querySelector(GRIP)).not.toBeNull(); + }); + it('while a search narrows a grouped menu, it offers no grip', () => { const { container } = renderNav(GROUPED, { enableReorder: true, onReorder: vi.fn(), searchQuery: 'a' }); // Control: the search matched entries, so rows are drawn. From 6eb56eb914b4c55f032296b73bf0aa0d8db74abf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 08:35:38 +0000 Subject: [PATCH 6/9] fix(layout,app-shell): a saved nav order holds where the app authors `order`, and the grip is the drag activator (objectui#11626) Review of the resumed branch found two holes the grouped arm inherited from the group-free one, both measured before this change: - The renderer sorts every level by `order`. `useNavOrder` applied a saved order by array position only, so where an app authors `order` a drag was stored and sorted straight back (group and `__root__` alike). A level with a saved order now carries its positions as `order`; a level without one is untouched. The moved-group detection compares `order` as well as ids, since a move under authored `order` can land on the listed id sequence. - dnd-kit's `attributes` (role=button, tabIndex=0) sat on the row wrapper and its `listeners` on the grip, so every row was a focusable "button" no key could drag from. The grip is now the activator (`setActivatorNodeRef`), with attributes and listeners together. Spreading the old wrapper to every grouped menu would have added one dead tab stop per entry on every stock app. The stored `__root__` record of a group-free app is byte-identical. Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- .../app-shell/src/layout/UnifiedSidebar.tsx | 41 ++++++++---- ...fiedSidebar.groupedNavOrder-11626.test.tsx | 65 +++++++++++++++++++ packages/layout/src/NavigationRenderer.tsx | 58 ++++++++++------- ...tionRenderer.groupedReorder-11626.test.tsx | 33 +++++++++- 4 files changed, 160 insertions(+), 37 deletions(-) diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index 06c761bca2..de8ef08f83 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -83,8 +83,15 @@ const ROOT_ORDER_KEY = '__root__'; /** * One level in its saved order: the saved ids first, in the saved order, then - * the entries the saved order does not name, in their own order. An id saved - * for an entry that is gone is skipped. + * the entries the saved order does not name, in the order the app lists them. + * An id saved for an entry that is gone is skipped. + * + * Each entry of a level with a saved order carries its position there as + * `order` (0, 1, 2, …). The renderer sorts every level by `order` (the spec's + * "Sort order within the same level"), so for an app that authors `order` a + * saved order left in array position only was sorted straight back into the + * app's own, and a drag snapped back (objectui#11626). A level with no saved + * order is returned untouched and keeps following the app. */ function applyLevelOrder(items: NavigationItem[], saved: string[] | undefined): NavigationItem[] { if (!saved) return items; @@ -95,7 +102,7 @@ function applyLevelOrder(items: NavigationItem[], saved: string[] | undefined): if (item) { ordered.push(item); byId.delete(id); } } byId.forEach(item => ordered.push(item)); - return ordered; + return ordered.map((item, idx) => (item.order === idx ? item : { ...item, order: idx })); } /** Every group's saved order applied to its children, at every depth. */ @@ -114,22 +121,23 @@ function applyGroupOrders( return changed ? next : items; } -/** The ids of each group's children, in order, by group `id`, at every depth. */ -function groupChildIds( +/** Each group's children, by group `id`, at every depth. */ +function groupChildren( items: NavigationItem[], - into: Map = new Map(), -): Map { + into: Map = new Map(), +): Map { for (const item of items) { if (item.type !== 'group') continue; const children = item.children ?? []; - into.set(item.id, children.map(c => c.id)); - groupChildIds(children, into); + into.set(item.id, children); + groupChildren(children, into); } return into; } -const sameIds = (a: string[], b: string[]) => - a.length === b.length && a.every((id, i) => id === b[i]); +/** The same entries, in the same places, each with the same `order`. */ +const sameLevel = (a: NavigationItem[], b: NavigationItem[]) => + a.length === b.length && a.every((item, i) => item.id === b[i].id && item.order === b[i].order); function useNavOrder(appName: string) { const storageKey = `objectui-nav-order-${appName}`; @@ -174,14 +182,19 @@ function useNavOrder(appName: string) { * and every other group keep following the app until the user moves them. * A report in which no group's children moved is a move among top-level * entries, stored under `__root__` exactly as it was before groups had one. + * + * Children are compared by id AND `order`. The moved level comes back with + * its positions as `order` (0, 1, 2, …); the renderer had sorted it by + * `order` first, so where an app authors `order` the move can land on the + * drawn ARRAY's own id sequence, and only the positions tell it apart. */ const handleReorder = React.useCallback( (reorderedItems: NavigationItem[], drawn: NavigationItem[]) => { - const before = groupChildIds(drawn); + const before = groupChildren(drawn); const movedGroups: Record = {}; - groupChildIds(reorderedItems).forEach((ids, groupId) => { + groupChildren(reorderedItems).forEach((children, groupId) => { const was = before.get(groupId); - if (was && !sameIds(was, ids)) movedGroups[groupId] = ids; + if (was && !sameLevel(was, children)) movedGroups[groupId] = children.map(c => c.id); }); if (Object.keys(movedGroups).length > 0) { persist({ ...orderMap, ...movedGroups }); diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx index d16556ed3b..3a0130edcb 100644 --- a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.groupedNavOrder-11626.test.tsx @@ -263,6 +263,58 @@ describe('UnifiedSidebar — a grouped app menu keeps a personal order per group }); }); +describe('UnifiedSidebar — a saved order holds where the app authors `order` (objectui#11626)', () => { + // The renderer sorts every level by `order`. A saved order applied by array + // position alone was sorted straight back into the app's: stored, never drawn. + const AUTHORED: NavigationItem[] = [ + { + id: 'grp_sales', + type: 'group', + label: 'Sales', + children: [ + entry('nav_accounts', 'Accounts', { order: 1 }), + entry('nav_contacts', 'Contacts', { order: 2 }), + entry('nav_leads', 'Leads', { order: 3 }), + ], + }, + ]; + + it('a moved group is drawn in its new order at once and after a reload', () => { + const first = render(sidebarUi(AUTHORED)); + expect(linkOrder(['Accounts', 'Contacts', 'Leads'])).toEqual(['Accounts', 'Contacts', 'Leads']); + + drop('nav-reorder-group-grp_sales', 'nav_leads', 'nav_accounts'); + + expect(stored()).toEqual({ grp_sales: ['nav_leads', 'nav_accounts', 'nav_contacts'] }); + expect(linkOrder(['Accounts', 'Contacts', 'Leads'])).toEqual(['Leads', 'Accounts', 'Contacts']); + first.unmount(); + + render(sidebarUi(AUTHORED)); + expect(linkOrder(['Accounts', 'Contacts', 'Leads'])).toEqual(['Leads', 'Accounts', 'Contacts']); + }); + + it('a move that lands on the listed sequence is still that group’s move, not a top-level one', () => { + // Listed Alpha, Beta; authored `order` draws Beta first. Dragging Alpha + // above Beta reports the group as Alpha, Beta: the listed id sequence. + const navigation: NavigationItem[] = [ + entry('nav_top', 'Top'), + { + id: 'grp_ab', + type: 'group', + label: 'AB', + children: [entry('nav_alpha', 'Alpha', { order: 2 }), entry('nav_beta', 'Beta', { order: 1 })], + }, + ]; + render(sidebarUi(navigation)); + expect(linkOrder(['Alpha', 'Beta'])).toEqual(['Beta', 'Alpha']); + + drop('nav-reorder-group-grp_ab', 'nav_alpha', 'nav_beta'); + + expect(stored()).toEqual({ grp_ab: ['nav_alpha', 'nav_beta'] }); + expect(linkOrder(['Alpha', 'Beta'])).toEqual(['Alpha', 'Beta']); + }); +}); + describe('UnifiedSidebar — a group-free app’s `__root__` record is unchanged (objectui#11626)', () => { const FLAT: NavigationItem[] = [entry('nav_a', 'Alpha'), entry('nav_b', 'Beta'), entry('nav_c', 'Gamma')]; @@ -282,4 +334,17 @@ describe('UnifiedSidebar — a group-free app’s `__root__` record is unchanged render(sidebarUi(FLAT)); expect(linkOrder(['Alpha', 'Beta', 'Gamma'])).toEqual(['Beta', 'Gamma', 'Alpha']); }); + + it('where the app authors `order`, the record is the same and a saved order is now drawn', () => { + const authored = FLAT.map((item, i) => ({ ...item, order: i + 1 })); + const first = render(sidebarUi(authored)); + drop('(no id)', 'nav_c', 'nav_a'); + + expect(localStorage.getItem(STORE)).toBe('{"__root__":["nav_c","nav_a","nav_b"]}'); + expect(linkOrder(['Alpha', 'Beta', 'Gamma'])).toEqual(['Gamma', 'Alpha', 'Beta']); + first.unmount(); + + render(sidebarUi(authored)); + expect(linkOrder(['Alpha', 'Beta', 'Gamma'])).toEqual(['Gamma', 'Alpha', 'Beta']); + }); }); diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index 6d4283a7d2..9f32628af6 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -41,6 +41,8 @@ import { useSensor, useSensors, type DragEndEvent, + type DraggableAttributes, + type DraggableSyntheticListeners, } from '@dnd-kit/core'; import { SortableContext, @@ -1239,6 +1241,28 @@ function SortableNavigationList({ ); } +/** A sortable row's grip: dnd-kit's activator ref, its ARIA attributes and its listeners. */ +interface NavDragHandle { + ref: (element: HTMLElement | null) => void; + attributes: DraggableAttributes; + listeners: DraggableSyntheticListeners; +} + +/** The drag grip drawn at the start of a sortable row; the row's one drag activator. */ +function NavDragGrip({ handle, t }: { handle: NavDragHandle; t?: NavigationRendererProps['t'] }) { + return ( + + + + ); +} + // --------------------------------------------------------------------------- // SortableNavigationItem (drag-reorder wrapper) // --------------------------------------------------------------------------- @@ -1278,6 +1302,7 @@ function SortableNavigationItem({ attributes, listeners, setNodeRef, + setActivatorNodeRef, transform, transition, isDragging, @@ -1290,8 +1315,13 @@ function SortableNavigationItem({ zIndex: isDragging ? 10 : undefined, }; + // The grip is the drag activator: dnd-kit's `attributes` (`role="button"`, + // `tabIndex={0}`, the sortable ARIA description) go on it together with the + // `listeners`, so the one element a keyboard can focus is the one the + // KeyboardSensor listens on. On the row wrapper they made every row a + // focusable "button" that no key could start a drag from (objectui#11626). return ( -
+
void; enablePinning?: boolean; onPinToggle?: (itemId: string, pinned: boolean, item?: NavigationItem, basePath?: string) => void; - dragListeners?: Record; + dragHandle?: NavDragHandle; resolveTargetLabel?: NavTargetLabelResolver; locale?: string; t?: (key: string, options?: any) => string; @@ -1501,15 +1531,7 @@ function NavigationItemRenderer({ const actionLabel = resolveNavItemLabel(item, tProp, resolveTargetLabel, locale); return ( - {dragListeners && ( - - - - )} + {dragHandle && } onAction?.(item)} @@ -1569,15 +1591,7 @@ function NavigationItemRenderer({ return ( - {dragListeners && ( - - - - )} + {dragHandle && } {external ? ( diff --git a/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx b/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx index 3e34517b33..213953b60d 100644 --- a/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx +++ b/packages/layout/src/__tests__/NavigationRenderer.groupedReorder-11626.test.tsx @@ -169,7 +169,11 @@ describe('objectui#11626 — a grouped menu offers drag-to-reorder within each l onReorder, evaluateVisibility: (expr: unknown) => expr !== false, }); - expect(container.querySelectorAll('[aria-roledescription="sortable"]')).toHaveLength(2); + // The sortable wrappers are the group menu's direct children: one per drawn + // entry, and none left empty by the gated-away one. + const wrappers = [...container.querySelectorAll('[data-sidebar="menu"] > div')]; + expect(wrappers).toHaveLength(2); + expect(wrappers.every((w) => w.childElementCount > 0)).toBe(true); drop('nav-reorder-group-grp_a', 'a2', 'a1'); @@ -223,6 +227,33 @@ describe('objectui#11626 — a grouped menu offers drag-to-reorder within each l }); }); +describe('objectui#11626 — the grip is the one drag activator a keyboard can reach', () => { + const FLAT: NavigationItem[] = [leaf('f1'), leaf('f2'), leaf('f3')]; + + it.each([ + ['grouped', GROUPED, 7], + ['group-free', FLAT, 3], + ] as const)('%s menu: each grip is a focusable sortable button; no row wrapper is', (_shape, items, rows) => { + const { container } = renderNav(items as NavigationItem[], { enableReorder: true, onReorder: vi.fn() }); + const grips = [...container.querySelectorAll(GRIP)]; + expect(grips).toHaveLength(rows); + for (const grip of grips) { + expect(grip.getAttribute('role')).toBe('button'); + expect(grip.getAttribute('tabindex')).toBe('0'); + expect(grip.getAttribute('aria-roledescription')).toBe('sortable'); + } + // The sortable description sits on the grips and nowhere else: a row + // wrapper carrying it was a focusable "button" no key could drag from. + expect(container.querySelectorAll('[aria-roledescription="sortable"]')).toHaveLength(rows); + const wrappers = [...container.querySelectorAll('[data-sidebar="menu"] > div')]; + expect(wrappers).toHaveLength(rows); + for (const wrapper of wrappers) { + expect(wrapper.hasAttribute('tabindex')).toBe(false); + expect(wrapper.hasAttribute('role')).toBe(false); + } + }); +}); + describe('objectui#11626 — the group-free menu is unchanged', () => { const FLAT: NavigationItem[] = [leaf('f1'), leaf('f2'), leaf('f3')]; From c7a5d7b8a58c0956c8b8c3854d43ed98eeab1dbc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 08:44:36 +0000 Subject: [PATCH 7/9] chore(changeset): declare the authored-`order` and grip-activator fixes (objectui#11626) Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- .changeset/11626-grouped-nav-reorder.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.changeset/11626-grouped-nav-reorder.md b/.changeset/11626-grouped-nav-reorder.md index 8dad749635..e08eb79673 100644 --- a/.changeset/11626-grouped-nav-reorder.md +++ b/.changeset/11626-grouped-nav-reorder.md @@ -5,8 +5,12 @@ Drag-to-reorder works on a grouped sidebar menu, within each level (objectui#11626). `NavigationRenderer` had a sortable path only in its group-free arm. Every stock app's menu is grouped, so the console's `enableReorder` drew no grip anywhere a user could reach. -**`@object-ui/layout`.** With `enableReorder` on, each group's children are now a sortable list of their own, and so is each run of top-level entries between two groups. An entry moves within its level only. It never moves into or out of a group, because which group an entry sits in is the app's structure, not a personal order. A move is reported through the existing `onReorder(reorderedItems)`, always with the top-level list. After a move within a group, that list is as drawn and the group's `children` are reordered. The moved level's entries carry their new positions as `order` (0, 1, 2, …), as a move in a group-free menu already did. While `searchQuery` narrows a grouped menu, the menu offers no grip, because a narrowed group shows only some of its children. A menu with no groups behaves exactly as before. +**`@object-ui/layout`.** With `enableReorder` on, each group's children are now a sortable list of their own, and so is each run of top-level entries between two groups. An entry moves within its level only. It never moves into or out of a group, because which group an entry sits in is the app's structure, not a personal order. A move is reported through the existing `onReorder(reorderedItems)`, always with the top-level list. After a move within a group, that list is as drawn and the group's `children` are reordered. The moved level's entries carry their new positions as `order` (0, 1, 2, …), as a move in a group-free menu already did. While `searchQuery` narrows a grouped menu, the menu offers no grip, because a narrowed group shows only some of its children. + +The drag grip is now the row's drag activator, in both arms. dnd-kit's sortable attributes (`role="button"`, `tabIndex={0}`, the sortable description) used to sit on the row wrapper while the listeners sat on the grip, so every row was a focusable button that no key could start a drag from. The attributes and listeners now sit together on the grip. The one element a keyboard can focus per row is now the element the keyboard sensor listens on. **`@object-ui/app-shell`.** The sidebar's personal order store (localStorage `objectui-nav-order-APP`) keeps an order for each group under the group's `id`, beside the top level's `__root__`. The `id` is the one key a group has that holds across reloads and locales, because its label is translated. A move writes only the level that moved, so the rest of the menu keeps following the app. Every stored level is applied on load. A group-free app's `__root__` record is written and read exactly as before, so orders users already saved keep working. +A saved order now holds where the app authors `order` on its navigation entries. The renderer sorts each level by `order`, and the store used to apply a saved order by array position only, so on such an app a drag was stored and then sorted straight back, in a group-free menu as well. A level with a saved order now carries its saved positions as `order`. A level with no saved order is passed on untouched. + **Clause-②: no.** No export, prop, type member, callback parameter or i18n key is added or removed, and no signature changes. `NavigationRendererProps.enableReorder` and `onReorder` keep their types. Their doc comments, which ship in the published `.d.ts`, now state the within-level scope and what `onReorder` receives. From e42f5d5de1316bb90441f69507b05dc4c2f7996a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 09:33:23 +0000 Subject: [PATCH 8/9] docs(layout): the gated-away wrapper note no longer calls a sortable wrapper focusable (objectui#11626) Since the grip became the drag activator, a row wrapper carries no tabIndex; what an empty one would leave behind is a drop target. Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- packages/layout/src/NavigationRenderer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index 9f32628af6..36fdc5964f 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -1178,7 +1178,7 @@ const GroupReorderContext = React.createContext(nul * Whether `item` draws anything: the decisions `NavigationItemRenderer` takes * before it returns `null`, asked through the same shared guard statement and * predicate. A sortable wrapper is put only around an entry that draws, so a - * gated-away entry does not leave an empty, focusable drag wrapper behind. + * gated-away entry does not leave an empty wrapper behind as a drop target. */ function drawsNavItem(item: NavigationItem, options: NavigationVisibilityOptions): boolean { if (item.type === 'separator') return passesNavItemGuards(item, options); From beef430cf2eb0a3b14275ddc39164244655c9e01 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 10:29:54 +0000 Subject: [PATCH 9/9] refactor(layout): the drag grip destructures its handle, so it adds no refs-during-render lint warning (objectui#11626) `NavDragGrip` read `handle.ref`, `handle.attributes` and `handle.listeners` during render; the react-hooks refs rule took the whole handle for a ref and flagged all three reads (main 23 warnings in NavigationRenderer.tsx, the branch 25). The handle is destructured in the parameter list and its setter is named `activator`; the file now lints at 22, one fewer than main. Claude-Session: https://claude.ai/code/session_015W8GBu6sBiqus2L2xjMsAL Co-authored-by: Claude --- packages/layout/src/NavigationRenderer.tsx | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index 36fdc5964f..5eefa60976 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -1241,21 +1241,27 @@ function SortableNavigationList({ ); } -/** A sortable row's grip: dnd-kit's activator ref, its ARIA attributes and its listeners. */ +/** A sortable row's grip: dnd-kit's activator node setter, its ARIA attributes and its listeners. */ interface NavDragHandle { - ref: (element: HTMLElement | null) => void; + activator: (element: HTMLElement | null) => void; attributes: DraggableAttributes; listeners: DraggableSyntheticListeners; } /** The drag grip drawn at the start of a sortable row; the row's one drag activator. */ -function NavDragGrip({ handle, t }: { handle: NavDragHandle; t?: NavigationRendererProps['t'] }) { +function NavDragGrip({ + handle: { activator, attributes, listeners }, + t, +}: { + handle: NavDragHandle; + t?: NavigationRendererProps['t']; +}) { return ( @@ -1332,7 +1338,7 @@ function SortableNavigationItem({ onAction={onAction} enablePinning={enablePinning} onPinToggle={onPinToggle} - dragHandle={enableReorder ? { ref: setActivatorNodeRef, attributes, listeners } : undefined} + dragHandle={enableReorder ? { activator: setActivatorNodeRef, attributes, listeners } : undefined} resolveTargetLabel={resolveTargetLabel} locale={locale} t={tProp}