From 41f0d780bba851d1ee356d94d4d9065f51a0af49 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:08:36 +0000 Subject: [PATCH 1/6] =?UTF-8?q?fix(types,plugin-designer)!:=20the=20app=20?= =?UTF-8?q?wizard=20saves=20a=20spec=20app=20=E2=80=94=20separator=20witho?= =?UTF-8?q?ut=20label,=20branding=20kept=20on=20edit,=20no=20Layout=20cont?= =?UTF-8?q?rol?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - @object-ui/types: NavigationItem is a union of NavigationEntryItem and NavigationSeparatorItem. The separator arm admits exactly the spec separator's keys (type, id, order); every other entry key is `?: never`. menuItemToNavigationItem drops a legacy separator's label. AppWizardDraft.layout is removed. - spec-derived-unions: the blocker-3 pin (separator label) is lifted and replaced by an agreement pin against the spec separator at both tiers. - @object-ui/plugin-designer: the wizard and NavigationDesigner write a separator as { id, type }; the wizard's Layout control is removed; EditAppPage keeps every stored branding key the spec's AppBrandingSchema declares (accentColor included) through the edit save. - @object-ui/i18n: the four appDesigner.layout* keys are removed from all ten packs, and stepBasicDesc no longer names a layout. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- ROADMAP.md | 2 +- .../phase1-page-blocks.render.test.tsx | 2 +- packages/i18n/src/locales/ar.ts | 6 +- packages/i18n/src/locales/de.ts | 6 +- packages/i18n/src/locales/en.ts | 6 +- packages/i18n/src/locales/es.ts | 6 +- packages/i18n/src/locales/fr.ts | 6 +- packages/i18n/src/locales/ja.ts | 6 +- packages/i18n/src/locales/ko.ts | 6 +- packages/i18n/src/locales/pt.ts | 6 +- packages/i18n/src/locales/ru.ts | 6 +- packages/i18n/src/locales/zh.ts | 6 +- .../src/__tests__/AppSchemaRenderer.test.tsx | 2 +- .../plugin-designer/src/AppCreationWizard.tsx | 57 +----- .../src/NavigationDesigner.tsx | 36 ++-- .../AppWizard.specDocument-10867.test.tsx | 187 ++++++++++++++++++ .../src/hooks/useDesignerTranslation.ts | 6 +- .../plugin-designer/src/pages/EditAppPage.tsx | 34 +++- .../src/__tests__/app-creation-types.test.ts | 5 - .../__tests__/app-declared-keys-10842.test.ts | 1 - .../app-logo-one-spelling-10827.test.ts | 1 - .../app-wizard-separator-layout-10867.test.ts | 94 +++++++++ .../src/__tests__/navigation-model.test.ts | 5 +- .../__tests__/navigation-spec-parity.test.ts | 2 +- .../src/__tests__/spec-derived-unions.test.ts | 47 +++-- packages/types/src/app.ts | 92 ++++++--- packages/types/src/index.ts | 2 + scripts/check-spec-symbol-derivation.mjs | 7 +- 28 files changed, 466 insertions(+), 176 deletions(-) create mode 100644 packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx create mode 100644 packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts diff --git a/ROADMAP.md b/ROADMAP.md index e6bd667823..e145d54b6f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -673,7 +673,7 @@ ObjectUI is a universal Server-Driven UI (SDUI) engine built on React + Tailwind - [x] `wizardDraftToAppSchema()` draft-to-schema conversion function **App Creation Wizard (4-step):** -- [x] Step 1: Basic Info — name (snake_case validated), title, description, icon, template, layout selector +- [x] Step 1: Basic Info — name (snake_case validated), title, description, icon, template (the layout selector it also carried was removed by objectui#10867: `@objectstack/spec` declares no app layout, so the choice was never saved) - [x] Step 2: Object Selection — card grid with search, select all/none, toggle selection - [x] Step 3: Navigation Builder — auto-generates NavigationItem[] from selected objects, add group/URL/separator, reorder up/down, remove - [x] Step 4: Branding — logo URL, primary color, favicon, live preview card diff --git a/packages/app-shell/src/views/__tests__/phase1-page-blocks.render.test.tsx b/packages/app-shell/src/views/__tests__/phase1-page-blocks.render.test.tsx index 98e3d3fa06..f71686c212 100644 --- a/packages/app-shell/src/views/__tests__/phase1-page-blocks.render.test.tsx +++ b/packages/app-shell/src/views/__tests__/phase1-page-blocks.render.test.tsx @@ -107,7 +107,7 @@ const APPS = [ objectName: 'sys_invoice', requiresObject: 'sys_invoice', }, - { id: 'divider_1', type: 'separator', label: '' }, + { id: 'divider_1', type: 'separator' }, ], }, { name: 'ops', label: 'Operations', icon: 'Wrench', navigation: [] }, diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 3d198a3ff5..911c7660d5 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -1327,10 +1327,6 @@ const ar = { appDescription: "الوصف", appIcon: "الأيقونة", template: "القالب", - layout: "التخطيط", - layoutSidebar: "الشريط الجانبي", - layoutHeader: "الرأس", - layoutEmpty: "فارغ", selectObjects: "تحديد الكائنات", searchObjects: "البحث في الكائنات…", selectAll: "تحديد الكل", @@ -1365,7 +1361,7 @@ const ar = { appearance: "المظهر", rowHeight: "ارتفاع الصف", livePreview: "معاينة مباشرة", - stepBasicDesc: "الاسم والعنوان والتخطيط", + stepBasicDesc: "الاسم والعنوان والأيقونة", stepObjectsDesc: "اختيار كائنات الأعمال", stepNavigationDesc: "بناء شجرة التنقل", stepBrandingDesc: "الشعار والألوان والأيقونة", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 7ed892f6fc..4356b55434 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -1307,10 +1307,6 @@ const de = { appDescription: "Beschreibung", appIcon: "Symbol", template: "Vorlage", - layout: "Layout", - layoutSidebar: "Seitenleiste", - layoutHeader: "Kopfzeile", - layoutEmpty: "Leer", selectObjects: "Objekte auswählen", searchObjects: "Objekte suchen…", selectAll: "Alle auswählen", @@ -1345,7 +1341,7 @@ const de = { appearance: "Erscheinungsbild", rowHeight: "Zeilenhöhe", livePreview: "Echtzeit-Vorschau", - stepBasicDesc: "Name, Titel und Layout", + stepBasicDesc: "Name, Titel und Symbol", stepObjectsDesc: "Geschäftsobjekte auswählen", stepNavigationDesc: "Navigationsbaum erstellen", stepBrandingDesc: "Logo, Farben und Favicon", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 94c3513e22..b853e3fb83 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -1572,10 +1572,6 @@ const en = { appDescription: 'Description', appIcon: 'Icon', template: 'Template', - layout: 'Layout', - layoutSidebar: 'Sidebar', - layoutHeader: 'Header', - layoutEmpty: 'Empty', selectObjects: 'Select Objects', searchObjects: 'Search objects…', selectAll: 'Select All', @@ -1610,7 +1606,7 @@ const en = { appearance: 'Appearance', rowHeight: 'Row Height', livePreview: 'Live Preview', - stepBasicDesc: 'Name, title, and layout', + stepBasicDesc: 'Name, title, and icon', stepObjectsDesc: 'Select business objects', stepNavigationDesc: 'Build navigation tree', stepBrandingDesc: 'Logo, colors, and favicon', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index f74b8249d5..1e5eec589a 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -1311,10 +1311,6 @@ const es = { appDescription: "Descripción", appIcon: "Icono", template: "Plantilla", - layout: "Diseño", - layoutSidebar: "Barra lateral", - layoutHeader: "Encabezado", - layoutEmpty: "Vacío", selectObjects: "Seleccionar objetos", searchObjects: "Buscar objetos…", selectAll: "Seleccionar todo", @@ -1349,7 +1345,7 @@ const es = { appearance: "Apariencia", rowHeight: "Altura de fila", livePreview: "Vista previa en vivo", - stepBasicDesc: "Nombre, título y diseño", + stepBasicDesc: "Nombre, título e icono", stepObjectsDesc: "Seleccionar objetos de negocio", stepNavigationDesc: "Construir árbol de navegación", stepBrandingDesc: "Logo, colores y favicon", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index d9fb4dd1e6..a2e31fd9af 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -1309,10 +1309,6 @@ const fr = { appDescription: "Description", appIcon: "Icône", template: "Modèle", - layout: "Disposition", - layoutSidebar: "Barre latérale", - layoutHeader: "En-tête", - layoutEmpty: "Vide", selectObjects: "Sélectionner des objets", searchObjects: "Rechercher des objets…", selectAll: "Tout sélectionner", @@ -1347,7 +1343,7 @@ const fr = { appearance: "Apparence", rowHeight: "Hauteur de ligne", livePreview: "Aperçu en direct", - stepBasicDesc: "Nom, titre et mise en page", + stepBasicDesc: "Nom, titre et icône", stepObjectsDesc: "Sélectionner les objets métier", stepNavigationDesc: "Construire l'arbre de navigation", stepBrandingDesc: "Logo, couleurs et favicon", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index b6cd2c9f37..147c73ebb6 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -1307,10 +1307,6 @@ const ja = { appDescription: "説明", appIcon: "アイコン", template: "テンプレート", - layout: "レイアウト", - layoutSidebar: "サイドバー", - layoutHeader: "ヘッダー", - layoutEmpty: "空", selectObjects: "オブジェクトを選択", searchObjects: "オブジェクトを検索…", selectAll: "すべて選択", @@ -1345,7 +1341,7 @@ const ja = { appearance: "外観", rowHeight: "行高さ", livePreview: "リアルタイムプレビュー", - stepBasicDesc: "名前、タイトル、レイアウト", + stepBasicDesc: "名前、タイトル、アイコン", stepObjectsDesc: "ビジネスオブジェクトを選択", stepNavigationDesc: "ナビゲーションツリーを構築", stepBrandingDesc: "ロゴ、色、ファビコン", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 799262f6d0..52ab8e7f7b 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -1307,10 +1307,6 @@ const ko = { appDescription: "설명", appIcon: "아이콘", template: "템플릿", - layout: "레이아웃", - layoutSidebar: "사이드바", - layoutHeader: "헤더", - layoutEmpty: "비어 있음", selectObjects: "오브젝트 선택", searchObjects: "오브젝트 검색…", selectAll: "모두 선택", @@ -1345,7 +1341,7 @@ const ko = { appearance: "외관", rowHeight: "행 높이", livePreview: "실시간 미리보기", - stepBasicDesc: "이름, 제목 및 레이아웃", + stepBasicDesc: "이름, 제목 및 아이콘", stepObjectsDesc: "비즈니스 객체 선택", stepNavigationDesc: "네비게이션 트리 구성", stepBrandingDesc: "로고, 색상 및 파비콘", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 98895a16b9..88fcb6e6d0 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -1306,10 +1306,6 @@ const pt = { appDescription: "Descrição", appIcon: "Ícone", template: "Modelo", - layout: "Layout", - layoutSidebar: "Barra lateral", - layoutHeader: "Cabeçalho", - layoutEmpty: "Vazio", selectObjects: "Selecionar objetos", searchObjects: "Pesquisar objetos…", selectAll: "Selecionar todos", @@ -1344,7 +1340,7 @@ const pt = { appearance: "Aparência", rowHeight: "Altura da linha", livePreview: "Pré-visualização em tempo real", - stepBasicDesc: "Nome, título e layout", + stepBasicDesc: "Nome, título e ícone", stepObjectsDesc: "Selecionar objetos de negócio", stepNavigationDesc: "Construir árvore de navegação", stepBrandingDesc: "Logo, cores e favicon", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index e30f60ff5b..49347199f9 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -1327,10 +1327,6 @@ const ru = { appDescription: "Описание", appIcon: "Значок", template: "Шаблон", - layout: "Макет", - layoutSidebar: "Боковая панель", - layoutHeader: "Заголовок", - layoutEmpty: "Пусто", selectObjects: "Выбрать объекты", searchObjects: "Поиск объектов…", selectAll: "Выбрать все", @@ -1365,7 +1361,7 @@ const ru = { appearance: "Внешний вид", rowHeight: "Высота строки", livePreview: "Предпросмотр в реальном времени", - stepBasicDesc: "Имя, заголовок и макет", + stepBasicDesc: "Имя, заголовок и значок", stepObjectsDesc: "Выбрать бизнес-объекты", stepNavigationDesc: "Построить дерево навигации", stepBrandingDesc: "Логотип, цвета и фавикон", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 7e8a415313..6d4c1dfa4e 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -1372,10 +1372,6 @@ const zh = { appDescription: '描述', appIcon: '图标', template: '模板', - layout: '布局', - layoutSidebar: '侧边栏', - layoutHeader: '顶部导航', - layoutEmpty: '空白', selectObjects: '选择对象', searchObjects: '搜索对象…', selectAll: '全选', @@ -1410,7 +1406,7 @@ const zh = { appearance: '外观', rowHeight: '行高', livePreview: '实时预览', - stepBasicDesc: '名称、标题和布局', + stepBasicDesc: '名称、标题和图标', stepObjectsDesc: '选择业务对象', stepNavigationDesc: '构建导航树', stepBrandingDesc: 'Logo、颜色和图标', diff --git a/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx b/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx index bde9702503..6d77a7a37e 100644 --- a/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx +++ b/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx @@ -440,7 +440,7 @@ describe('AppSchemaRenderer', () => { it('ignores separators — a divider is not content', () => { expect( - hasVisibleNavigationItems([{ id: 's1', type: 'separator', label: '' }]), + hasVisibleNavigationItems([{ id: 's1', type: 'separator' }]), ).toBe(false); }); diff --git a/packages/plugin-designer/src/AppCreationWizard.tsx b/packages/plugin-designer/src/AppCreationWizard.tsx index 0e3cfd8ad5..3d6a146dbf 100644 --- a/packages/plugin-designer/src/AppCreationWizard.tsx +++ b/packages/plugin-designer/src/AppCreationWizard.tsx @@ -11,7 +11,7 @@ * * Multi-step wizard for creating applications following the Airtable * Interface Designer UX pattern. Steps: - * 1. Basic Info (name, title, description, icon, template, layout) + * 1. Basic Info (name, title, description, icon, template) * 2. Object Selection (select business objects to include) * 3. Navigation Builder (build navigation tree from selected objects) * 4. Branding (logo, primary color, favicon) @@ -34,9 +34,6 @@ import { ChevronDown, Search, Trash2, - Layout, - PanelLeft, - LayoutTemplate, FolderOpen, Link, Minus, @@ -88,7 +85,6 @@ const DEFAULT_DRAFT: AppWizardDraft = { description: '', icon: '', template: '', - layout: 'sidebar', objects: [], navigation: [], branding: { @@ -310,42 +306,6 @@ function BasicInfoStep({ draft, templates, readOnly, onChange, t }: BasicInfoSte )} - - {/* Layout */} -
- {t('appDesigner.layout')} -
- {([ - { value: 'sidebar', labelKey: 'appDesigner.layoutSidebar', Icon: PanelLeft }, - { value: 'header', labelKey: 'appDesigner.layoutHeader', Icon: Layout }, - { value: 'empty', labelKey: 'appDesigner.layoutEmpty', Icon: LayoutTemplate }, - ] as const).map(({ value, labelKey, Icon }) => ( - - ))} -
-
); } @@ -786,13 +746,14 @@ export function AppCreationWizard({ }, []); const addNavItem = useCallback((type: 'group' | 'url' | 'separator') => { - const newItem: NavigationItem = { - id: createNavId(type), - type, - label: type === 'separator' ? '' : type === 'group' ? 'New Group' : 'New Link', - ...(type === 'group' ? { children: [] } : {}), - ...(type === 'url' ? { url: '' } : {}), - }; + // A separator carries only `type` and `id`: the spec's separator branch + // declares no `label`, and the save door refuses one (objectui#10867). + const newItem: NavigationItem = + type === 'separator' + ? { id: createNavId(type), type } + : type === 'group' + ? { id: createNavId(type), type, label: 'New Group', children: [] } + : { id: createNavId(type), type, label: 'New Link', url: '' }; setDraft((prev) => ({ ...prev, navigation: [...prev.navigation, newItem], diff --git a/packages/plugin-designer/src/NavigationDesigner.tsx b/packages/plugin-designer/src/NavigationDesigner.tsx index dc6cc9b5f0..aaf6822372 100644 --- a/packages/plugin-designer/src/NavigationDesigner.tsx +++ b/packages/plugin-designer/src/NavigationDesigner.tsx @@ -80,6 +80,26 @@ function createId(prefix: string): string { return `${prefix}_${Date.now()}_${ndCounter}`; } +/** + * A new navigation item of `type`. A separator carries only `type` and `id`: + * the spec's separator branch declares no `label`, and the save door refuses + * one (objectui#10867). Every other type gets the label `labelFor` names. + */ +function newNavItem( + type: NavigationItemType, + id: string, + labelFor: (type: Exclude) => string, +): NavigationItem { + if (type === 'separator') return { id, type }; + return { + id, + type, + label: labelFor(type), + ...(type === 'group' ? { children: [] } : {}), + ...(type === 'url' ? { url: '' } : {}), + }; +} + // Keyed by the spec-derived union, so a nav type the spec adds stops this file // compiling until it has an entry -- keep it a `Record`, never `Partial` or // `Record`. @@ -538,13 +558,7 @@ export function NavigationDesigner({ const addChild = useCallback( (parentId: string, type: NavigationItemType) => { - const newItem: NavigationItem = { - id: createId(type), - type, - label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, - ...(type === 'group' ? { children: [] } : {}), - ...(type === 'url' ? { url: '' } : {}), - }; + const newItem = newNavItem(type, createId(type), (entryType) => `New ${t(NAV_TYPE_META[entryType].labelKey)}`); function insertChild(list: NavigationItem[]): NavigationItem[] { return list.map((item) => { @@ -565,13 +579,7 @@ export function NavigationDesigner({ const addTopLevel = useCallback( (type: NavigationItemType) => { - const newItem: NavigationItem = { - id: createId(type), - type, - label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, - ...(type === 'group' ? { children: [] } : {}), - ...(type === 'url' ? { url: '' } : {}), - }; + const newItem = newNavItem(type, createId(type), (entryType) => `New ${t(NAV_TYPE_META[entryType].labelKey)}`); onChange([...items, newItem]); }, [items, onChange, t] diff --git a/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx b/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx new file mode 100644 index 0000000000..3d70871ff9 --- /dev/null +++ b/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx @@ -0,0 +1,187 @@ +/** + * 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. + */ + +/** + * The Studio app wizard saves a document the platform accepts and loses + * nothing it stored (objectui#10867). + * + * `CreateAppPage` and `EditAppPage` save through `client.meta.saveItem('app', + * …)`, and the platform's write door judges that body with + * `@objectstack/spec`'s strict `AppSchema`. Each case drives the REAL + * `AppCreationWizard` through its steps and reads what reaches `saveItem`: + * + * 1. "Add separator" wrote `{ id, type: 'separator', label: '' }`, and the + * spec's separator branch declares no `label`, so the save was refused + * (`unrecognized_keys` `['label']` at `navigation.N`). The separator now + * carries only `type` and `id`. + * 2. The edit save replaced the stored `branding` with the wizard's, which + * maintains only the logo, primary colour and favicon, so a stored + * `accentColor` (declared by the spec's `AppBrandingSchema`, read by the + * console) was dropped on every edit. It is now kept. + * 3. The Layout control persisted nothing (the spec declares no app layout) + * and is gone, with `AppWizardDraft.layout`. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { AppSchema as SpecAppSchema } from '@objectstack/spec/ui'; +import type { AppWizardDraft, ObjectSelection } from '@object-ui/types'; + +const saveItem = vi.fn().mockResolvedValue({}); +const apps: Record[] = []; +let routeParams: Record = {}; + +vi.mock('react-router-dom', () => ({ + useParams: () => routeParams, + useNavigate: () => vi.fn(), +})); + +const OBJECTS = [{ name: 'account', label: 'Account', pluralLabel: 'Accounts', icon: 'Building' }]; + +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + useAdapter: () => ({ getClient: () => ({ meta: { saveItem } }) }), + useMetadata: () => ({ apps, objects: OBJECTS, refresh: () => Promise.resolve() }), + }; +}); + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() } })); + +import { AppCreationWizard } from '../AppCreationWizard'; +import { CreateAppPage } from '../pages/CreateAppPage'; +import { EditAppPage } from '../pages/EditAppPage'; + +/** The spec's issues for a document, as `code` plus the path and keys it names. */ +const specIssues = (doc: unknown) => { + const r = SpecAppSchema.safeParse(doc); + return r.success + ? null + : r.error.issues.map((i) => ({ code: i.code, path: i.path.join('.'), keys: (i as { keys?: string[] }).keys })); +}; + +const next = () => fireEvent.click(screen.getByTestId('wizard-next')); + +/** The one body `saveItem` received. */ +async function savedBody(): Promise> { + await waitFor(() => expect(saveItem).toHaveBeenCalledTimes(1)); + const [type, name, body] = saveItem.mock.calls[0]; + expect([type, name]).toEqual(['app', 'acme_crm']); + return body as Record; +} + +beforeEach(() => { + saveItem.mockClear(); + apps.length = 0; + routeParams = {}; + localStorage.clear(); +}); +afterEach(() => cleanup()); + +describe('objectui#10867 — member 1: a wizard app with a separator saves a document the spec accepts', () => { + async function createWithSeparator() { + render(); + fireEvent.change(screen.getByTestId('app-name-input'), { target: { value: 'acme_crm' } }); + fireEvent.change(screen.getByTestId('app-title-input'), { target: { value: 'Acme CRM' } }); + next(); // → objects + fireEvent.click(screen.getByTestId('object-card-account')); + next(); // → navigation, generated from the selected object + fireEvent.click(screen.getByTestId('add-separator-btn')); + next(); // → branding + fireEvent.click(screen.getByTestId('wizard-complete')); + return savedBody(); + } + + it('the saved document parses green through the spec `AppSchema`', async () => { + const body = await createWithSeparator(); + expect(specIssues(body)).toBeNull(); + }); + + it('the saved separator carries only `type` and `id`', async () => { + const body = await createWithSeparator(); + const navigation = body.navigation as Array>; + expect(navigation.map((item) => item.type)).toEqual(['object', 'separator']); + expect(Object.keys(navigation[1]).sort()).toEqual(['id', 'type']); + }); + + it('CONTROL — the separator the wizard used to write is refused by the spec, so the pass above is not vacuous', async () => { + const body = await createWithSeparator(); + const navigation = body.navigation as Array>; + const old = { ...body, navigation: [navigation[0], { ...navigation[1], label: '' }] }; + expect(specIssues(old)).toEqual([{ code: 'unrecognized_keys', path: 'navigation.1', keys: ['label'] }]); + }); +}); + +describe('objectui#10867 — member 2: an edit keeps every stored `branding` key the spec declares', () => { + const STORED = { + name: 'acme_crm', + label: 'Acme CRM', + navigation: [{ id: 'account', type: 'object', label: 'Accounts', objectName: 'account' }], + branding: { logo: '/acme.svg', primaryColor: '#2563eb', accentColor: '#f59e0b', favicon: '/acme.ico' }, + }; + + async function editThrough(stored: Record, onBranding?: () => void) { + apps.push(stored); + routeParams = { editAppName: 'acme_crm' }; + render(); + next(); // → objects + next(); // → navigation + next(); // → branding + onBranding?.(); + fireEvent.click(screen.getByTestId('wizard-complete')); + return savedBody(); + } + + it('a stored `accentColor` survives the edit round trip', async () => { + const body = await editThrough(STORED); + expect(body.branding).toEqual(STORED.branding); + expect(specIssues(body)).toBeNull(); + }); + + it('the keys the wizard maintains come from the wizard; `accentColor` from storage', async () => { + const body = await editThrough(STORED, () => + fireEvent.change(screen.getByTestId('branding-color-input'), { target: { value: '#dc2626' } }), + ); + expect(body.branding).toEqual({ ...STORED.branding, primaryColor: '#dc2626' }); + }); + + it('CONTROL — a stored `branding` key the spec does not declare is not echoed into the save', async () => { + const body = await editThrough({ + ...STORED, + branding: { ...STORED.branding, fontFamily: 'Inter' }, + }); + expect(body.branding).toEqual(STORED.branding); + expect(specIssues(body)).toBeNull(); + }); +}); + +describe('objectui#10867 — member 3: the wizard has no Layout control and its draft no `layout`', () => { + it('the basic step renders no layout choice and does not mention one', () => { + const { container } = render(); + expect(container.querySelectorAll('[data-testid^="app-layout-"]')).toHaveLength(0); + expect(screen.queryAllByRole('radio')).toHaveLength(0); + expect(screen.queryAllByText(/layout/i)).toHaveLength(0); + }); + + it('the draft the wizard completes with carries no `layout` key', () => { + const onComplete = vi.fn(); + const objects: ObjectSelection[] = []; + render(); + fireEvent.change(screen.getByTestId('app-name-input'), { target: { value: 'acme_crm' } }); + fireEvent.change(screen.getByTestId('app-title-input'), { target: { value: 'Acme CRM' } }); + next(); + next(); + next(); + fireEvent.click(screen.getByTestId('wizard-complete')); + expect(onComplete).toHaveBeenCalledTimes(1); + const draft = onComplete.mock.calls[0][0] as AppWizardDraft; + expect(Object.prototype.hasOwnProperty.call(draft, 'layout')).toBe(false); + }); +}); diff --git a/packages/plugin-designer/src/hooks/useDesignerTranslation.ts b/packages/plugin-designer/src/hooks/useDesignerTranslation.ts index 3cecda4251..87bde0fedc 100644 --- a/packages/plugin-designer/src/hooks/useDesignerTranslation.ts +++ b/packages/plugin-designer/src/hooks/useDesignerTranslation.ts @@ -33,7 +33,7 @@ export const DESIGNER_DEFAULT_TRANSLATIONS: Record = { 'appDesigner.objects': 'Objects', 'appDesigner.navigation': 'Navigation', 'appDesigner.branding': 'Branding', - 'appDesigner.stepBasicDesc': 'Name, title, and layout', + 'appDesigner.stepBasicDesc': 'Name, title, and icon', 'appDesigner.stepObjectsDesc': 'Select business objects', 'appDesigner.stepNavigationDesc': 'Build navigation tree', 'appDesigner.stepBrandingDesc': 'Logo, colors, and favicon', @@ -43,10 +43,6 @@ export const DESIGNER_DEFAULT_TRANSLATIONS: Record = { 'appDesigner.appDescription': 'Description', 'appDesigner.appIcon': 'Icon', 'appDesigner.template': 'Template', - 'appDesigner.layout': 'Layout', - 'appDesigner.layoutSidebar': 'Sidebar', - 'appDesigner.layoutHeader': 'Header', - 'appDesigner.layoutEmpty': 'Empty', 'appDesigner.snakeCaseHint': 'Must be snake_case (e.g. my_app)', // Step 2 'appDesigner.searchObjects': 'Search objects…', diff --git a/packages/plugin-designer/src/pages/EditAppPage.tsx b/packages/plugin-designer/src/pages/EditAppPage.tsx index 51f488c25c..ce81488586 100644 --- a/packages/plugin-designer/src/pages/EditAppPage.tsx +++ b/packages/plugin-designer/src/pages/EditAppPage.tsx @@ -12,7 +12,7 @@ import { useNavigate, useParams } from 'react-router-dom'; import { AppCreationWizard } from '../AppCreationWizard'; import { wizardDraftToAppSchema } from '@object-ui/types'; import type { AppWizardDraft, ObjectSelection } from '@object-ui/types'; -import { AppSchema as SpecAppSchema } from '@objectstack/spec/ui'; +import { AppSchema as SpecAppSchema, AppBrandingSchema as SpecAppBrandingSchema } from '@objectstack/spec/ui'; import { useMetadata } from '@object-ui/react'; import { useAdapter } from '@object-ui/react'; import { toast } from 'sonner'; @@ -29,6 +29,22 @@ function getAppDeclaredKeys(): ReadonlySet { return appDeclaredKeys; } +/** + * The keys an app's `branding` may carry, read off the spec's strict + * `AppBrandingSchema` the same way (objectui#10867). + */ +let brandingDeclaredKeys: ReadonlySet | undefined; +function getBrandingDeclaredKeys(): ReadonlySet { + brandingDeclaredKeys ??= new Set(Object.keys(SpecAppBrandingSchema.shape)); + return brandingDeclaredKeys; +} + +/** The entries of `record` whose key `declared` holds. */ +function pickDeclared(record: unknown, declared: ReadonlySet): Record { + if (!record || typeof record !== 'object') return {}; + return Object.fromEntries(Object.entries(record).filter(([key]) => declared.has(key))); +} + export function EditAppPage() { const navigate = useNavigate(); const { appName, editAppName } = useParams(); @@ -59,7 +75,6 @@ export function EditAppPage() { title: appToEdit.label || '', description: appToEdit.description || '', icon: appToEdit.icon || '', - layout: appToEdit.layout || 'sidebar', navigation: appToEdit.navigation || [], branding: { logo: appToEdit.branding?.logo || '', @@ -78,11 +93,16 @@ export function EditAppPage() { // (objectui#10842). A row stored before that schema closed is served // with the old wizard's top-level `type` / `title` / `logo` / `favicon` // / `layout`, and the door refuses a save that echoes them back. - const declared = getAppDeclaredKeys(); - const preserved = Object.fromEntries( - Object.entries(appToEdit ?? {}).filter(([key]) => declared.has(key)), - ); - const merged = { ...preserved, ...appSchema }; + const preserved = pickDeclared(appToEdit, getAppDeclaredKeys()); + // The same rule one level down (objectui#10867): the wizard maintains + // the logo, primary colour and favicon, and its `branding` would + // otherwise REPLACE the stored block, dropping every other declared + // key — `accentColor`, which the console reads, on every edit. + const branding = { + ...pickDeclared(appToEdit?.branding, getBrandingDeclaredKeys()), + ...appSchema.branding, + }; + const merged = { ...preserved, ...appSchema, branding }; // Persist app metadata to backend const client = adapter?.getClient(); if (client) { diff --git a/packages/types/src/__tests__/app-creation-types.test.ts b/packages/types/src/__tests__/app-creation-types.test.ts index 49eb2b536a..08d2f5e508 100644 --- a/packages/types/src/__tests__/app-creation-types.test.ts +++ b/packages/types/src/__tests__/app-creation-types.test.ts @@ -49,7 +49,6 @@ describe('App Creation Types', () => { title: 'Test Application', description: 'A test app', icon: 'LayoutDashboard', - layout: 'sidebar', objects: [], navigation: [ { id: 'nav_1', type: 'object', label: 'Contacts', objectName: 'contacts' }, @@ -86,7 +85,6 @@ describe('App Creation Types', () => { const draft: AppWizardDraft = { name: 'empty_app', title: 'Empty', - layout: 'header', objects: [], navigation: [], branding: {}, @@ -94,7 +92,6 @@ describe('App Creation Types', () => { const schema = wizardDraftToAppSchema(draft); expect(schema.navigation).toEqual([]); - expect('layout' in schema).toBe(false); expect(schema.label).toBe('Empty'); }); @@ -103,7 +100,6 @@ describe('App Creation Types', () => { name: 'sales_crm', title: 'Sales CRM', icon: 'TrendingUp', - layout: 'sidebar', objects: [], navigation: [], branding: { @@ -126,7 +122,6 @@ describe('App Creation Types', () => { const draft: AppWizardDraft = { name: 'no_icon_app', title: 'No Icon', - layout: 'sidebar', objects: [], navigation: [], branding: {}, diff --git a/packages/types/src/__tests__/app-declared-keys-10842.test.ts b/packages/types/src/__tests__/app-declared-keys-10842.test.ts index 0e6e8856fe..b514c88a31 100644 --- a/packages/types/src/__tests__/app-declared-keys-10842.test.ts +++ b/packages/types/src/__tests__/app-declared-keys-10842.test.ts @@ -37,7 +37,6 @@ const DRAFT: AppWizardDraft = { title: 'Acme CRM', description: 'Accounts and deals', icon: 'Briefcase', - layout: 'header', objects: [{ name: 'account', label: 'Account', pluralLabel: 'Accounts', icon: 'Building', selected: true }], navigation: [ { id: 'account', type: 'object', label: 'Accounts', icon: 'Building', objectName: 'account' }, diff --git a/packages/types/src/__tests__/app-logo-one-spelling-10827.test.ts b/packages/types/src/__tests__/app-logo-one-spelling-10827.test.ts index d17a77d65e..ea1bc37953 100644 --- a/packages/types/src/__tests__/app-logo-one-spelling-10827.test.ts +++ b/packages/types/src/__tests__/app-logo-one-spelling-10827.test.ts @@ -45,7 +45,6 @@ const DRAFT: AppWizardDraft = { name: 'acme_crm', title: 'Acme CRM', icon: 'Briefcase', - layout: 'sidebar', objects: [], navigation: [], branding: { logo: 'https://cdn.example.test/acme.svg', primaryColor: '#2563eb' }, diff --git a/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts b/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts new file mode 100644 index 0000000000..49f8822159 --- /dev/null +++ b/packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts @@ -0,0 +1,94 @@ +/** + * 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. + */ + +/** + * A navigation separator carries only the spec separator's keys, and the + * wizard draft carries no `layout` (objectui#10867). + * + * `wizardDraftToAppSchema` feeds `client.meta.saveItem('app', …)`, which the + * platform judges with `@objectstack/spec`'s strict `AppSchema`. The spec's + * separator branch declares `type`, `id` and `order`; objectui's + * `NavigationItem` required a `label` on every item, so every separator the + * Studio wrote carried one and the save was refused (`unrecognized_keys` + * `['label']` at `navigation.N`). `NavigationItem` is now a union whose + * separator arm admits exactly the spec's keys. + * + * The compile-time halves (`@ts-expect-error`, `satisfies`) run under + * `tsc -p tsconfig.test.json`; the runtime halves under vitest. + */ + +import { describe, it, expect } from 'vitest'; +import { + AppSchema as SpecAppSchema, + NavigationItemSchema as SpecNavigationItemSchema, +} from '@objectstack/spec/ui'; +import { menuItemToNavigationItem, wizardDraftToAppSchema } from '../index'; +import type { AppWizardDraft, NavigationItem } from '../index'; + +/** The spec's issues for a document, as `code` plus the path and keys it names. */ +const specIssues = (doc: unknown) => { + const r = SpecAppSchema.safeParse(doc); + return r.success + ? null + : r.error.issues.map((i) => ({ code: i.code, path: i.path.join('.'), keys: (i as { keys?: string[] }).keys })); +}; + +const OBJECT_ENTRY: NavigationItem = { id: 'account', type: 'object', label: 'Accounts', objectName: 'account' }; +const GROUP_ENTRY: NavigationItem = { id: 'group_1', type: 'group', label: 'New Group', children: [] }; + +/** A draft the way `AppCreationWizard` builds it, with one separator. */ +const DRAFT: AppWizardDraft = { + name: 'acme_crm', + title: 'Acme CRM', + objects: [], + navigation: [OBJECT_ENTRY, { id: 'separator_1', type: 'separator' }, GROUP_ENTRY], + branding: { primaryColor: '#2563eb' }, +}; + +describe('objectui#10867 — a separator carries only `type`, `id` and `order`', () => { + it('a wizard draft with a separator converts to a document the spec `AppSchema` accepts', () => { + expect(specIssues(wizardDraftToAppSchema(DRAFT))).toBeNull(); + }); + + it('CONTROL — the separator the wizard used to write is refused at its own path', () => { + const doc = { + ...wizardDraftToAppSchema(DRAFT), + navigation: [OBJECT_ENTRY, { id: 'separator_1', type: 'separator', label: '' }, GROUP_ENTRY], + }; + expect(specIssues(doc)).toEqual([{ code: 'unrecognized_keys', path: 'navigation.1', keys: ['label'] }]); + }); + + it('`menuItemToNavigationItem` maps a legacy separator to the spec separator shape', () => { + const item = menuItemToNavigationItem({ type: 'separator', label: 'Section' }); + expect(item).toEqual({ id: 'migrated_0', type: 'separator' }); + expect(SpecNavigationItemSchema.safeParse(item).success).toBe(true); + }); + + it('a `label` on a separator is a compile error; a bare separator is not', () => { + // @ts-expect-error — a separator admits only `type`, `id` and `order`. + const labelled: NavigationItem = { id: 'sep', type: 'separator', label: '' }; + const bare: NavigationItem = { id: 'sep', type: 'separator', order: 3 }; + expect(labelled.type).toBe(bare.type); + }); + + it('CONTROL — every other item type still requires its `label`', () => { + // @ts-expect-error — an entry without a label. + const unlabelled: NavigationItem = { id: 'account', type: 'object', objectName: 'account' }; + expect(unlabelled.type).toBe('object'); + }); +}); + +describe('objectui#10867 — the wizard draft carries no `layout`', () => { + it('`layout` is not a key of `AppWizardDraft`', () => { + const noLayout = false satisfies 'layout' extends keyof AppWizardDraft ? true : false; + // @ts-expect-error — the Layout control persisted nothing and was removed with this member. + const withLayout: AppWizardDraft = { ...DRAFT, layout: 'sidebar' }; + expect(noLayout).toBe(false); + expect(withLayout.name).toBe('acme_crm'); + }); +}); diff --git a/packages/types/src/__tests__/navigation-model.test.ts b/packages/types/src/__tests__/navigation-model.test.ts index c87d4f7a8d..2bb4ff27e7 100644 --- a/packages/types/src/__tests__/navigation-model.test.ts +++ b/packages/types/src/__tests__/navigation-model.test.ts @@ -114,7 +114,6 @@ describe('NavigationItem Zod Schema', () => { const item = { id: 'sep_1', type: 'separator', - label: '', }; const result = NavigationItemSchema.safeParse(item); expect(result.success).toBe(true); @@ -344,8 +343,8 @@ describe('menuItemToNavigationItem', () => { const menuItem: AppMenuItem = { type: 'separator' }; const result = menuItemToNavigationItem(menuItem, 3); - expect(result.type).toBe('separator'); - expect(result.label).toBe(''); + // Only the spec separator's keys — no `label` (objectui#10867). + expect(result).toEqual({ id: 'migrated_3', type: 'separator' }); }); it('should invert hidden to visible', () => { diff --git a/packages/types/src/__tests__/navigation-spec-parity.test.ts b/packages/types/src/__tests__/navigation-spec-parity.test.ts index b4f30a4ab6..1798c412fe 100644 --- a/packages/types/src/__tests__/navigation-spec-parity.test.ts +++ b/packages/types/src/__tests__/navigation-spec-parity.test.ts @@ -163,7 +163,7 @@ describe('referencing the spec NavigationItemSchema would reject metadata object ['visible: boolean', { id: 'ai', type: 'url', label: 'AI', url: '/ai', visible: true }, 'menuItemToNavigationItem MANUFACTURES one when it inverts AppMenuItem.hidden'], ['separator label', { type: 'separator', label: 'Section' }, - 'menuItemToNavigationItem emits one; the spec separator declares only id/order'], + 'the flat mirror declares label for every type; the TS face refuses it on a separator since objectui#10867, and the spec separator declares only id/order'], ['single-character id', { id: 'a', type: 'url', label: 'A', url: '/a' }, 'objectui requires only a non-empty id; the spec requires two characters'], ])('the spec rejects %s (%s)', (_name, input, _why) => { diff --git a/packages/types/src/__tests__/spec-derived-unions.test.ts b/packages/types/src/__tests__/spec-derived-unions.test.ts index 7b7b89302d..7b7f076da9 100644 --- a/packages/types/src/__tests__/spec-derived-unions.test.ts +++ b/packages/types/src/__tests__/spec-derived-unions.test.ts @@ -195,8 +195,10 @@ const _validationErrorShape: ValidationError = { field: 'name', message: 'requir // pinned in `report-chart-query-spec-parity.test.ts`. // - `NavigationItem` — upstream IS precise now (`IsAny` and `IsUnknown` both // `false`), so #4171 really did land. Binding is still -// wrong: the three semantic blockers pinned below are -// unaffected by it. `any` was never the only blocker — +// wrong: the semantic blockers pinned below are +// unaffected by it (three when this was written; +// objectui#10867 lifted the separator `label`, by a +// local change, not an upstream one). `any` was never the only blocker — // #3177 established that, and it still holds. // - `NavigationItemSchema`— upstream IS precise now; pinned below. The live // blocker is SHAPE, and it is a RUNTIME one: see @@ -256,7 +258,10 @@ type KeysOfUnion = T extends unknown ? keyof T : never; type SpecNavDeclares = K extends KeysOfUnion | KeysOfUnion ? true : false; -// ── NavigationItem: the three blockers, none of which `any` ever caused ────── +// ── NavigationItem: the blockers, none of which `any` ever caused ──────────── +// +// Three were pinned here; objectui#10867 lifted the third (the separator +// `label`), which now stands as an agreement pin in its place. // // Umbrella verdict: still not bindable. The lines under it say why, and are the // ones to act on — this one stays `false` while ANY blocker remains. @@ -284,13 +289,32 @@ const _specNavVisibleStillRejectsBoolean = false satisfies boolean extends SpecN const _specNavStillHasNoPinned = false satisfies SpecNavDeclares<'pinned'>; const _specNavStillHasNoDefaultOpen = false satisfies SpecNavDeclares<'defaultOpen'>; -// 3. objectui's separator carries a `label`; the spec's separator branch -// declares only `type` / `id?` / `order?`. `menuItemToNavigationItem` emits -// one (measured: TS2353), so this is load-bearing, not decorative. -const _specSeparatorStillHasNoLabel = false satisfies 'label' extends keyof Extract< - SpecNavigationItem, - { type: 'separator' } -> +// 3. LIFTED by objectui#10867. objectui's separator used to carry a `label`, +// which the spec's separator branch (`type` / `id?` / `order?`) does not +// declare, and the Studio wizard saved one that the platform's `AppSchema` +// refused (`unrecognized_keys`). `NavigationItem` is now a union whose +// separator arm admits exactly the spec separator's keys, so what stood here +// as a blocker is asserted as an AGREEMENT instead. It fails the day either +// side moves: a key the spec adds to its separator, or one this arm admits +// that the spec does not declare. +// +// "Admits" = a key whose type is not `never`. The arm carries every other +// entry key as `?: never`, refused by name at compile, so a plain `keyof` +// would count those too. +type AdmittedKeys = { + [K in keyof T]-?: [Exclude] extends [never] ? never : K; +}[keyof T]; +type LocalSeparator = Extract; +type SpecSeparator = Extract; +type SpecSeparatorInput = Extract; +type SameKeys = [A] extends [B] ? ([B] extends [A] ? true : false) : false; +const _separatorAdmitsTheSpecKeys = true satisfies SameKeys, keyof SpecSeparator>; +const _separatorAdmitsTheSpecInputKeys = true satisfies SameKeys< + AdmittedKeys, + keyof SpecSeparatorInput +>; +// ...and every separator this arm admits is one the spec's accepts, at both tiers. +const _localSeparatorIsSpecValid = true satisfies [LocalSeparator] extends [SpecSeparator & SpecSeparatorInput] ? true : false; @@ -369,7 +393,8 @@ void _breakpointCovers; void _importModeCovers; void _importStatusCovers; void _validationErrorShape; void _localNavIsNotYetTheSpecUnion; void _specNavVisibleStillRejectsBoolean; void _specNavStillHasNoPinned; void _specNavStillHasNoDefaultOpen; -void _specSeparatorStillHasNoLabel; void _navTypeCoversSpec; +void _separatorAdmitsTheSpecKeys; void _separatorAdmitsTheSpecInputKeys; +void _localSeparatorIsSpecValid; void _navTypeCoversSpec; void _specNavSchemaIsNoLongerAny; void _specFormFieldIsNoLongerAny; void _specFormFieldStillHasNoName; void _specFormFieldInputStillHasNoName; void _specFieldSlotIsStillAName; diff --git a/packages/types/src/app.ts b/packages/types/src/app.ts index 0a6753d4dc..0b89cbc52c 100644 --- a/packages/types/src/app.ts +++ b/packages/types/src/app.ts @@ -35,14 +35,18 @@ // a boolean), `pinned` (backs `useNavPins` + `FavoritesProvider`), and the // legacy `defaultOpen` spelling. Binding deletes all three from the type // while their implementations keep running. -// 3. A separator carrying a `label`; the spec's separator branch declares none. +// 3. CLOSED by objectui#10867: a separator carried a `label`, which the spec's +// separator branch does not declare. `NavigationItem` is now a union whose +// separator arm ({@link NavigationSeparatorItem}) admits exactly the +// spec separator's keys — `type`, `id` and `order`. // // So the symbol stays local and the KEYS come off the spec one by one — the // `badgeVariant` precedent (objectstack#4115), widened here to every key with a // precise spec counterpart. That is the part of the burn-down that is safe // today: a restated enum or payload shape can drift, and now cannot. -// `spec-derived-unions.test.ts` pins the three blockers above, each written so -// it fails the day the spec closes it. +// `spec-derived-unions.test.ts` pins the blockers that remain, each written so +// it fails the day the spec closes it, and asserts the separator agreement in +// place of the blocker it replaced. import type { I18nLabel, NavigationArea as SpecNavigationArea, @@ -78,21 +82,21 @@ import type { APP_SPEC_EXCLUDED, AppContextSelectorSchema } from './zod/app.zod. export type NavigationItemType = SpecNavigationItem['type']; /** - * Unified Navigation Item - * - * The single navigation primitive used across ObjectUI and @objectstack/spec. - * Replaces the legacy `AppMenuItem` for application navigation trees. - * + * A navigation ENTRY — every {@link NavigationItem} except a separator. + * * Supports typed navigation targets (object, dashboard, page, report, url), * nested groups, visibility expressions, RBAC permissions, and UX enhancements * like badges, pinning, and sort ordering. + * + * `type` excludes `'separator'`, so a separator can only be written through + * {@link NavigationSeparatorItem} (objectui#10867). */ -export interface NavigationItem { +export interface NavigationEntryItem { /** Unique identifier */ id: string; - /** Navigation item type */ - type: NavigationItemType; + /** Navigation item type — any spec nav type except `'separator'`. */ + type: Exclude; /** Display label (plain string per @objectstack/spec v4 protocol) */ label: string; @@ -312,6 +316,44 @@ export interface NavigationItem { order?: number; } +/** + * A navigation SEPARATOR — a rule between entries, not an entry + * (objectui#10867). + * + * It admits exactly the keys `@objectstack/spec`'s strict separator branch + * declares: `type`, `id` and `order`. Every other {@link NavigationEntryItem} + * key is `?: never` here, so writing one — a `label` above all — is a compile + * error rather than a document the platform's `AppSchema` refuses at save + * (`unrecognized_keys`). The `never` keys are DERIVED from + * `NavigationEntryItem`, so a key added there is refused here without an edit. + * + * Reading one of them off an unnarrowed {@link NavigationItem} still compiles + * and answers `undefined` on this arm, which is also what the object holds. + * + * `id` stays required, as on every objectui navigation item (the spec makes it + * optional); a required `id` is still spec-valid. The agreement with the spec's + * separator is asserted in `__tests__/spec-derived-unions.test.ts`. + */ +export type NavigationSeparatorItem = Pick & { + /** The separator discriminant. */ + type: 'separator'; +} & { + [K in Exclude]?: never; +}; + +/** + * Unified Navigation Item + * + * The single navigation primitive used across ObjectUI and @objectstack/spec. + * Replaces the legacy `AppMenuItem` for application navigation trees. + * + * A union of two arms, discriminated by `type`: a {@link NavigationEntryItem} + * (every spec nav type but `'separator'`) and a {@link NavigationSeparatorItem} + * (objectui#10867). Narrow on `item.type === 'separator'` before relying on an + * entry's `label`. + */ +export type NavigationItem = NavigationEntryItem | NavigationSeparatorItem; + /** * Navigation Area — a business-domain partition of navigation items. * @@ -349,9 +391,9 @@ export interface NavigationItem { * - `navigation` — objectui's own {@link NavigationItem}, not the spec's. * Spec 17.0.0-rc.1 gave the spec's item a real type, so this is no longer * the `any` erasure objectstack#4171 was filed about — and it is still not - * bindable, for the three reasons the module header above records - * (`visible: boolean`, `pinned` / `defaultOpen`, a separator carrying a - * `label`). Precision is not equivalence: this is case 2c in the guard's + * bindable, for the reasons the module header above records + * (`visible: boolean`, `pinned` / `defaultOpen`; the separator `label` was + * closed by objectui#10867). Precision is not equivalence: this is case 2c in the guard's * header, and the umbrella verdict lives with the element type in * `__tests__/spec-derived-unions.test.ts`, which is where the blockers are * pinned one by one. @@ -711,7 +753,8 @@ export interface AppMenuItem { * Mapping rules: * - `type: 'item'` → inferred from `href` (url) or `path` (page) * - `type: 'group'` → `type: 'group'` - * - `type: 'separator'` → `type: 'separator'` + * - `type: 'separator'` → `type: 'separator'` (its `label` is dropped: the spec + * separator declares none, objectui#10867) * - `hidden` → `visible` (inverted) * - `path` → `pageName` (last segment) or kept as-is for url * - `href` → `url` with `target: '_blank'` @@ -723,11 +766,9 @@ export function menuItemToNavigationItem( const id = `migrated_${index}`; if (item.type === 'separator') { - return { - id, - type: 'separator', - label: item.label || '', - }; + // The spec's separator declares no `label` (objectui#10867); a legacy + // separator's label has no place to go. + return { id, type: 'separator' }; } if (item.type === 'group') { @@ -857,8 +898,9 @@ export interface AppWizardDraft { /** Template to start from */ template?: string; - /** Layout strategy */ - layout: 'sidebar' | 'header' | 'empty'; + // No `layout` (objectui#10867): `@objectstack/spec`'s `AppSchema` declares no + // app layout, no console surface reads one and nothing stores one, so the + // wizard's Layout control persisted nothing and was removed with this member. /** Selected business objects */ objects: ObjectSelection[]; @@ -895,8 +937,10 @@ export function isValidAppName(name: string): boolean { * - the logo and favicon travel in `branding` only (objectui#10827); * - no `type`: that is the renderer-node discriminator of * {@link AppComponentSchema}, not a key of the stored app; - * - no `layout`: the spec declares no app layout, so the draft's layout - * choice is not part of the saved document. + * - no `layout`: the spec declares no app layout, and the draft carries + * none either (objectui#10867 removed the wizard's Layout control); + * - a separator in `navigation` carries only `type`, `id` and `order`, the + * spec separator's keys ({@link NavigationSeparatorItem}, objectui#10867). * The pin is `__tests__/app-declared-keys-10842.test.ts`, which parses this * output with the spec's own `AppSchema`. */ diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index b5aad8ff13..c1e395330e 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -53,6 +53,8 @@ export type { AppComponentSchema, NavigationItem, + NavigationEntryItem, + NavigationSeparatorItem, NavigationItemType, NavigationArea, AppMenuItem, diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index 9688a06741..d028124d48 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -608,11 +608,12 @@ const ALLOW = { "Precise upstream, still not bindable — guard header case 2c. objectstack#4171 landed and " + "the spec's NavigationItem is neither `any` nor `unknown` any more, so `no longer any` is " + "settled and is NOT a licence to bind. The spec models navigation as a nine-variant " + - "discriminated union; objectui keeps one flat shape carrying `visible: boolean` (the spec " + + "discriminated union; objectui keeps a flat entry shape carrying `visible: boolean` (the spec " + "takes a CEL string / Expression envelope, and `menuItemToNavigationItem` MANUFACTURES a " + "boolean when it inverts legacy `MenuItem.hidden`), plus `pinned` (`useNavPins` + " + - "`FavoritesProvider`) and `defaultOpen`, neither of which the spec declares at either tier, " + - "plus a separator carrying `label`. Each blocker is pinned one-per-line in " + + "`FavoritesProvider`) and `defaultOpen`, neither of which the spec declares at either tier. " + + "Its separator arm agrees with the spec's since objectui#10867, which removed the " + + "separator `label`. Each remaining blocker is pinned one-per-line in " + "packages/types/src/__tests__/spec-derived-unions.test.ts, written to stop compiling the day " + "that specific blocker lifts. What IS derivable is already derived (`NavigationItemType` and " + "the per-branch keys come off the spec).", From 920fe8f9d7ab0754c2117cde74e6d4915bd94f1f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:28:56 +0000 Subject: [PATCH 2/6] chore(changeset): objectui#10867 changeset, and dated notes on the 10842 and 3162 entries The new changeset declares @object-ui/types minor (BREAKING authoring banner) and @object-ui/plugin-designer and @object-ui/i18n patch. The 10842 entry's "the wizard's layout choice is not saved" and the 3162 entry's "three semantic blockers" get dated notes; both frontmatters are byte-identical. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .changeset/10842-wizard-app-declared-keys.md | 7 +++++++ .changeset/10867-app-wizard-saves-spec-app.md | 21 +++++++++++++++++++ .../3162-types-ledger-batch8-verdicts.md | 6 ++++++ 3 files changed, 34 insertions(+) create mode 100644 .changeset/10867-app-wizard-saves-spec-app.md diff --git a/.changeset/10842-wizard-app-declared-keys.md b/.changeset/10842-wizard-app-declared-keys.md index c326e649b8..286cab5d26 100644 --- a/.changeset/10842-wizard-app-declared-keys.md +++ b/.changeset/10842-wizard-app-declared-keys.md @@ -33,3 +33,10 @@ has no app layout, so the wizard's layout choice is not saved. `logo`, `favicon` or `title`; it reads `branding.logo`, `branding.favicon` and `label`. Migration: move a top-level `favicon` URL to `branding: { favicon: '…' }`. + +⚠️ **Dated note, 2026-09-28 — the wizard has no layout choice, and a separator no longer blocks the save — objectui#10867.** +Later in this same release the wizard's Layout control was removed, with `AppWizardDraft.layout`, so there is no +layout choice left to not save. "Creating an app is no longer refused" did not yet hold for an app with a +navigation separator: the wizard wrote one with a `label`, which the spec's separator does not declare, and that +save was still refused until objectui#10867 made the separator carry only `type`, `id` and `order`. The rest of +this entry still holds. diff --git a/.changeset/10867-app-wizard-saves-spec-app.md b/.changeset/10867-app-wizard-saves-spec-app.md new file mode 100644 index 0000000000..6439e9c0a2 --- /dev/null +++ b/.changeset/10867-app-wizard-saves-spec-app.md @@ -0,0 +1,21 @@ +--- +'@object-ui/types': minor +'@object-ui/plugin-designer': patch +'@object-ui/i18n': patch +--- + +fix(types,plugin-designer)!: the Studio app wizard saves a document the platform accepts, and an edit keeps the stored `accentColor` (objectui#10867) + +⚠️ **BREAKING (authoring)**, marked `minor` under this repository's version-alignment rule (a `major` in the fixed group would move all of it off the `@objectstack` major). Two published `@object-ui/types` members narrow: a navigation separator no longer takes a `label`, and `AppWizardDraft.layout` is removed. A TypeScript literal that writes either no longer compiles. + +**Clause-②: yes (narrowing)** — the separator arm of `NavigationItem` loses `label`, and `AppWizardDraft` loses `layout`. + +- **A separator carries only `type`, `id` and `order`.** `@objectstack/spec`'s separator branch declares exactly those keys, and its `AppSchema` refuses anything else. `NavigationItem` required a `label` on every item, so the wizard's "Add separator" wrote `{ id, type: 'separator', label: '' }`, and the console's create-app and edit-app saves were refused with `422 INVALID_METADATA` (`unrecognized_keys` `['label']` at `navigation.N`). `NavigationItem` is now a union of two arms, discriminated by `type`. `NavigationEntryItem` holds every other nav type and keeps its required `label`. `NavigationSeparatorItem` admits `type`, `id` and `order`, and every other entry key is `?: never` on it. Both arms are exported. Reading an entry-only key off an unnarrowed item still compiles and answers `undefined` on the separator arm. Narrow on `item.type === 'separator'` before relying on `label`. `menuItemToNavigationItem` maps a legacy separator to `{ id, type: 'separator' }` and drops its label. `spec-derived-unions.test.ts` no longer pins the separator `label` as a blocker. It asserts, at both spec tiers, that the separator arm admits the spec separator's keys and no others. +- **`@object-ui/plugin-designer`: the wizard and `NavigationDesigner` write a separator as `{ id, type }`.** +- **`@object-ui/plugin-designer`: `EditAppPage` keeps the stored branding.** The wizard maintains the logo, primary colour and favicon, and its `branding` replaced the stored block, so a stored `accentColor` was dropped on every edit. The console reads that key. The save now keeps every stored `branding` key the spec's `AppBrandingSchema` declares, read from that schema. The wizard's values win for the keys it maintains. A stored key the spec does not declare is still left out. +- **The wizard's Layout control is removed, with `AppWizardDraft.layout`.** The spec declares no app `layout`, no console surface reads one, and since objectui#10842 the save wrote none. The control persisted nothing. `EditAppPage` no longer reads a stored `layout` into the draft. The Basic Info step's description now reads "Name, title, and icon". +- **`@object-ui/i18n`:** the four `appDesigner` layout keys (`layout`, `layoutSidebar`, `layoutHeader` and `layoutEmpty`) are removed from all ten packs, and `appDesigner.stepBasicDesc` no longer names a layout. + +**Migration:** write a separator as `{ id, type: 'separator' }`, with an optional `order`. Remove `layout` from any `AppWizardDraft` you build. `AppComponentSchema.layout`, the renderer node's own layout strategy, is a different member and is unchanged. + +Pinned in `packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts` and `packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx`. diff --git a/.changeset/3162-types-ledger-batch8-verdicts.md b/.changeset/3162-types-ledger-batch8-verdicts.md index d578606c34..fb757ca76f 100644 --- a/.changeset/3162-types-ledger-batch8-verdicts.md +++ b/.changeset/3162-types-ledger-batch8-verdicts.md @@ -31,3 +31,9 @@ Two stale justifications were corrected in passing, both the same defect class t exists to catch — a comment asserting an upstream type "erases to `any`" when it no longer does. Left alone, the next triage checks the claim, finds it false, and lands the regression. No published behaviour changes; no runtime code was touched. + +⚠️ **Dated note, 2026-09-28 — one of the three `NavigationItem` blockers has since lifted — objectui#10867.** +Later in this same release the separator `label` stopped being a blocker: `NavigationItem` became a union whose +separator arm admits exactly the spec separator's keys, and `spec-derived-unions.test.ts` pins that agreement where +the blocker stood. Two semantic blockers remain (`visible: boolean`, and `pinned` / `defaultOpen`), so the symbol +is still not bindable. The rest of this entry still holds. From ce4df73f6cd47524759d666110b169047cdc7750 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 02:52:13 +0000 Subject: [PATCH 3/6] fix(layout,plugin-designer): narrow on the separator arm where an entry-only key is read or written - layout: resolveNavItemLabel answers '' for a separator; the mobile bottom nav's leaf list is typed as entries; collectPinnedItems checks the type before `pinned`. - plugin-designer: NavigationDesigner's label / icon / visibility patchers skip a separator. - layout tests: fixtures that spread an entry and add `visible` or `requiredPermissions` are typed as NavigationEntryItem. - changeset: @object-ui/layout patch, and the migration line for spreads. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .changeset/10867-app-wizard-saves-spec-app.md | 6 +++-- packages/layout/src/AppSchemaRenderer.tsx | 8 +++--- packages/layout/src/NavigationRenderer.tsx | 4 ++- .../src/__tests__/AppSchemaRenderer.test.tsx | 27 ++++++++++--------- .../__tests__/resolveHref.runAction.test.ts | 4 +-- .../layout/src/__tests__/resolveHref.test.ts | 6 ++--- .../src/NavigationDesigner.tsx | 9 ++++--- 7 files changed, 36 insertions(+), 28 deletions(-) diff --git a/.changeset/10867-app-wizard-saves-spec-app.md b/.changeset/10867-app-wizard-saves-spec-app.md index 6439e9c0a2..1db69d27c9 100644 --- a/.changeset/10867-app-wizard-saves-spec-app.md +++ b/.changeset/10867-app-wizard-saves-spec-app.md @@ -2,6 +2,7 @@ '@object-ui/types': minor '@object-ui/plugin-designer': patch '@object-ui/i18n': patch +'@object-ui/layout': patch --- fix(types,plugin-designer)!: the Studio app wizard saves a document the platform accepts, and an edit keeps the stored `accentColor` (objectui#10867) @@ -11,11 +12,12 @@ fix(types,plugin-designer)!: the Studio app wizard saves a document the platform **Clause-②: yes (narrowing)** — the separator arm of `NavigationItem` loses `label`, and `AppWizardDraft` loses `layout`. - **A separator carries only `type`, `id` and `order`.** `@objectstack/spec`'s separator branch declares exactly those keys, and its `AppSchema` refuses anything else. `NavigationItem` required a `label` on every item, so the wizard's "Add separator" wrote `{ id, type: 'separator', label: '' }`, and the console's create-app and edit-app saves were refused with `422 INVALID_METADATA` (`unrecognized_keys` `['label']` at `navigation.N`). `NavigationItem` is now a union of two arms, discriminated by `type`. `NavigationEntryItem` holds every other nav type and keeps its required `label`. `NavigationSeparatorItem` admits `type`, `id` and `order`, and every other entry key is `?: never` on it. Both arms are exported. Reading an entry-only key off an unnarrowed item still compiles and answers `undefined` on the separator arm. Narrow on `item.type === 'separator'` before relying on `label`. `menuItemToNavigationItem` maps a legacy separator to `{ id, type: 'separator' }` and drops its label. `spec-derived-unions.test.ts` no longer pins the separator `label` as a blocker. It asserts, at both spec tiers, that the separator arm admits the spec separator's keys and no others. -- **`@object-ui/plugin-designer`: the wizard and `NavigationDesigner` write a separator as `{ id, type }`.** +- **`@object-ui/plugin-designer`: the wizard and `NavigationDesigner` write a separator as `{ id, type }`.** `NavigationDesigner` no longer writes a label, icon or visibility onto a separator. +- **`@object-ui/layout` narrows on the separator arm; nothing it renders changes.** `resolveNavItemLabel` answers `''` for a separator, which is what a separator's `label: ''` resolved to before. The mobile bottom nav's leaf list, which already skipped separators, is now typed as entries. - **`@object-ui/plugin-designer`: `EditAppPage` keeps the stored branding.** The wizard maintains the logo, primary colour and favicon, and its `branding` replaced the stored block, so a stored `accentColor` was dropped on every edit. The console reads that key. The save now keeps every stored `branding` key the spec's `AppBrandingSchema` declares, read from that schema. The wizard's values win for the keys it maintains. A stored key the spec does not declare is still left out. - **The wizard's Layout control is removed, with `AppWizardDraft.layout`.** The spec declares no app `layout`, no console surface reads one, and since objectui#10842 the save wrote none. The control persisted nothing. `EditAppPage` no longer reads a stored `layout` into the draft. The Basic Info step's description now reads "Name, title, and icon". - **`@object-ui/i18n`:** the four `appDesigner` layout keys (`layout`, `layoutSidebar`, `layoutHeader` and `layoutEmpty`) are removed from all ten packs, and `appDesigner.stepBasicDesc` no longer names a layout. -**Migration:** write a separator as `{ id, type: 'separator' }`, with an optional `order`. Remove `layout` from any `AppWizardDraft` you build. `AppComponentSchema.layout`, the renderer node's own layout strategy, is a different member and is unchanged. +**Migration:** write a separator as `{ id, type: 'separator' }`, with an optional `order`. Remove `layout` from any `AppWizardDraft` you build. Code that spreads an entry-only key (`label`, `visible`, `requiredPermissions` and the rest) onto a value typed `NavigationItem` narrows it first (`item.type !== 'separator'`) or types it `NavigationEntryItem`: the separator arm refuses those keys. `AppComponentSchema.layout`, the renderer node's own layout strategy, is a different member and is unchanged. Pinned in `packages/types/src/__tests__/app-wizard-separator-layout-10867.test.ts` and `packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx`. diff --git a/packages/layout/src/AppSchemaRenderer.tsx b/packages/layout/src/AppSchemaRenderer.tsx index d2a8b14401..f27c662530 100644 --- a/packages/layout/src/AppSchemaRenderer.tsx +++ b/packages/layout/src/AppSchemaRenderer.tsx @@ -35,7 +35,7 @@ import { SidebarGroupContent, SidebarInput, } from '@object-ui/components'; -import type { AppComponentSchema, NavigationItem, NavigationArea } from '@object-ui/types'; +import type { AppComponentSchema, NavigationItem, NavigationEntryItem, NavigationArea } from '@object-ui/types'; import { menuItemToNavigationItem } from '@object-ui/types'; // Aliased on import, following PR #4169's convention: this repo has its OWN // `resolveI18nLabel` over a DIFFERENT vocabulary, and neither accepts the @@ -267,8 +267,10 @@ function MobileBottomNav({ // Show up to 5 non-group leaf items. Flatten group children so apps that // organise navigation into groups (e.g. Setup → Overview / Administration / // …) still surface real links in the mobile bottom nav. - const collectLeaves = (list: typeof items): typeof items => { - const out: typeof items = []; + // Separators are skipped, so what comes back is entries only — each carries + // the `label` the bottom nav draws (objectui#10867). + const collectLeaves = (list: NavigationItem[]): NavigationEntryItem[] => { + const out: NavigationEntryItem[] = []; for (const item of list) { if (item.type === 'separator') continue; if (item.type === 'group') { diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index bda3399c5f..3a90108f7e 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -308,6 +308,8 @@ export function resolveNavItemLabel( dashboardResolver?: (dashboardName: string, fallbackLabel: string) => string, viewResolver?: (objectName: string, viewName: string, fallbackLabel: string) => string, ): string { + // A separator carries no `label` (objectui#10867): there is nothing to name. + if (item.type === 'separator') return ''; const base = resolveLabel(item.label, t); // Only apply convention-based resolution for items with plain string labels. // I18nLabel objects (with explicit key/defaultValue) already have their own translation keys. @@ -1514,7 +1516,7 @@ function collectPinnedItems( // of a "Favorites" heading over an empty list. if (!passesNavItemGuards(item, options)) continue; - if (item.pinned && item.type !== 'group' && item.type !== 'separator') { + if (item.type !== 'group' && item.type !== 'separator' && item.pinned) { pinned.push(item); } if (item.children?.length) { diff --git a/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx b/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx index 6d77a7a37e..f9a68eeb88 100644 --- a/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx +++ b/packages/layout/src/__tests__/AppSchemaRenderer.test.tsx @@ -10,7 +10,7 @@ import { describe, it, expect, vi } from 'vitest'; import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; -import type { AppComponentSchema, NavigationItem, NavigationArea } from '@object-ui/types'; +import type { AppComponentSchema, NavigationItem, NavigationEntryItem, NavigationArea } from '@object-ui/types'; import { AppSchemaRenderer } from '../AppSchemaRenderer'; import { hasVisibleNavigationItems } from '../NavigationRenderer'; @@ -53,22 +53,23 @@ const schemaWithNav: AppComponentSchema = { navigation: navItems, }; +// Typed as ENTRIES, so a case can spread one and add an entry-only key +// (`visible`, `requiredPermissions`); a separator carries neither (objectui#10867). +const salesEntry: NavigationEntryItem = { id: 'a1', type: 'object', label: 'Opportunities', icon: 'Target', objectName: 'opportunity' }; +const serviceEntry: NavigationEntryItem = { id: 'a2', type: 'object', label: 'Cases', icon: 'Inbox', objectName: 'case' }; + const salesArea: NavigationArea = { id: 'area-sales', label: 'Sales', icon: 'Briefcase', - navigation: [ - { id: 'a1', type: 'object', label: 'Opportunities', icon: 'Target', objectName: 'opportunity' }, - ], + navigation: [salesEntry], }; const serviceArea: NavigationArea = { id: 'area-service', label: 'Service', icon: 'Headphones', - navigation: [ - { id: 'a2', type: 'object', label: 'Cases', icon: 'Inbox', objectName: 'case' }, - ], + navigation: [serviceEntry], }; const marketingArea: NavigationArea = { @@ -223,7 +224,7 @@ describe('AppSchemaRenderer', () => { { ...salesArea, navigation: [ - { ...salesArea.navigation[0], visible: false }, + { ...salesEntry, visible: false }, { id: 'a1b', type: 'object', label: 'Quotes', objectName: 'quote' }, ], }, @@ -250,7 +251,7 @@ describe('AppSchemaRenderer', () => { { ...salesArea, navigation: [ - { ...salesArea.navigation[0], requiredPermissions: ['sales:admin'] }, + { ...salesEntry, requiredPermissions: ['sales:admin'] }, { id: 'a1b', type: 'object', label: 'Quotes', objectName: 'quote' }, ], }, @@ -278,7 +279,7 @@ describe('AppSchemaRenderer', () => { describe('derived area visibility (#3311)', () => { const gatedSales: NavigationArea = { ...salesArea, - navigation: [{ ...salesArea.navigation[0], visible: false }], + navigation: [{ ...salesEntry, visible: false }], }; it('keeps every area in the switcher when every area has a visible item', () => { @@ -309,7 +310,7 @@ describe('AppSchemaRenderer', () => { const adminSales: NavigationArea = { ...salesArea, navigation: [ - { ...salesArea.navigation[0], requiredPermissions: ['sales:admin'] }, + { ...salesEntry, requiredPermissions: ['sales:admin'] }, ], }; renderApp( @@ -341,7 +342,7 @@ describe('AppSchemaRenderer', () => { title: 'CRM', areas: [ gatedSales, - { ...serviceArea, navigation: [{ ...serviceArea.navigation[0], visible: false }] }, + { ...serviceArea, navigation: [{ ...serviceEntry, visible: false }] }, ], }, { evaluateVisibility: (expr) => expr !== false }, @@ -394,7 +395,7 @@ describe('AppSchemaRenderer', () => { const adminSales: NavigationArea = { ...salesArea, navigation: [ - { ...salesArea.navigation[0], requiredPermissions: ['sales:admin'] }, + { ...salesEntry, requiredPermissions: ['sales:admin'] }, ], }; const schema: AppComponentSchema = { diff --git a/packages/layout/src/__tests__/resolveHref.runAction.test.ts b/packages/layout/src/__tests__/resolveHref.runAction.test.ts index 32532e6907..3ca22b2e46 100644 --- a/packages/layout/src/__tests__/resolveHref.runAction.test.ts +++ b/packages/layout/src/__tests__/resolveHref.runAction.test.ts @@ -24,12 +24,12 @@ */ import { describe, it, expect } from 'vitest'; -import type { NavigationItem } from '@object-ui/types'; +import type { NavigationItem, NavigationEntryItem } from '@object-ui/types'; import { resolveHref, resolveActiveNavItem, NAV_RUN_ACTION_PARAM } from '../NavigationRenderer'; const BASE = '/apps/cloud_control'; -function objectItem(extra: Partial = {}): NavigationItem { +function objectItem(extra: Partial = {}): NavigationItem { return { id: 'nav_env', type: 'object', label: 'Environments', objectName: 'sys_environment', ...extra }; } diff --git a/packages/layout/src/__tests__/resolveHref.test.ts b/packages/layout/src/__tests__/resolveHref.test.ts index a96c18ccd1..ea00fa4cec 100644 --- a/packages/layout/src/__tests__/resolveHref.test.ts +++ b/packages/layout/src/__tests__/resolveHref.test.ts @@ -13,12 +13,12 @@ */ import { describe, it, expect } from 'vitest'; -import type { NavigationItem } from '@object-ui/types'; +import type { NavigationItem, NavigationEntryItem } from '@object-ui/types'; import { resolveHref } from '../NavigationRenderer'; const BASE = '/apps/crm'; -function objectItem(extra: Partial = {}): NavigationItem { +function objectItem(extra: Partial = {}): NavigationItem { return { id: 'nav_task', type: 'object', label: 'Tasks', objectName: 'task', ...extra }; } @@ -219,7 +219,7 @@ describe('resolveActiveNavItem — single winner across the tree', () => { // ============================================================================ describe('resolveHref — component targets', () => { - function componentItem(extra: Partial = {}): NavigationItem { + function componentItem(extra: Partial = {}): NavigationItem { return { id: 'nav_comp', type: 'component', label: 'Comp', ...extra }; } diff --git a/packages/plugin-designer/src/NavigationDesigner.tsx b/packages/plugin-designer/src/NavigationDesigner.tsx index aaf6822372..50af81db31 100644 --- a/packages/plugin-designer/src/NavigationDesigner.tsx +++ b/packages/plugin-designer/src/NavigationDesigner.tsx @@ -290,7 +290,7 @@ function NavItemRow({ !readOnly && 'cursor-text' )} onDoubleClick={() => { - if (!readOnly && item.type !== 'separator') { + if (!readOnly) { setLabelDraft(resolveKeyedI18nLabel(item.label) ?? ''); setEditingLabel(true); } @@ -516,7 +516,8 @@ export function NavigationDesigner({ (id: string, label: string) => { function update(list: NavigationItem[]): NavigationItem[] { return list.map((item) => { - if (item.id === id) return { ...item, label }; + // A separator carries no label, icon or visibility (objectui#10867). + if (item.id === id && item.type !== 'separator') return { ...item, label }; if (item.children) return { ...item, children: update(item.children) }; return item; }); @@ -530,7 +531,7 @@ export function NavigationDesigner({ (id: string, icon: string) => { function update(list: NavigationItem[]): NavigationItem[] { return list.map((item) => { - if (item.id === id) return { ...item, icon: icon || undefined }; + if (item.id === id && item.type !== 'separator') return { ...item, icon: icon || undefined }; if (item.children) return { ...item, children: update(item.children) }; return item; }); @@ -544,7 +545,7 @@ export function NavigationDesigner({ (id: string) => { function update(list: NavigationItem[]): NavigationItem[] { return list.map((item) => { - if (item.id === id) { + if (item.id === id && item.type !== 'separator') { return { ...item, visible: item.visible === false ? true : false }; } if (item.children) return { ...item, children: update(item.children) }; From fb03f093f69ee8fde09bfd37a8db16ccd9c8c009 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 03:11:36 +0000 Subject: [PATCH 4/6] fix(app-shell): narrow on the separator arm in the pins hook and the Studio nav walk - useNavPins: togglePin registers a favorite only for an entry (a separator is never pinnable and carries no label); applyPins leaves a separator as it is. - UnifiedSidebar: the Studio navigation walk returns a separator as it is, before rewriting children or the packages entry. - UnifiedSidebar.derivedAreaVisibility test: the spread fixtures are typed as NavigationEntryItem. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- packages/app-shell/src/hooks/useNavPins.ts | 5 ++++- .../app-shell/src/layout/UnifiedSidebar.tsx | 5 ++++- ...fiedSidebar.derivedAreaVisibility.test.tsx | 19 ++++++++++++------- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/app-shell/src/hooks/useNavPins.ts b/packages/app-shell/src/hooks/useNavPins.ts index 2ff3c38053..17f45dcc19 100644 --- a/packages/app-shell/src/hooks/useNavPins.ts +++ b/packages/app-shell/src/hooks/useNavPins.ts @@ -61,7 +61,8 @@ export function useNavPins() { // If a NavigationItem is provided, register/refresh the favorite with // proper label/href so backend sync carries portable data. Otherwise // just flip the flag — the existing favorite (if any) keeps its data. - if (item) { + // A separator is never pinnable and carries no `label` (objectui#10867). + if (item && item.type !== 'separator') { addFavorite({ id: favId, label: item.label, @@ -108,6 +109,8 @@ export function useNavPins() { let pinCount = 0; const walk = (list: NavigationItem[]): NavigationItem[] => list.map(item => { + // A separator carries no `pinned` and no children (objectui#10867). + if (item.type === 'separator') return item; const shouldPin = pinnedNavIds.has(item.id) && pinCount < MAX_PINS; if (shouldPin) pinCount++; const children = item.children?.length ? walk(item.children) : item.children; diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index 08eafebbd2..57360c3eea 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -410,8 +410,11 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { defaultValue: 'Package management', }); const walk = (items: NavigationItem[]): NavigationItem[] => - items.flatMap((item) => { + items.flatMap((item): NavigationItem[] => { if (isMetadataDirectoryItem(item)) return []; + // A separator has no children and is never the packages entry, and + // carries none of the keys rewritten below (objectui#10867). + if (item.type === 'separator') return [item]; const children = item.children?.length ? walk(item.children) : item.children; if (item.type === 'group' && children?.length === 0) return []; if (isPackagesItem(item)) { diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx index 58a0d97ce9..ba3e83ac89 100644 --- a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.derivedAreaVisibility.test.tsx @@ -21,7 +21,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; -import type { NavigationArea } from '@object-ui/types'; +import type { NavigationArea, NavigationEntryItem } from '@object-ui/types'; // --------------------------------------------------------------------------- // Mocks — providers and console-only chrome. @object-ui/components and @@ -117,16 +117,21 @@ import { UnifiedSidebar } from '../UnifiedSidebar'; // Fixtures — labels are pairwise distinct so a hit is unambiguous. // --------------------------------------------------------------------------- +// Typed as ENTRIES, so a case can spread one and add an entry-only key +// (`visible`, `requiredPermissions`); a separator carries neither (objectui#10867). +const salesEntry: NavigationEntryItem = { id: 'a1', type: 'object', label: 'Opportunities', objectName: 'opportunity' }; +const serviceEntry: NavigationEntryItem = { id: 'a2', type: 'object', label: 'Cases', objectName: 'case' }; + const salesArea: NavigationArea = { id: 'area-sales', label: 'Sales', - navigation: [{ id: 'a1', type: 'object', label: 'Opportunities', objectName: 'opportunity' }], + navigation: [salesEntry], }; const serviceArea: NavigationArea = { id: 'area-service', label: 'Service', - navigation: [{ id: 'a2', type: 'object', label: 'Cases', objectName: 'case' }], + navigation: [serviceEntry], }; const marketingArea: NavigationArea = { @@ -137,7 +142,7 @@ const marketingArea: NavigationArea = { const gatedSales: NavigationArea = { ...salesArea, - navigation: [{ ...salesArea.navigation[0], visible: false }], + navigation: [{ ...salesEntry, visible: false }], }; function sidebarUi(areas: NavigationArea[]) { @@ -183,7 +188,7 @@ describe('UnifiedSidebar derived area visibility (#3319)', () => { const partialSales: NavigationArea = { ...salesArea, navigation: [ - { ...salesArea.navigation[0], visible: false }, + { ...salesEntry, visible: false }, { id: 'a1b', type: 'object', label: 'Quotes', objectName: 'quote' }, ], }; @@ -217,7 +222,7 @@ describe('UnifiedSidebar derived area visibility (#3319)', () => { render( sidebarUi([ gatedSales, - { ...serviceArea, navigation: [{ ...serviceArea.navigation[0], visible: false }] }, + { ...serviceArea, navigation: [{ ...serviceEntry, visible: false }] }, ]), ); expect(screen.queryByText('Sales')).not.toBeInTheDocument(); @@ -245,7 +250,7 @@ describe('UnifiedSidebar derived area visibility (#3319)', () => { it('re-elects when the ACTIVE area is gated away, and a mere reveal does not steal the selection', () => { const adminSales: NavigationArea = { ...salesArea, - navigation: [{ ...salesArea.navigation[0], requiredPermissions: ['sales:admin'] }], + navigation: [{ ...salesEntry, requiredPermissions: ['sales:admin'] }], }; const areas = [adminSales, serviceArea, marketingArea]; From e2493c7967627e9f7b66d7481c75c863e054e1c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 03:18:14 +0000 Subject: [PATCH 5/6] chore(changeset): objectui#10867 names @object-ui/app-shell and its separator narrowing Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .changeset/10867-app-wizard-saves-spec-app.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/10867-app-wizard-saves-spec-app.md b/.changeset/10867-app-wizard-saves-spec-app.md index 1db69d27c9..40415f7dc2 100644 --- a/.changeset/10867-app-wizard-saves-spec-app.md +++ b/.changeset/10867-app-wizard-saves-spec-app.md @@ -3,6 +3,7 @@ '@object-ui/plugin-designer': patch '@object-ui/i18n': patch '@object-ui/layout': patch +'@object-ui/app-shell': patch --- fix(types,plugin-designer)!: the Studio app wizard saves a document the platform accepts, and an edit keeps the stored `accentColor` (objectui#10867) @@ -14,6 +15,7 @@ fix(types,plugin-designer)!: the Studio app wizard saves a document the platform - **A separator carries only `type`, `id` and `order`.** `@objectstack/spec`'s separator branch declares exactly those keys, and its `AppSchema` refuses anything else. `NavigationItem` required a `label` on every item, so the wizard's "Add separator" wrote `{ id, type: 'separator', label: '' }`, and the console's create-app and edit-app saves were refused with `422 INVALID_METADATA` (`unrecognized_keys` `['label']` at `navigation.N`). `NavigationItem` is now a union of two arms, discriminated by `type`. `NavigationEntryItem` holds every other nav type and keeps its required `label`. `NavigationSeparatorItem` admits `type`, `id` and `order`, and every other entry key is `?: never` on it. Both arms are exported. Reading an entry-only key off an unnarrowed item still compiles and answers `undefined` on the separator arm. Narrow on `item.type === 'separator'` before relying on `label`. `menuItemToNavigationItem` maps a legacy separator to `{ id, type: 'separator' }` and drops its label. `spec-derived-unions.test.ts` no longer pins the separator `label` as a blocker. It asserts, at both spec tiers, that the separator arm admits the spec separator's keys and no others. - **`@object-ui/plugin-designer`: the wizard and `NavigationDesigner` write a separator as `{ id, type }`.** `NavigationDesigner` no longer writes a label, icon or visibility onto a separator. - **`@object-ui/layout` narrows on the separator arm; nothing it renders changes.** `resolveNavItemLabel` answers `''` for a separator, which is what a separator's `label: ''` resolved to before. The mobile bottom nav's leaf list, which already skipped separators, is now typed as entries. +- **`@object-ui/app-shell` narrows the same way; nothing it renders changes.** `useNavPins` registers a favorite only for an entry and leaves a separator unpinned, and the Studio sidebar's navigation walk passes a separator through unchanged. - **`@object-ui/plugin-designer`: `EditAppPage` keeps the stored branding.** The wizard maintains the logo, primary colour and favicon, and its `branding` replaced the stored block, so a stored `accentColor` was dropped on every edit. The console reads that key. The save now keeps every stored `branding` key the spec's `AppBrandingSchema` declares, read from that schema. The wizard's values win for the keys it maintains. A stored key the spec does not declare is still left out. - **The wizard's Layout control is removed, with `AppWizardDraft.layout`.** The spec declares no app `layout`, no console surface reads one, and since objectui#10842 the save wrote none. The control persisted nothing. `EditAppPage` no longer reads a stored `layout` into the draft. The Basic Info step's description now reads "Name, title, and icon". - **`@object-ui/i18n`:** the four `appDesigner` layout keys (`layout`, `layoutSidebar`, `layoutHeader` and `layoutEmpty`) are removed from all ten packs, and `appDesigner.stepBasicDesc` no longer names a layout. From 2c19f3047f94d1efdecc54d425af30dcbbbff3ad Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 03:59:42 +0000 Subject: [PATCH 6/6] fix(types)!: the zod mirror refuses a separator key the spec separator does not declare; changeset corrections (objectui#10867 round 1) - NavigationItemSchema's separator branch refuses every key outside the spec separator's own set (type, id, order), read off the spec AppSchema's navigation union rather than restated, with a message that names the key to drop. objectui validate no longer passes a separator label the save door refuses. - navigation-spec-parity: the separator-label row leaves the divergence list and becomes an agreement assertion; the mirror doc and the NavigationItemSchema ledger reason count four divergences, not five. - New pins: nav-separator-mirror-refusal-10867 (safeValidateSchema, the strict face and the mirror refuse a separator label; a bare separator is accepted), and an EditAppPage row that a cleared logo saves as ''. - Changeset: @object-ui/i18n is minor; the banner names the union-alias, validate and raw-key consequences; the layout, reading, designer and pins wording is corrected. The 10842 dated note keeps only its layout half (frontmatter byte-identical). - spec-derived-unions: the spec-validity pin's comment says what it can and cannot see. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN --- .changeset/10842-wizard-app-declared-keys.md | 7 +- .changeset/10867-app-wizard-saves-spec-app.md | 12 +-- .../AppWizard.specDocument-10867.test.tsx | 10 +++ ...nav-separator-mirror-refusal-10867.test.ts | 79 +++++++++++++++++++ .../__tests__/navigation-spec-parity.test.ts | 11 ++- .../src/__tests__/spec-derived-unions.test.ts | 2 +- packages/types/src/zod/app.zod.ts | 69 ++++++++++++++-- scripts/check-spec-symbol-derivation.mjs | 7 +- 8 files changed, 175 insertions(+), 22 deletions(-) create mode 100644 packages/types/src/__tests__/nav-separator-mirror-refusal-10867.test.ts diff --git a/.changeset/10842-wizard-app-declared-keys.md b/.changeset/10842-wizard-app-declared-keys.md index 286cab5d26..8484738ebf 100644 --- a/.changeset/10842-wizard-app-declared-keys.md +++ b/.changeset/10842-wizard-app-declared-keys.md @@ -34,9 +34,6 @@ has no app layout, so the wizard's layout choice is not saved. Migration: move a top-level `favicon` URL to `branding: { favicon: '…' }`. -⚠️ **Dated note, 2026-09-28 — the wizard has no layout choice, and a separator no longer blocks the save — objectui#10867.** +⚠️ **Dated note, 2026-09-28 — the wizard has no layout choice — objectui#10867.** Later in this same release the wizard's Layout control was removed, with `AppWizardDraft.layout`, so there is no -layout choice left to not save. "Creating an app is no longer refused" did not yet hold for an app with a -navigation separator: the wizard wrote one with a `label`, which the spec's separator does not declare, and that -save was still refused until objectui#10867 made the separator carry only `type`, `id` and `order`. The rest of -this entry still holds. +layout choice left to not save. The rest of this entry still holds. diff --git a/.changeset/10867-app-wizard-saves-spec-app.md b/.changeset/10867-app-wizard-saves-spec-app.md index 40415f7dc2..7c24706e61 100644 --- a/.changeset/10867-app-wizard-saves-spec-app.md +++ b/.changeset/10867-app-wizard-saves-spec-app.md @@ -1,21 +1,21 @@ --- '@object-ui/types': minor '@object-ui/plugin-designer': patch -'@object-ui/i18n': patch +'@object-ui/i18n': minor '@object-ui/layout': patch '@object-ui/app-shell': patch --- fix(types,plugin-designer)!: the Studio app wizard saves a document the platform accepts, and an edit keeps the stored `accentColor` (objectui#10867) -⚠️ **BREAKING (authoring)**, marked `minor` under this repository's version-alignment rule (a `major` in the fixed group would move all of it off the `@objectstack` major). Two published `@object-ui/types` members narrow: a navigation separator no longer takes a `label`, and `AppWizardDraft.layout` is removed. A TypeScript literal that writes either no longer compiles. +⚠️ **BREAKING (authoring)**, marked `minor` under this repository's version-alignment rule (a `major` in the fixed group would move all of it off the `@objectstack` major). Two published `@object-ui/types` members narrow: a navigation separator no longer takes a `label`, and `AppWizardDraft.layout` is removed. A TypeScript literal that writes either no longer compiles. `NavigationItem` is now a union type alias, not an interface. So code that reads `label` off an unnarrowed `NavigationItem` into a `string` slot (its type is now `string | undefined`) also stops compiling, as does code that spreads an entry-only key onto one, and an `interface` that `extends NavigationItem` or augments it (extend `NavigationEntryItem` instead). `objectui validate` now refuses a separator `label` too: the zod mirror's `NavigationItemSchema` refuses every key on a separator that the spec's separator does not declare. The four `appDesigner` layout keys also leave the published `@object-ui/i18n` packs and `DESIGNER_DEFAULT_TRANSLATIONS`, so an application that calls `t()` with one of them now renders the raw key (unless the call passes a `defaultValue`). **Clause-②: yes (narrowing)** — the separator arm of `NavigationItem` loses `label`, and `AppWizardDraft` loses `layout`. -- **A separator carries only `type`, `id` and `order`.** `@objectstack/spec`'s separator branch declares exactly those keys, and its `AppSchema` refuses anything else. `NavigationItem` required a `label` on every item, so the wizard's "Add separator" wrote `{ id, type: 'separator', label: '' }`, and the console's create-app and edit-app saves were refused with `422 INVALID_METADATA` (`unrecognized_keys` `['label']` at `navigation.N`). `NavigationItem` is now a union of two arms, discriminated by `type`. `NavigationEntryItem` holds every other nav type and keeps its required `label`. `NavigationSeparatorItem` admits `type`, `id` and `order`, and every other entry key is `?: never` on it. Both arms are exported. Reading an entry-only key off an unnarrowed item still compiles and answers `undefined` on the separator arm. Narrow on `item.type === 'separator'` before relying on `label`. `menuItemToNavigationItem` maps a legacy separator to `{ id, type: 'separator' }` and drops its label. `spec-derived-unions.test.ts` no longer pins the separator `label` as a blocker. It asserts, at both spec tiers, that the separator arm admits the spec separator's keys and no others. -- **`@object-ui/plugin-designer`: the wizard and `NavigationDesigner` write a separator as `{ id, type }`.** `NavigationDesigner` no longer writes a label, icon or visibility onto a separator. -- **`@object-ui/layout` narrows on the separator arm; nothing it renders changes.** `resolveNavItemLabel` answers `''` for a separator, which is what a separator's `label: ''` resolved to before. The mobile bottom nav's leaf list, which already skipped separators, is now typed as entries. -- **`@object-ui/app-shell` narrows the same way; nothing it renders changes.** `useNavPins` registers a favorite only for an entry and leaves a separator unpinned, and the Studio sidebar's navigation walk passes a separator through unchanged. +- **A separator carries only `type`, `id` and `order`.** `@objectstack/spec`'s separator branch declares exactly those keys, and its `AppSchema` refuses anything else. `NavigationItem` required a `label` on every item, so the wizard's "Add separator" wrote `{ id, type: 'separator', label: '' }`, and the console's create-app and edit-app saves were refused with `422 INVALID_METADATA` (`unrecognized_keys` `['label']` at `navigation.N`). `NavigationItem` is now a union of two arms, discriminated by `type`. `NavigationEntryItem` holds every other nav type and keeps its required `label`. `NavigationSeparatorItem` admits `type`, `id` and `order`, and every other entry key is `?: never` on it. Both arms are exported. Reading an entry-only key off an unnarrowed item still compiles, but its type now includes `undefined` (a `label` is `string | undefined`), so passing it where a `string` is required does not. Narrow on `item.type === 'separator'` before relying on `label`. `menuItemToNavigationItem` maps a legacy separator to `{ id, type: 'separator' }` and drops its label. `spec-derived-unions.test.ts` no longer pins the separator `label` as a blocker. It asserts, at both spec tiers, that the separator arm admits the spec separator's keys and no others. +- **`@object-ui/plugin-designer`: the wizard and `NavigationDesigner` write a separator as `{ id, type }`.** `NavigationDesigner` no longer writes a `label` onto a new separator, and its label, icon and visibility patchers skip one. +- **`@object-ui/layout` narrows on the separator arm; nothing it renders changes.** `resolveNavItemLabel` answers `''` for every separator. A stored separator carrying a non-empty `label`, which `menuItemToNavigationItem` produced before this change, used to resolve to that label. No renderer asks it for a separator's label. The mobile bottom nav's leaf list, which already skipped separators, is now typed as entries. +- **`@object-ui/app-shell` narrows the same way; nothing it renders changes.** `useNavPins` registers a favorite only for an entry and leaves a separator as it is, and the Studio sidebar's navigation walk passes a separator through unchanged. - **`@object-ui/plugin-designer`: `EditAppPage` keeps the stored branding.** The wizard maintains the logo, primary colour and favicon, and its `branding` replaced the stored block, so a stored `accentColor` was dropped on every edit. The console reads that key. The save now keeps every stored `branding` key the spec's `AppBrandingSchema` declares, read from that schema. The wizard's values win for the keys it maintains. A stored key the spec does not declare is still left out. - **The wizard's Layout control is removed, with `AppWizardDraft.layout`.** The spec declares no app `layout`, no console surface reads one, and since objectui#10842 the save wrote none. The control persisted nothing. `EditAppPage` no longer reads a stored `layout` into the draft. The Basic Info step's description now reads "Name, title, and icon". - **`@object-ui/i18n`:** the four `appDesigner` layout keys (`layout`, `layoutSidebar`, `layoutHeader` and `layoutEmpty`) are removed from all ten packs, and `appDesigner.stepBasicDesc` no longer names a layout. diff --git a/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx b/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx index 3d70871ff9..10663f6c12 100644 --- a/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx +++ b/packages/plugin-designer/src/__tests__/AppWizard.specDocument-10867.test.tsx @@ -152,6 +152,16 @@ describe('objectui#10867 — member 2: an edit keeps every stored `branding` key expect(body.branding).toEqual({ ...STORED.branding, primaryColor: '#dc2626' }); }); + it('a logo the author clears saves as cleared, not as the stored value', async () => { + // Guards the merge's order: the wizard's `branding` wins for the keys it + // maintains, empty strings included, so a later "drop empty values" tidy-up + // would bring a removed logo back from storage. + const body = await editThrough(STORED, () => + fireEvent.change(screen.getByTestId('branding-logo-input'), { target: { value: '' } }), + ); + expect(body.branding).toEqual({ ...STORED.branding, logo: '' }); + }); + it('CONTROL — a stored `branding` key the spec does not declare is not echoed into the save', async () => { const body = await editThrough({ ...STORED, diff --git a/packages/types/src/__tests__/nav-separator-mirror-refusal-10867.test.ts b/packages/types/src/__tests__/nav-separator-mirror-refusal-10867.test.ts new file mode 100644 index 0000000000..6f7ac3cb97 --- /dev/null +++ b/packages/types/src/__tests__/nav-separator-mirror-refusal-10867.test.ts @@ -0,0 +1,79 @@ +/** + * 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. + */ + +/** + * The zod mirror refuses a separator key the spec's separator does not declare + * (objectui#10867). + * + * `NavigationItemSchema` is one flat object, so it declares `label`, `icon` and + * the rest for every item type, and its refinement's separator branch used to + * return early. `objectui validate` therefore passed `{ type: 'separator', + * label }`, which `@objectstack/spec`'s strict separator branch (`type`, `id`, + * `order`) refuses, so the platform's save door answered 422. The branch now + * refuses every key outside the set it reads off the spec's own separator arm. + * + * Pinned on both published faces: `safeValidateSchema` (the tolerant node face) + * and `StrictAnyComponentSchema` (the strict authoring face), plus the mirror + * itself. + */ + +import { describe, it, expect } from 'vitest'; +import { NavigationItemSchema as SpecNavigationItemSchema } from '@objectstack/spec/ui'; +import { NavigationItemSchema } from '../zod/app.zod'; +import { safeValidateSchema, StrictAnyComponentSchema } from '../zod/index.zod'; + +type Issue = { code: string; path: string; message: string }; +const issuesOf = (r: { success: boolean; error?: { issues: Array<{ code: string; path: PropertyKey[]; message: string }> } }): Issue[] | null => + r.success ? null : r.error!.issues.map((i) => ({ code: i.code, path: i.path.map(String).join('.'), message: i.message })); + +const app = (navigation: unknown[]) => ({ type: 'app', name: 'acme_crm', label: 'Acme CRM', navigation }); +const OBJECT_ENTRY = { id: 'account', type: 'object', label: 'Accounts', objectName: 'account' }; +const LABELLED = { id: 'sep_1', type: 'separator', label: 'Section' }; +const BARE = { id: 'sep_1', type: 'separator' }; + +describe('objectui#10867 — a separator `label` is refused on every published face', () => { + it('`safeValidateSchema` refuses it with one issue at the label, naming the fix', () => { + const issues = issuesOf(safeValidateSchema(app([OBJECT_ENTRY, LABELLED]))); + expect(issues).not.toBeNull(); + const atLabel = issues!.filter((i) => i.path.endsWith('navigation.1.label')); + expect(atLabel).toHaveLength(1); + expect(atLabel[0].code).toBe('custom'); + expect(atLabel[0].message).toContain('drop `label`'); + }); + + it('the strict authoring face refuses it too', () => { + const issues = issuesOf(StrictAnyComponentSchema.safeParse(app([OBJECT_ENTRY, LABELLED]))); + expect(issues).not.toBeNull(); + expect(issues!.some((i) => i.path.endsWith('navigation.1.label') && i.code === 'custom')).toBe(true); + }); + + it('the mirror refuses every entry-only key on a separator, each at its own path', () => { + const issues = issuesOf(NavigationItemSchema.safeParse({ ...BARE, label: 'Section', icon: 'Minus', pinned: true })); + expect(issues!.map((i) => [i.code, i.path])).toEqual([ + ['custom', 'label'], + ['custom', 'icon'], + ['custom', 'pinned'], + ]); + }); + + it('CONTROL — a bare `{ id, type: separator }` (and one with `order`) is accepted by both faces and the spec', () => { + for (const separator of [BARE, { ...BARE, order: 3 }]) { + expect(issuesOf(safeValidateSchema(app([OBJECT_ENTRY, separator])))).toBeNull(); + expect(issuesOf(StrictAnyComponentSchema.safeParse(app([OBJECT_ENTRY, separator])))).toBeNull(); + expect(SpecNavigationItemSchema.safeParse(separator).success).toBe(true); + } + }); + + it('CONTROL — the spec refuses the same `label`, so the faces now agree with it', () => { + expect(SpecNavigationItemSchema.safeParse(LABELLED).success).toBe(false); + }); + + it('CONTROL — an entry keeps its `label`, so the refusal is scoped to the separator', () => { + expect(issuesOf(safeValidateSchema(app([OBJECT_ENTRY])))).toBeNull(); + }); +}); diff --git a/packages/types/src/__tests__/navigation-spec-parity.test.ts b/packages/types/src/__tests__/navigation-spec-parity.test.ts index 1798c412fe..e25296aae5 100644 --- a/packages/types/src/__tests__/navigation-spec-parity.test.ts +++ b/packages/types/src/__tests__/navigation-spec-parity.test.ts @@ -162,8 +162,6 @@ describe('referencing the spec NavigationItemSchema would reject metadata object 'the legacy spelling this file keeps accepting for published metadata'], ['visible: boolean', { id: 'ai', type: 'url', label: 'AI', url: '/ai', visible: true }, 'menuItemToNavigationItem MANUFACTURES one when it inverts AppMenuItem.hidden'], - ['separator label', { type: 'separator', label: 'Section' }, - 'the flat mirror declares label for every type; the TS face refuses it on a separator since objectui#10867, and the spec separator declares only id/order'], ['single-character id', { id: 'a', type: 'url', label: 'A', url: '/a' }, 'objectui requires only a non-empty id; the spec requires two characters'], ])('the spec rejects %s (%s)', (_name, input, _why) => { @@ -172,4 +170,13 @@ describe('referencing the spec NavigationItemSchema would reject metadata object // ...and the spec does not, which is the whole reason for the local schema. expect(SpecNavigationItemSchema.safeParse(input).success).toBe(false); }); + + // A separator carrying `label` was a row above until objectui#10867: the flat + // mirror admitted it, the spec refused it. The mirror now refuses it too, so + // it is no longer a divergence and is asserted as an agreement instead. + it('a separator `label` is no longer a divergence: both refuse it (objectui#10867)', () => { + const input = { type: 'separator', label: 'Section' }; + expect(NavigationItemSchema.safeParse(input).success).toBe(false); + expect(SpecNavigationItemSchema.safeParse(input).success).toBe(false); + }); }); diff --git a/packages/types/src/__tests__/spec-derived-unions.test.ts b/packages/types/src/__tests__/spec-derived-unions.test.ts index 7b7f076da9..5e27ccf61a 100644 --- a/packages/types/src/__tests__/spec-derived-unions.test.ts +++ b/packages/types/src/__tests__/spec-derived-unions.test.ts @@ -313,7 +313,7 @@ const _separatorAdmitsTheSpecInputKeys = true satisfies SameKeys< AdmittedKeys, keyof SpecSeparatorInput >; -// ...and every separator this arm admits is one the spec's accepts, at both tiers. +// ...and the value types of the keys this arm admits are ones the spec's separator accepts, at both tiers. `extends` ignores extra keys, so key agreement is the two pins above: this one alone passes a labelled arm, and it is vacuous on BASE. const _localSeparatorIsSpecValid = true satisfies [LocalSeparator] extends [SpecSeparator & SpecSeparatorInput] ? true : false; diff --git a/packages/types/src/zod/app.zod.ts b/packages/types/src/zod/app.zod.ts index c99c0a60f9..5b01210fc5 100644 --- a/packages/types/src/zod/app.zod.ts +++ b/packages/types/src/zod/app.zod.ts @@ -96,6 +96,47 @@ export const NavigationItemTypeSchema = z.enum([ * `../__tests__/zod-lazy-getter-identity-7918.test.ts` — read that before * "fixing" any of them to match this one. */ +/** + * The keys `@objectstack/spec`'s separator branch declares — `type`, `id` and + * `order` on the installed pin — READ OFF the spec rather than restated + * (objectui#10867). The spec does not export its `SeparatorNavItemSchema`, so + * this walks the spec `AppSchema`'s own `navigation` element (optional → array + * → lazy → the nav-item union) and takes the arm whose `type` literal is + * `'separator'`. Reading it through `AppSchema`, which this mirror already + * crosses, keeps the read inside the objectui#8317 import boundary's measured + * population instead of adding a lazy root that population cannot walk. + * + * Computed on first use, because the spec schema is lazy. It throws when no such + * arm exists: a separator check that silently allowed nothing, or everything, + * would read as enforcement, and a thrown error is what the pin file sees. + */ +let specSeparatorKeys: readonly string[] | undefined; +function getSpecSeparatorKeys(): readonly string[] { + if (specSeparatorKeys) return specSeparatorKeys; + type Node = { + unwrap?: () => Node; + element?: Node; + options?: readonly Node[]; + shape?: Record; + }; + let node = (stripImportedDefaults(SpecAppSchema) as unknown as Node).shape?.navigation as Node | undefined; + for (let hop = 0; node && !node.options && hop < 8; hop++) node = node.element ?? node.unwrap?.(); + const arm = node?.options?.find((option) => option.shape?.type?.value === 'separator'); + if (!arm?.shape) { + throw new Error( + "objectui#10867: @objectstack/spec's AppSchema.navigation has no `type: 'separator'` arm to read the separator's keys from", + ); + } + specSeparatorKeys = Object.freeze(Object.keys(arm.shape)); + return specSeparatorKeys; +} + +/** `a`, `b` and `c` — the separator refusal's list of what a separator may carry. */ +function codeList(keys: readonly string[]): string { + const quoted = keys.map((key) => `\`${key}\``); + return quoted.length > 1 ? `${quoted.slice(0, -1).join(', ')} and ${quoted[quoted.length - 1]}` : quoted.join(''); +} + const NavigationItemObject = z.object({ // Declared optional so a bare `{ type: 'separator' }` — which the spec // accepts, and which carries no identity or text by definition — validates @@ -159,7 +200,24 @@ const NavigationItemObject = z.object({ // separator — a rule, not an entry — is exempt. Declaring the fields // optional above is what lets `{ type: 'separator' }` through, so without // this an id-less `type: 'object'` item would validate too. - if (item.type === 'separator') return; + // + // The separator carries exactly what the spec's separator declares + // (objectui#10867). This shape is flat, so it declares `label`, `icon` and + // the rest for every type; without this branch refusing them, `objectui + // validate` passed a separator `label` the platform's save door refuses with + // `unrecognized_keys`. The allowed set is read off the spec, not restated. + if (item.type === 'separator') { + const allowed = getSpecSeparatorKeys(); + for (const [key, value] of Object.entries(item)) { + if (allowed.includes(key) || value === undefined) continue; + ctx.addIssue({ + code: 'custom', + path: [key], + message: `a separator carries only ${codeList(allowed)}; drop \`${key}\``, + }); + } + return; + } for (const key of ['id', 'label'] as const) { if (typeof item[key] !== 'string' || item[key] === '') { ctx.addIssue({ @@ -215,10 +273,11 @@ export const NavigationItemSchema: z.ZodType = z.lazy(() => NavigationItemO * navigation as a discriminated union of `.strict()` variants; objectui keeps * one flat, all-optional object that deliberately accepts more. Measured * against spec 17.2.0, referencing the spec's schema would make `objectui - * validate` REJECT metadata this renderer accepts today: `pinned`, - * `defaultOpen` and a separator carrying `label` all fail `unrecognized_keys`, - * `visible: boolean` fails `invalid_union`, and a one-character `id` fails - * `too_small`. Pinned by `__tests__/navigation-spec-parity.test.ts`. + * validate` REJECT metadata this renderer accepts today: `pinned` and + * `defaultOpen` fail `unrecognized_keys`, `visible: boolean` fails + * `invalid_union`, and a one-character `id` fails `too_small`. Pinned by + * `__tests__/navigation-spec-parity.test.ts`. (A separator carrying `label` + * was a fifth until objectui#10867, which made the mirror refuse it too.) * * Converging on the union is a breaking change for every consumer that reads * fields off `NavigationItem` without narrowing — tracked separately, and diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index d028124d48..a1a49549fe 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -627,10 +627,11 @@ const ALLOW = { "reason (`the spec's is z.ZodType and would validate nothing`) is spent. The live " + "blocker is RUNTIME shape, invisible to every type-level probe: this schema has a published " + "consumer in `objectui validate`, and referencing the spec's would make it REJECT metadata " + - "objectui accepts today — `pinned`, `defaultOpen` and a separator `label` all fail " + + "objectui accepts today — `pinned` and `defaultOpen` fail " + "`unrecognized_keys` against the spec's `.strict()` branches, `visible: boolean` fails " + - "`invalid_union`, and a one-character `id` fails `too_small`. All five are pinned, behind two " + - "positive controls, in packages/types/src/__tests__/navigation-spec-parity.test.ts. " + + "`invalid_union`, and a one-character `id` fails `too_small`. All four are pinned, behind two " + + "positive controls, in packages/types/src/__tests__/navigation-spec-parity.test.ts; a separator " + + "`label` was a fifth until objectui#10867 made this mirror refuse it too. " + "Converging on the union is a breaking change tracked separately.", issue: 4115, },