diff --git a/ui/packages/app-shell/src/views/metadata-admin/previews/AppNavCanvas.tsx b/ui/packages/app-shell/src/views/metadata-admin/previews/AppNavCanvas.tsx index 5fa13a4edae..445106608ab 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/previews/AppNavCanvas.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/previews/AppNavCanvas.tsx @@ -8,6 +8,10 @@ * fits the Studio's 240 px rail (objectui#12519). Drag-drop reorders within * the root list. * + * Each level is listed in the order the running menu draws it (objectstack#26979): + * by `order`, ties in authored order (`inMenuOrder`). A drop therefore has to + * move the entry in that order, which `order` can override: see `moveItem`. + * * Kind and target are read from the spec's discriminated union — `type` * plus that branch's own target key — never inferred from off-spec keys * (objectui#3275). @@ -41,7 +45,7 @@ import type { I18nLabel } from '@objectstack/spec/ui'; import { Badge, cn } from '@object-ui/components'; import type { NavTargetLabelResolver } from '@object-ui/layout'; import { useNavTargetLabel } from '../../../hooks/useNavTargetLabel.js'; -import { appendArray, moveArray, spliceArray } from '../inspectors/_shared.js'; +import { appendArray, spliceArray } from '../inspectors/_shared.js'; import { t, tFormat, useMetadataLocale } from '../i18n.js'; import { navEntryLabelText, navItemLabelText, renamedLabel } from './navItemLabel.js'; @@ -60,6 +64,8 @@ interface RawNav { book?: string; doc?: string; actionDef?: { actionName?: string }; + /** The spec's sort weight within a level (lower = first); read through `inMenuOrder` only. */ + order?: number; children?: RawNav[]; [k: string]: unknown; } @@ -254,6 +260,31 @@ export function navPayloadOf(entries: readonly unknown[]): Array(list: readonly T[]): Array { + return list.map((it, i) => [it, i] as const).sort(([a], [b]) => (a.order ?? 0) - (b.order ?? 0)); +} + export interface AppNavCanvasProps { draft: Record; rootKey: string; @@ -276,7 +307,12 @@ export function AppNavCanvas({ return Array.isArray(v) ? (v as RawNav[]) : []; }, [draft, rootKey]); + // The root level as the menu lists it, each entry with its array index: + // selection ids, renames and removals address the authored array. + const shown = React.useMemo(() => inMenuOrder(items), [items]); + const selectedId = selection && selection.kind === 'nav' ? selection.id : null; + /** The ARRAY index of the entry being dragged. */ const [dragIndex, setDragIndex] = React.useState(null); const setItems = React.useCallback( @@ -341,13 +377,36 @@ export function AppNavCanvas({ [onPatch, items, setItems, rootKey, selectedId, onSelectionChange, locale], ); + /** + * Drop the entry at array index `from` before the entry SHOWN at position + * `before` (`items.length` = at the end), as the menu will then draw it + * (objectstack#26979). + * + * Reordering the array alone does nothing visible once `order` is set, + * since the menu sorts by it. So the drop writes the array in the new + * shown order, and writes the moved entry's `order` only when its current + * weight no longer falls between its new neighbours': then it takes its + * neighbour's weight, and the stable sort keeps it on the array side it was + * dropped on. At most one `order` write per drop, and none for an app whose + * entries carry no `order`. + */ const moveItem = React.useCallback( (from: number, before: number) => { if (!onPatch) return; + const seq = shown.map(([, i]) => i); + const at = seq.indexOf(from); + if (at < 0) return; let to = before; - if (from < before) to = before - 1; - if (to === from) return; - const next = moveArray(items, from, to); + if (at < before) to = before - 1; + if (to === at) return; + seq.splice(at, 1); + seq.splice(to, 0, from); + const next = seq.map((i) => items[i]); + const moved = next[to]; + const prev = to > 0 ? (next[to - 1].order ?? 0) : -Infinity; + const after = to < next.length - 1 ? (next[to + 1].order ?? 0) : Infinity; + const own = moved.order ?? 0; + if (own < prev || own > after) next[to] = { ...moved, order: to > 0 ? prev : after }; setItems(next); onSelectionChange?.({ kind: 'nav', @@ -355,7 +414,7 @@ export function AppNavCanvas({ label: navLabel(next[to] ?? {}, to, locale, targetLabel), }); }, - [onPatch, items, setItems, rootKey, onSelectionChange, locale, targetLabel], + [onPatch, items, shown, setItems, rootKey, onSelectionChange, locale, targetLabel], ); return ( @@ -394,7 +453,7 @@ export function AppNavCanvas({ if (!e.dataTransfer.types.includes(DND_MIME)) return; e.preventDefault(); if (dragIndex == null) return; - moveItem(dragIndex, items.length); + moveItem(dragIndex, shown.length); setDragIndex(null); }} > @@ -405,7 +464,7 @@ export function AppNavCanvas({ : t('engine.appNav.emptyReadonly', locale)} ) : ( - items.map((it, i) => ( + shown.map(([it, i], at) => ( setDragIndex(null)} onDropBefore={() => { if (dragIndex == null) return; - moveItem(dragIndex, i); + moveItem(dragIndex, at); setDragIndex(null); }} /> @@ -484,7 +543,7 @@ function NavCardTree({ onDropBefore={onDropBefore} /> {Array.isArray(item.children) && - item.children.map((c, j) => { + inMenuOrder(item.children).map(([c, j]) => { const childPath = `${path}.children[${j}]`; return ( { }); import { InterfacesPillar, StudioNavItemInspector } from './StudioDesignSurface'; +import { AppNavCanvas } from '../metadata-admin/previews/AppNavCanvas'; import { t } from '../metadata-admin/i18n'; import { listStudioCanvasPreviewTypes } from './studio-canvas-preview'; @@ -1417,6 +1418,203 @@ describe('StudioDesignSurface.zhPillarNames-11801', () => { }); }); +// ── StudioDesignSurface.railNavOrder-26979 ─────────────────────────────────── + +/** + * objectstack#26979 — the Studio shows an app's nav entries in the order the + * running menu draws them. + * + * The menu sorts every level by `order` (`byNavOrder` in + * `packages/layout/src/navOrder.ts`: lower first, an absent `order` as `0`, + * ties in authored order), at the top level and inside each group. The + * Interfaces rail (`NavTree`) and the nav canvas (`AppNavCanvas`, which + * Setup's app editor mounts too) listed the authored array instead, so an + * entry given `order` stood in one place in Studio and another in the + * published console, and a canvas drag, which only reordered the array, could + * do nothing visible. Both now list a level through `inMenuOrder`, which + * states the menu's rule; `byNavOrder` is internal to its package, so these + * pins are what hold the two rules together: each renders the menu + * (`NavigationRenderer`) beside the rail and the canvas over the same entries. + * + * - entries with `order` [3, 1, 2] (and a group whose children carry + * [2, 1]) read in one order on all three surfaces; + * - a drag in the canvas leaves all three agreeing, writing `order` on the + * moved entry only; + * - CONTROL: entries with no `order` keep their array order, and a drag + * among them writes no `order` at all. + */ +describe('StudioDesignSurface.railNavOrder-26979', () => { + /** The navigation the mocked client serves the pillar; each test sets it. */ + let served: Array> = []; + + beforeEach(() => { + studio.client = mockClient; + }); + + const PKG = 'com.acme.app'; + + const page = (label: string, order?: number) => ({ + id: `nav_${label.toLowerCase()}`, + type: 'page', + label, + pageName: label.toLowerCase(), + ...(order === undefined ? {} : { order }), + }); + + /** Authored out of menu order: the menu reads Bravo, Charlie, Alpha, Folder (Yankee, Xray). */ + const ORDERED = [ + page('Alpha', 3), + page('Bravo', 1), + page('Charlie', 2), + { id: 'nav_folder', type: 'group', label: 'Folder', order: 4, expanded: true, children: [page('Xray', 2), page('Yankee', 1)] }, + ]; + + /** No `order` anywhere: the menu reads the array as authored. */ + const UNORDERED = [page('Alpha'), page('Bravo'), page('Charlie')]; + + const LABELS = ['Alpha', 'Bravo', 'Charlie', 'Folder', 'Xray', 'Yankee']; + + const mockClient = { + list: vi.fn(async (type: string) => (type === 'app' ? [{ name: 'acme_app', label: 'Acme' }] : [])), + listDrafts: vi.fn(async () => []), + layered: vi.fn(async (type: string, name: string) => { + if (type === 'app') return { effective: { name: 'acme_app', label: 'Acme', navigation: served } }; + if (type === 'page') return { effective: { name, label: name, type: 'app', regions: [{ name: 'main', components: [] }] } }; + return { effective: { name } }; + }), + getDraft: vi.fn(async () => null), + save: vi.fn(async () => ({})), + get: vi.fn(async () => undefined), + }; + + afterEach(cleanup); + + /** The labels `el` shows, in the order it shows them; each must show once. */ + function orderIn(el: HTMLElement): string[] { + const text = el.textContent ?? ''; + const shown = LABELS.filter((l) => text.includes(l)); + for (const l of shown) expect(text.indexOf(l)).toBe(text.lastIndexOf(l)); + return shown.sort((a, b) => text.indexOf(a) - text.indexOf(b)); + } + + /** The running app's sidebar over `nav`. */ + function menuOrder(nav: unknown[]): string[] { + const { container, unmount } = render( + + + + + , + ); + const order = orderIn(container); + unmount(); + return order; + } + + /** The Studio Interfaces rail over `nav`, once its rows have rendered. */ + async function railOrder(nav: Array>, first: string): Promise { + served = nav; + const { unmount } = render( + + + + + , + ); + // objectui#12455 — a page row is named by its label and its kind tag. + const row = await screen.findByRole('button', { name: `${first} Page` }, { timeout: 8000 }); + const rail = row.closest('nav'); + expect(rail).not.toBeNull(); + const order = orderIn(rail as HTMLElement); + unmount(); + return order; + } + + /** The nav canvas over `nav`, holding its patches as the host's draft does. */ + function renderCanvas(nav: unknown[]) { + const patches: unknown[][] = []; + function Host() { + const [draft, setDraft] = React.useState>({ navigation: nav }); + return ( +
+ { + patches.push(patch.navigation as unknown[]); + setDraft((d) => ({ ...d, ...patch })); + }} + /> +
+ ); + } + render(); + return { canvas: screen.getByTestId('nav-canvas'), patches }; + } + + /** The canvas card for `label`: the row button whose text starts with it. */ + const card = (label: string) => + screen.getAllByRole('button').find((b) => b.getAttribute('aria-pressed') !== null && b.textContent?.startsWith(label))!; + + /** Drag the card `label` and drop it before the card `before`, as a pointer does. */ + function dragBefore(label: string, before: string) { + // The canvas's own drag type (`DND_MIME` in `AppNavCanvas.tsx`); a drop of any other type is ignored. + const dataTransfer = { types: ['text/x-objectui-nav'], setData: vi.fn(), getData: vi.fn(), effectAllowed: '', dropEffect: '' }; + fireEvent.dragStart(card(label), { dataTransfer }); + fireEvent.dragOver(card(before), { dataTransfer }); + fireEvent.drop(card(before), { dataTransfer }); + } + + it('the fixtures are what the spec accepts', () => { + for (const item of [...ORDERED, ...UNORDERED]) expect(NavigationItemSchema.safeParse(item).success).toBe(true); + }); + + it('entries with order [3, 1, 2] read in the menu order on the rail, the canvas and the menu', async () => { + const menu = menuOrder(ORDERED); + // Measured on the menu first: it sorts the top level and the group's children. + expect(menu).toEqual(['Bravo', 'Charlie', 'Alpha', 'Folder', 'Yankee', 'Xray']); + expect(await railOrder(ORDERED, 'Bravo')).toEqual(menu); + const { canvas } = renderCanvas(ORDERED); + expect(orderIn(canvas)).toEqual(menu); + }); + + it('a drag in the canvas leaves all three agreeing, and writes order on the moved entry only', async () => { + const { canvas, patches } = renderCanvas(ORDERED); + // Alpha (order 3, shown third) to the top, before Bravo (order 1). + dragBefore('Alpha', 'Bravo'); + expect(patches).toHaveLength(1); + const next = patches[0] as Array>; + expect(orderIn(canvas)).toEqual(['Alpha', 'Bravo', 'Charlie', 'Folder', 'Yankee', 'Xray']); + // One `order` write: Alpha takes the weight of the entry it now precedes. + // Every other entry is the object it was. + const before = new Map(ORDERED.map((it) => [it.id, it])); + for (const it of next) { + if (it.id === 'nav_alpha') expect(it).toEqual({ ...before.get('nav_alpha'), order: 1 }); + else expect(it).toBe(before.get(it.id as string)); + } + cleanup(); + const menu = menuOrder(next); + expect(menu).toEqual(['Alpha', 'Bravo', 'Charlie', 'Folder', 'Yankee', 'Xray']); + expect(await railOrder(next, 'Alpha')).toEqual(menu); + }); + + it('CONTROL: entries with no order keep their array order, and a drag among them writes no order', async () => { + const menu = menuOrder(UNORDERED); + expect(menu).toEqual(['Alpha', 'Bravo', 'Charlie']); + expect(await railOrder(UNORDERED, 'Alpha')).toEqual(menu); + const { canvas, patches } = renderCanvas(UNORDERED); + expect(orderIn(canvas)).toEqual(menu); + dragBefore('Charlie', 'Alpha'); + const next = patches[0] as Array>; + expect(next.map((it) => it.id)).toEqual(['nav_charlie', 'nav_alpha', 'nav_bravo']); + for (const it of next) expect(it).not.toHaveProperty('order'); + expect(orderIn(canvas)).toEqual(['Charlie', 'Alpha', 'Bravo']); + cleanup(); + expect(menuOrder(next)).toEqual(['Charlie', 'Alpha', 'Bravo']); + }); +}); + // ── StudioDesignSurface.navEntryIdentity-11774 ─────────────────────────────── /** diff --git a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 635d52c65fc..5731dbc25f3 100644 --- a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -278,7 +278,7 @@ import { ObjectPicker } from '../metadata-admin/inspectors/ObjectPicker.js'; import { IconPickerWidget } from '../metadata-admin/widgets.js'; import { useDisplayLocale } from '@object-ui/i18n'; import { SuggestedBindingsPanel } from '../../components/SuggestedBindingsPanel.js'; -import { AppNavCanvas, navPayloadOf } from '../metadata-admin/previews/AppNavCanvas.js'; +import { AppNavCanvas, inMenuOrder, navPayloadOf } from '../metadata-admin/previews/AppNavCanvas.js'; import { FlowRegistrationContext, FlowRuntimeContext, @@ -2741,9 +2741,14 @@ function NavTree({ // What a label-less row inherits is asked of the console's own resolver, so // the rail names it as the console's sidebar does (objectui#11196). const targetLabel = useNavTargetLabel(); + // objectstack#26979 — a level is listed in the order the running menu draws + // it, through the canvas's `inMenuOrder` (the menu's `byNavOrder` rule). `i` + // stays the entry's position in the authored array, which the canvas numbers + // an untitled entry by too. + const ordered = inMenuOrder(nodes); return ( <> - {nodes.map((node, i) => { + {ordered.map(([node, i]) => { // objectui#11158 — the spec types a nav item's `label` as `I18nLabel`, // so a locale map rendered raw threw ("Objects are not valid as a // React child") and took the whole rail down. Every read below goes diff --git a/ui/packages/app-shell/src/views/studio-design/navSurface.ts b/ui/packages/app-shell/src/views/studio-design/navSurface.ts index 65242993733..595ebbe8c06 100644 --- a/ui/packages/app-shell/src/views/studio-design/navSurface.ts +++ b/ui/packages/app-shell/src/views/studio-design/navSurface.ts @@ -91,6 +91,8 @@ export interface NavNode { label?: I18nLabel; type?: string; icon?: string; + /** The spec's sort weight within a level (lower = first); read through `inMenuOrder` only. */ + order?: number; children?: NavNode[]; /** * The surface-binding target keys — CANONICAL SPELLINGS ONLY (objectui#4881).