diff --git a/.changeset/5144-editinline-fold.md b/.changeset/5144-editinline-fold.md new file mode 100644 index 0000000000..8a528c2365 --- /dev/null +++ b/.changeset/5144-editinline-fold.md @@ -0,0 +1,30 @@ +--- +'@object-ui/core': minor +'@object-ui/plugin-list': minor +'@object-ui/app-shell': minor +'@object-ui/plugin-view': patch +--- + +List views read `userActions.editInline` with the spec's default, off. A view's `inlineEdit` folds into it, and the console's inline-edit toggle no longer writes to the view (objectui#5144). + +**Breaking for a view that relied on the old default.** The spec declares `userActions.editInline` with `.default(false)`: the list is read-only unless the author opts in. The interface page already read it that way. The object-list toolbar did not: it read an absent `editInline` as "defer to the host", so every console grid offered the inline-edit toggle unless a view said `editInline: false`. The toolbar now reads it as the spec does. A grid view that declares neither `userActions.editInline` nor `inlineEdit` no longer offers the inline-edit toggle, on the wide toolbar or in the compact settings popover, and never opens in edit mode. + +**The fold.** `normalizeListViewSchema` (`@object-ui/core`) now folds a boolean `inlineEdit` into `userActions.editInline`, the way it already folds the `show*` flags into the other toggles: + +- `inlineEdit: true` reads as `editInline: true`: the toggle is offered and the grid opens in edit mode; +- `inlineEdit: false` reads as `editInline: false`: no toggle; +- an explicit `userActions.editInline` wins over `inlineEdit`, in both directions; +- a view with neither key reads off. + +`inlineEdit` stays on the folded view, because `ListView` opens the grid in edit mode from it. Nothing migrates stored views. + +**The console toggle is session-only (`@object-ui/app-shell`).** Both keys are the author's permission. The console's toolbar toggle used to store a user's edit mode in the view's `inlineEdit`; after the fold, switching it off would have taken the toggle away for good. It now writes nothing. The toggle switches edit mode for the session, and each load starts from the view's own `inlineEdit`. `ListView` still reports the toggle through `onInlineEditChange`. + +**A named view's `inlineEdit` keeps its precedence (`@object-ui/plugin-view`).** On a host's `renderListView`, `ObjectView` merges `userActions` from the node, the host's `views` entry and the active named view, the named view last. The named view's `userActions` now go through the same fold as the other two. So a named view's `inlineEdit` decides whether inline editing is offered ahead of a host entry's, as it already decided the edit mode. + +**What to do.** A view that should offer inline editing declares `userActions.editInline: true`. The toggle then stays offered whatever the user does with it. Two costs come with the session-only toggle: + +- the edit mode is not remembered across loads; +- a view, or a personalization overlay, where the old toggle stored `inlineEdit: false` still reads off. That is existing data, and it is not migrated. Declaring `userActions.editInline: true` on the view brings the toggle back. + +Nothing is added to a package entry: no export, prop, type member or language-pack key. diff --git a/content/docs/plugins/plugin-view.mdx b/content/docs/plugins/plugin-view.mdx index 858dcfb098..41e88cfee1 100644 --- a/content/docs/plugins/plugin-view.mdx +++ b/content/docs/plugins/plugin-view.mdx @@ -514,6 +514,29 @@ node takes the value from the active named view first, ahead of the host's `views` entry (objectui#10758). `src/__tests__/ObjectView.namedViewProtocolKeys-8980.test.tsx` pins each family. +**Inline editing on that node is opt-in.** Both keys below are the author's +permission. `ListView` reads the node's `userActions.editInline` with the +spec's default, off, so a view that declares neither key offers no inline +editing (objectui#5144). When the node carries no `editInline` of its own, its +`inlineEdit` stands in for it. The named view's keys win over the host +`views` entry's, as for every member above: + +| The view declares | Inline-edit toggle | Grid opens in edit mode | +| --- | --- | --- | +| neither key | not offered | no | +| `inlineEdit: true` | offered | yes | +| `inlineEdit: false` | not offered | no | +| `userActions.editInline: true` | offered | only with `inlineEdit: true` | +| `userActions.editInline: false` | not offered | no, whatever `inlineEdit` says | + +The toggle switches the edit mode for the session and reports it through +`ListView`'s `onInlineEditChange`. The console stores nothing for it: each load +opens in the mode the view's `inlineEdit` declares, so a user's choice is not +remembered across loads. A view where an earlier console stored +`inlineEdit: false` still reads off; that data is not migrated. To offer inline +editing, declare `userActions.editInline: true`. Neither key opens editing past +the object's own editability or the user's permission to update the object. + ### Update Editing is reached from a row's edit action, and `operations.update` is what diff --git a/packages/app-shell/src/views/InterfaceListPage.editInlineFold-5144.test.tsx b/packages/app-shell/src/views/InterfaceListPage.editInlineFold-5144.test.tsx new file mode 100644 index 0000000000..967b236912 --- /dev/null +++ b/packages/app-shell/src/views/InterfaceListPage.editInlineFold-5144.test.tsx @@ -0,0 +1,134 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#5144, the interface-page half. + * + * An ADR-0047 interface page reads the page config's `userActions.editInline` + * as `=== true` and hands it to `ListView` as the view's `inlineEdit`. It does + * not forward `editInline` itself, so the `userActions` block it builds carries + * no `editInline` key. Before objectui#5144, `ListView` read that absent key as + * "defer to the host channel" and offered inline editing on this page's compact + * toolbar. The page itself read the absent case as off. + * + * After the B-fold, `ListView` reads what this page composes through + * `normalizeListViewSchema`, which folds the `inlineEdit` the page sets into + * `userActions.editInline`. So the two readings agree. This file pins it by + * capturing the schema the page hands `ListView` and running it through the + * real fold, which is the input `ListView`'s `inlineEditOffered` reads. What + * `ListView` does with `editInline` is pinned in `@object-ui/plugin-list`'s + * `ListView.permissions.test.tsx`. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; + +vi.mock('react-router-dom', () => ({ + useSearchParams: () => [new URLSearchParams(), vi.fn()], + useNavigate: () => vi.fn(), +})); + +vi.mock('@object-ui/i18n', async (importOriginal) => { + const actual = await (importOriginal as any)(); + return { + ...actual, + useObjectTranslation: () => ({ t: (_k: string, o?: any) => o?.defaultValue ?? _k }), + }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => { + const actual = await (importOriginal as any)(); + return { ...actual, useAuth: () => ({}) }; +}); + +// Only the schema this page hands `ListView` is under test. +vi.mock('@object-ui/plugin-list', async (importOriginal) => ({ + ...(await importOriginal()), + ListView: (props: any) => ( +
+ ), +})); + +let testDataSource: any; +let testObjects: any[]; + +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await (importOriginal as any)(); + return { + ...actual, + useAdapter: () => testDataSource, + useMetadata: () => ({ objects: testObjects }), + }; +}); + +import { normalizeListViewSchema } from '@object-ui/core'; +import { InterfaceListPage } from './InterfaceListPage'; + +const OBJECT_NAME = 'showcase_task'; +const VIEW_ID = `${OBJECT_NAME}.all`; + +const objectDef = { + name: OBJECT_NAME, + fields: { title: { type: 'text' }, status: { type: 'text' } }, + listViews: { + [VIEW_ID]: { name: VIEW_ID, type: 'grid', columns: ['title', 'status'] }, + }, +}; + +/** Renders the page and returns the schema it hands `ListView`. */ +async function composedSchema(pageUserActions: Record | undefined) { + testDataSource = {}; + testObjects = [objectDef]; + const page = { + name: 'showcase_task_list', + label: 'Tasks', + interfaceConfig: { + source: OBJECT_NAME, + sourceView: 'all', + recordAction: 'none', + ...(pageUserActions ? { userActions: pageUserActions } : {}), + }, + }; + render(); + await waitFor(() => expect(screen.queryByTestId('list-view-schema')).not.toBeNull()); + return JSON.parse(screen.getByTestId('list-view-schema').getAttribute('data-schema') || 'null'); +} + +/** The `editInline` `ListView` reads: the composed schema, through the fold. */ +const editInlineRead = (schema: Record) => + (normalizeListViewSchema(schema).userActions as Record | undefined)?.editInline; + +describe('InterfaceListPage: editInline reads the same as ListView after the fold (objectui#5144)', () => { + beforeEach(() => { + testDataSource = undefined; + testObjects = []; + }); + + it('an ABSENT page editInline reads off on both: no edit mode, and `editInline` folds to false', async () => { + const schema = await composedSchema({ search: true }); + // The page's own reading (unchanged): `=== true`. + expect(schema.inlineEdit).toBe(false); + // The page forwards no `editInline` of its own… + expect(schema.userActions).not.toHaveProperty('editInline'); + // …so `ListView` reads the folded one, which is off. + expect(editInlineRead(schema)).toBe(false); + }); + + it('a page with no `userActions` block at all reads off the same way', async () => { + const schema = await composedSchema(undefined); + expect(schema.inlineEdit).toBe(false); + expect(editInlineRead(schema)).toBe(false); + }); + + it('a page that opts in reads on, on both', async () => { + const schema = await composedSchema({ editInline: true }); + expect(schema.inlineEdit).toBe(true); + expect(editInlineRead(schema)).toBe(true); + }); + + it('an explicit page `editInline: false` stays off', async () => { + const schema = await composedSchema({ editInline: false }); + expect(schema.inlineEdit).toBe(false); + expect(editInlineRead(schema)).toBe(false); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.fallbackTabSessionOnly-11643.test.tsx b/packages/app-shell/src/views/ObjectView.fallbackTabSessionOnly-11643.test.tsx index bb28faec73..1e3418b62a 100644 --- a/packages/app-shell/src/views/ObjectView.fallbackTabSessionOnly-11643.test.tsx +++ b/packages/app-shell/src/views/ObjectView.fallbackTabSessionOnly-11643.test.tsx @@ -309,7 +309,9 @@ describe('objectui#11643 — the console-made tab of a view-less object keeps it expect(listSchema.label).toBe(FALLBACK_TAB_LABEL); // Every control that reaches `persistViewPatch`, through both the schema's - // and the list's own spelling of it. + // and the list's own spelling of it. `onInlineEditChange` no longer reaches + // it at all (objectui#5144, ruling E: the toggle is session-only); it stays + // here because it must still write nothing. act(() => { listSchema.onDensityChange('comfortable'); listSchema.onSortChange(SORT); diff --git a/packages/app-shell/src/views/ObjectView.inlineEditSessionOnly-5144.test.tsx b/packages/app-shell/src/views/ObjectView.inlineEditSessionOnly-5144.test.tsx new file mode 100644 index 0000000000..47843efbfd --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.inlineEditSessionOnly-5144.test.tsx @@ -0,0 +1,375 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#5144, triage's ruling E — the console's inline-edit toggle is + * session state and writes nothing. + * + * The view's `inlineEdit` and `userActions.editInline` are the author's + * permission keys, and `normalizeListViewSchema` folds the first into the + * second. The toggle used to persist a user's edit mode into `inlineEdit` + * through `persistViewPatch`. After the fold, switching it off stored + * `inlineEdit: false`, which reads as "not offered", so the toggle was gone + * from the next load. Ruling E removes that write. + * + * ## What runs + * + * The harness of `ObjectView.fallbackTabSessionOnly-11643.test.tsx`: the real + * object page and the real `persistViewPatch` (its 300 ms debounce included), + * over the real `ObjectStackAdapter`, whose metadata client is a store that + * judges every PUT with the spec's `ViewMetadataSchema`. `ListView` is stubbed + * to capture what the page hands it. Whether the toggle is OFFERED is + * `ListView`'s `inlineEditOffered`, which reads the folded + * `userActions.editInline` (pinned in `@object-ui/plugin-list`'s + * `ListView.permissions.test.tsx`). These cases read that input: the schema the + * page hands `ListView`, through the fold. + * + * Every "writes nothing" case carries its firing control: a density change on + * the same view in the same test DOES write, so an empty write log is a + * reading of the toggle and not of a harness that cannot see writes. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, act, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { normalizeListViewSchema } from '@object-ui/core'; +import { ViewMetadataSchema } from '@objectstack/spec/ui'; +import { ObjectStackAdapter } from '@object-ui/data-objectstack'; + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + // Stable identities: `ListView` names `perms` in its fetch dependencies. + const perms = { + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + isLoaded: false, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }; + const fieldPerms = { canRead: () => true, canWrite: () => true, permissions: [] }; + return { ...actual, usePermissions: () => perms, useFieldPermissions: () => fieldPerms }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: true, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRealtimeSubscription: () => ({ lastMessage: null }), + useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }), +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); + +/** What the object page hands `ListView`: captured, not rendered. */ +let listProps: any = null; +let listSchema: any = null; +vi.mock('@object-ui/plugin-list', async (importOriginal) => ({ + ...(await importOriginal()), + ListView: (props: any) => { + listProps = props; + listSchema = props.schema; + return null; + }, +})); + +import { toast } from 'sonner'; +import { ObjectView } from './ObjectView'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; + +const OBJECT_NAME = 'track_note'; +const SAVED_ID = `${OBJECT_NAME}.mine`; + +const FIELDS = { + id: { type: 'text', label: 'Id' }, + title: { type: 'text', label: 'Title' }, + status: { type: 'text', label: 'Status' }, +}; + +/** A served view that opts in to inline editing, the remedy the changeset names. */ +const OPTED_IN_OBJECT = { + name: OBJECT_NAME, + label: 'Note', + fields: FIELDS, + listViews: { + all: { label: 'All notes', type: 'grid', columns: ['title', 'status'], userActions: { editInline: true } }, + }, +}; + +/** A served view that declares neither key. */ +const PLAIN_OBJECT = { + name: OBJECT_NAME, + label: 'Note', + fields: FIELDS, + listViews: { all: { label: 'All notes', type: 'grid', columns: ['title', 'status'] } }, +}; + +/** A user-saved view whose own row stores `inlineEdit: true`. */ +const SAVED_ROW = { + name: SAVED_ID, + object: OBJECT_NAME, + viewKind: 'list', + label: 'Mine', + config: { + type: 'grid', + data: { provider: 'object', object: OBJECT_NAME }, + columns: ['title', 'status'], + inlineEdit: true, + }, +}; + +/** + * A marked overlay on the served view from an earlier session, so the tab + * carries a `viewKind` and its density write is a row the door keeps (the + * same seed `ObjectView.fallbackTabSessionOnly-11643.test.tsx` uses). + */ +const SERVED_OVERLAY = { + name: 'all', + object: OBJECT_NAME, + viewKind: 'list', + columnState: { widths: { title: 240 } }, + _isOverride: true, +}; + +/** An overlay the OLD toggle wrote on the served view: `inlineEdit: false`. */ +const OLD_TOGGLE_OVERLAY = { + name: 'all', + object: OBJECT_NAME, + viewKind: 'list', + inlineEdit: false, + _isOverride: true, +}; + +/** + * A `sys_metadata`-shaped store: every PUT is judged by the spec's + * `ViewMetadataSchema`, the PARSED value is kept, and the PUT is answered the + * way the door answers it — with no row. + */ +function makeStore(seed: Record[]) { + const rows = new Map(); + for (const row of seed) rows.set(row.name, structuredClone(row)); + let seq = 0; + const bodies: any[] = []; + const meta = { + getItems: vi.fn(async (type: string) => ({ + type, + items: type === 'view' ? [...rows.values()].map((r) => structuredClone(r)) : [], + })), + getItem: vi.fn(async (type: string, name: string) => { + const item = type === 'view' ? rows.get(name) : undefined; + if (!item) throw Object.assign(new Error(`Not found: ${type}/${name}`), { status: 404 }); + return { type, name, item: structuredClone(item) }; + }), + saveItem: vi.fn(async (_type: string, name: string, item: any) => { + bodies.push(structuredClone(item)); + const judged = ViewMetadataSchema.safeParse(item); + if (!judged.success) { + throw Object.assign(new Error(`422 INVALID_METADATA: ${judged.error.message}`), { status: 422 }); + } + const kept = Object.fromEntries( + Object.entries(judged.data as Record).filter(([key]) => key in item), + ); + rows.set(name, kept); + seq += 1; + return { success: true, version: `hmac-sha256:${'0'.repeat(63)}${seq}`, seq, state: 'active', message: `Saved view '${name}'` }; + }), + }; + return { meta, rows, bodies }; +} + +/** The real adapter over the store, and the page's data source over the adapter. */ +function pageDataSource(meta: any) { + const ds: any = new ObjectStackAdapter({ + baseUrl: 'http://test.local', + fetch: vi.fn(async () => + new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + })), + }); + ds.connected = true; + ds.connectionState = 'connected'; + ds.client = { meta }; + return { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + listViews: (objectName: string, o?: any) => ds.listViews(objectName, o), + listViewOverrides: (objectName: string) => ds.listViewOverrides(objectName), + getView: (objectName: string, viewId: string) => ds.getView(objectName, viewId), + updateViewConfig: vi.fn((objectName: string, viewId: string, config: any, o?: any) => + ds.updateViewConfig(objectName, viewId, config, o)), + } as any; +} + +const wait = (ms: number) => act(() => new Promise((resolve) => setTimeout(resolve, ms))); + +/** Mount the object page (on `viewId`, or on the object's default tab) and wait for its list schema. */ +async function openPage(meta: any, object: Record, viewId?: string) { + listProps = null; + listSchema = null; + const dataSource = pageDataSource(meta); + const page = ( + {}} /> + ); + render( + + + + + + + + , + ); + await waitFor(() => expect(typeof listSchema?.onDensityChange).toBe('function')); + // Let the page's own reads (saved views, stored rows) land before a toggle. + await wait(50); + return dataSource; +} + + +/** The schema the page hands `ListView`, through the fold `ListView` runs. */ +function folded(): { editInline: unknown; inlineEdit: unknown } { + const schema = normalizeListViewSchema(listSchema) as { userActions?: Record; inlineEdit?: unknown }; + return { editInline: schema.userActions?.editInline, inlineEdit: schema.inlineEdit }; +} + +/** Every body the store was asked to save that carries an `inlineEdit` anywhere. */ +function bodiesCarryingInlineEdit(bodies: any[]): any[] { + return bodies.filter((b) => b && ('inlineEdit' in b || (b.config && 'inlineEdit' in b.config))); +} + +beforeEach(() => { + cleanup(); + listProps = null; + listSchema = null; + vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }))); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + vi.clearAllMocks(); +}); + +describe('objectui#5144 (ruling E) — the console inline-edit toggle writes nothing', () => { + it('switching the toggle on a served view sends no write; a density change beside it does', async () => { + const { meta, rows, bodies } = makeStore([SERVED_OVERLAY]); + const ds = await openPage(meta, OPTED_IN_OBJECT, 'all'); + expect(listSchema.label).toBe('All notes'); + expect(typeof listProps.onInlineEditChange).toBe('function'); + + act(() => { + listProps.onInlineEditChange(true); + listProps.onInlineEditChange(false); + }); + // Past the 300 ms debounce and any round trip it would have started. + await wait(700); + expect(ds.updateViewConfig).not.toHaveBeenCalled(); + expect(meta.saveItem).not.toHaveBeenCalled(); + expect(rows.get('all')).toEqual(SERVED_OVERLAY); + + // Firing control: the same page, the same view, a toolbar control that is + // still persisted. + act(() => listSchema.onDensityChange('comfortable')); + await wait(700); + expect(ds.updateViewConfig).toHaveBeenCalledTimes(1); + expect(rows.get('all').rowHeight).toBe('medium'); + expect(rows.get('all')).not.toHaveProperty('inlineEdit'); + expect(bodiesCarryingInlineEdit(bodies)).toEqual([]); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('two-way: on a view that declares `userActions.editInline: true`, the toggle is still offered after it is switched off and the view remounts', async () => { + const { meta, rows } = makeStore([SERVED_OVERLAY]); + await openPage(meta, OPTED_IN_OBJECT, 'all'); + expect(folded().editInline).toBe(true); + + act(() => listProps.onInlineEditChange(false)); + await wait(700); + expect(rows.get('all')).toEqual(SERVED_OVERLAY); + + cleanup(); + await openPage(meta, OPTED_IN_OBJECT, 'all'); + expect(folded().editInline).toBe(true); + // Nothing was stored, so the mode is seeded from the view again. + expect(folded().inlineEdit).toBeUndefined(); + }); + + it('a saved view: the toggle switched off writes nothing, and the stored `inlineEdit: true` is what the next load reads', async () => { + // Recorded cost of ruling E: the edit mode is not remembered across loads. + const { meta, rows, bodies } = makeStore([SAVED_ROW]); + const ds = await openPage(meta, PLAIN_OBJECT, SAVED_ID); + expect(listSchema.label).toBe('Mine'); + expect(folded()).toEqual({ editInline: true, inlineEdit: true }); + + act(() => listProps.onInlineEditChange(false)); + await wait(700); + expect(ds.updateViewConfig).not.toHaveBeenCalled(); + expect(meta.saveItem).not.toHaveBeenCalled(); + + // Firing control: a density change on the same saved view writes its row + // whole. The row's own `inlineEdit` rides along unchanged, never the + // toggle's `false`. + act(() => listSchema.onDensityChange('comfortable')); + await wait(700); + expect(ds.updateViewConfig).toHaveBeenCalledTimes(1); + expect(rows.get(SAVED_ID).config.rowHeight).toBe('medium'); + expect(rows.get(SAVED_ID).config.inlineEdit).toBe(true); + expect(bodiesCarryingInlineEdit(bodies).every((b) => (b.config ?? b).inlineEdit === true)).toBe(true); + + cleanup(); + await openPage(meta, PLAIN_OBJECT, SAVED_ID); + expect(folded()).toEqual({ editInline: true, inlineEdit: true }); + }); + + it('an overlay the old toggle wrote, `inlineEdit: false`, still reads off', async () => { + // Recorded cost of ruling E: existing data, and the maintainer's ruling + // rejects migrating it. `@object-ui/data-objectstack` still lists + // `inlineEdit` among the keys an overlay owns, so the row is read. + const { meta } = makeStore([OLD_TOGGLE_OVERLAY]); + await openPage(meta, PLAIN_OBJECT, 'all'); + expect(folded()).toEqual({ editInline: false, inlineEdit: false }); + }); + + it('control: the same served view with no overlay declares nothing, and reads off by the spec default', async () => { + const { meta } = makeStore([]); + await openPage(meta, PLAIN_OBJECT, 'all'); + expect(folded()).toEqual({ editInline: undefined, inlineEdit: undefined }); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts b/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts index 88fbaba88e..573d11c41b 100644 --- a/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts +++ b/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts @@ -571,7 +571,12 @@ describe('objectui#5233 ratchet — the owned-key list tracks the writers', () = const written = [...new Set([...objectViewSrc.matchAll(PERSIST_CALLS)].map((m) => m[1]!))]; // Vacuous-pass guard: the call sites are the evidence, so an empty // match set means the regex stopped matching, not that nothing writes. - expect(written.length).toBeGreaterThanOrEqual(5); + // Four since objectui#5144 (triage's ruling E): the inline-edit toggle + // is session state and no longer persists `inlineEdit`. The adapter + // still lists `inlineEdit` as an owned key, so an overlay an earlier + // console wrote is still read; that is the reading the ruling keeps. + expect(written.length).toBeGreaterThanOrEqual(4); + expect(written).not.toContain('inlineEdit'); for (const key of written) { expect( VIEW_OVERLAY_OWNED_KEYS as readonly string[], diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 13c1ed236e..b803bc26d9 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -1513,6 +1513,31 @@ export interface ConsoleObjectViewProps { externalRefreshKey?: number; } +/** + * The list toolbar's inline-edit toggle, as this page wires it: it writes + * nothing (objectui#5144, triage's ruling E). + * + * The view's `inlineEdit` and `userActions.editInline` are the AUTHOR's + * permission keys. The spec says so for both ("the list is read-only unless + * the author opts in"), and `normalizeListViewSchema` folds the first into the + * second. This toggle used to persist a USER's edit mode into `inlineEdit` + * through `persistViewPatch`. After the fold, switching it off stored + * `inlineEdit: false`, which reads as "not offered", so the toggle was gone + * from the next load and nothing in the console could bring it back. + * + * The edit mode is now session state. `ListView` keeps it, and seeds it from + * the view's `inlineEdit` on each load. The callback stays wired because + * `ListView` offers the wide toolbar toggle only to a host that wires one. + * + * Recorded costs: the edit mode is not remembered across loads, and an overlay + * that already stores `inlineEdit: false` still reads off. That is existing + * data, and the maintainer's ruling rejects migrating it. A view that should + * offer inline editing declares `userActions.editInline: true`. + */ +function keepInlineEditModeForTheSession(): void { + // Deliberately empty: see the docblock. +} + export function ObjectView({ dataSource, objects, onEdit, externalRefreshKey }: ConsoleObjectViewProps) { const { objectName } = useParams(); const { t } = useObjectTranslation(); @@ -1624,7 +1649,8 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co // changes apply for the session (the list keeps them in its own // state) and nothing is scheduled here: no read, no PUT, no toast. // One rule for every control that reaches this function — density, - // sort, hidden fields, column order and widths, inline edit. A + // sort, hidden fields, column order and widths. (The inline-edit + // toggle no longer reaches it: objectui#5144, ruling E.) A // stored row that shadows the tab is a real row (`isSavedViewId`, // the classification the write below uses) and keeps its save // path. See `CONSOLE_MADE_TAB`. @@ -3715,9 +3741,8 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co writeListUrlState({ search }); }} onHiddenFieldsChange={persistHiddenFields} - onInlineEditChange={(next: boolean) => { - persistViewPatch(viewDef.id, viewDef, { inlineEdit: next }); - }} + // objectui#5144 (ruling E): session-only, writes nothing. + onInlineEditChange={keepInlineEditModeForTheSession} onColumnStateChange={(state: { order?: string[]; widths?: Record }) => { persistViewPatch(viewDef.id, viewDef, { columnState: state }); }} diff --git a/packages/core/src/utils/__tests__/normalize-list-view.inlineEditFold-5144.test.ts b/packages/core/src/utils/__tests__/normalize-list-view.inlineEditFold-5144.test.ts new file mode 100644 index 0000000000..30c95fc954 --- /dev/null +++ b/packages/core/src/utils/__tests__/normalize-list-view.inlineEditFold-5144.test.ts @@ -0,0 +1,120 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#5144 — a view's `inlineEdit` folds into `userActions.editInline`. + * + * The spec declares `userActions.editInline` with `.default(false)`. Stored + * views never carried that key: they carry `inlineEdit`, which authors declare + * and the console's list toolbar used to write. Until this fold, `ListView` + * could not read `editInline` with the spec default without taking inline + * editing away from every stored console view, so it read an absent + * `editInline` as "defer to the host". The maintainer ruled the B-fold: the + * stored key folds into the spec key, and the spec default is read as written. + * Triage's ruling E then stopped the toolbar writing `inlineEdit` at all. + * + * This file pins the fold's table. `@object-ui/plugin-list`'s + * `ListView.permissions.test.tsx` pins what the toolbar does with its output. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { ListViewSchema as SpecListViewSchema } from '@objectstack/spec/ui'; +import { normalizeListViewSchema } from '../normalize-list-view.js'; + +type View = Record; + +const fold = (view: View): View => normalizeListViewSchema(view); +const editInlineOf = (view: View): unknown => + (view.userActions as Record | undefined)?.editInline; + +/** Already canonical apart from the pair under test, so nothing else folds. */ +const BASE: View = { type: 'list-view', objectName: 'task', viewType: 'grid' }; + +describe('a view`s inlineEdit folds into userActions.editInline (objectui#5144)', () => { + beforeEach(() => { + // #8372's undrawable-kind warning is developer-facing noise here. + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + /** + * One row per stored shape. `editInline` is what `ListView` reads, with the + * spec default, to decide whether inline editing is offered. `inlineEdit` is + * what it seeds the grid's edit mode from, so the fold must keep it. + */ + const TABLE: ReadonlyArray<{ + name: string; + input: View; + editInline: boolean | undefined; + inlineEdit: boolean | undefined; + byReference: boolean; + }> = [ + { name: 'neither key: nothing to fold', input: {}, editInline: undefined, inlineEdit: undefined, byReference: true }, + { name: 'stored inlineEdit: true folds to on', input: { inlineEdit: true }, editInline: true, inlineEdit: true, byReference: false }, + { name: 'stored inlineEdit: false folds to off', input: { inlineEdit: false }, editInline: false, inlineEdit: false, byReference: false }, + { + name: 'explicit editInline: false wins over a stored inlineEdit: true', + input: { inlineEdit: true, userActions: { editInline: false } }, + editInline: false, inlineEdit: true, byReference: true, + }, + { + name: 'explicit editInline: true wins over a stored inlineEdit: false', + input: { inlineEdit: false, userActions: { editInline: true } }, + editInline: true, inlineEdit: false, byReference: true, + }, + { name: 'explicit editInline alone is left as declared', input: { userActions: { editInline: true } }, editInline: true, inlineEdit: undefined, byReference: true }, + { name: 'a non-boolean inlineEdit is not a value and does not fold', input: { inlineEdit: 'yes' }, editInline: undefined, inlineEdit: 'yes' as never, byReference: true }, + ]; + + it('covers a non-empty table', () => { + expect(TABLE.length).toBe(7); + }); + + for (const row of TABLE) { + it(row.name, () => { + const input = { ...BASE, ...row.input }; + const out = fold(input); + expect(editInlineOf(out)).toBe(row.editInline); + expect(out.inlineEdit).toBe(row.inlineEdit); + // Returned by reference when there is nothing to fold, so `ListView`'s + // `useMemo`s keep a stable dependency on the already-canonical path. + expect(out === input).toBe(row.byReference); + }); + } + + it('keeps the other `userActions` toggles, and does not mutate its input', () => { + const input = { ...BASE, inlineEdit: true, userActions: { search: false, hideFields: true } }; + const out = fold(input); + expect(out.userActions).toEqual({ search: false, hideFields: true, editInline: true }); + expect(input.userActions).toEqual({ search: false, hideFields: true }); + }); + + it('folds in the same pass as the legacy `show*` flags, and keeps only inlineEdit', () => { + const out = fold({ ...BASE, inlineEdit: true, showSearch: false, showHideFields: true }); + expect(out.userActions).toEqual({ search: false, hideFields: true, editInline: true }); + // The `show*` rows are deleted after they fold. `inlineEdit` is not. + expect('showSearch' in out).toBe(false); + expect('showHideFields' in out).toBe(false); + expect(out.inlineEdit).toBe(true); + }); + + it('the folded document is spec-valid, `editInline` and `inlineEdit` both', () => { + // Both keys are the spec's: `ListView.inlineEdit` and + // `userActions.editInline`. `viewType` is objectui's own spelling and is + // scoped out, as in `normalize-list-view.foldOutputAuthorable-5435.test.ts`. + const out = fold({ name: 'tasks', label: 'Tasks', type: 'grid', columns: [{ field: 'title' }], inlineEdit: true }); + expect(editInlineOf(out)).toBe(true); + const { viewType: _viewType, ...specSpelled } = out; + const result = SpecListViewSchema.safeParse(specSpelled); + expect(result.success).toBe(true); + expect(result.error?.issues ?? []).toEqual([]); + }); +}); diff --git a/packages/core/src/utils/normalize-list-view.ts b/packages/core/src/utils/normalize-list-view.ts index 7cb80439b2..6e7bf5f5bb 100644 --- a/packages/core/src/utils/normalize-list-view.ts +++ b/packages/core/src/utils/normalize-list-view.ts @@ -107,6 +107,11 @@ const isRecord = (v: unknown): v is Record => * firing control. ⛔ Do not read the three as legacy-only spellings awaiting * retirement — the protocol declares them, so removing them here would leave * objectui narrower than the protocol. + * + * ⛔ `inlineEdit` → `editInline` is NOT a row here, although it lands in the same + * block (objectui#5144). Every row of this table is deleted after it folds; + * `inlineEdit` must survive its fold, because it is also the MODE `ListView` + * opens the grid in. See {@link foldsInlineEditIntoEditInline}. */ const SHOW_FLAG_TO_USER_ACTION: Record = { showSearch: 'search', @@ -118,6 +123,42 @@ const SHOW_FLAG_TO_USER_ACTION: Record = { showColor: 'rowColor', }; +/** + * Whether a view's `inlineEdit` folds into `userActions.editInline` + * (objectui#5144, the maintainer's B-fold). + * + * The spec declares `userActions.editInline` with `.default(false)`: "the list + * is read-only unless the author opts in". It declares the view's `inlineEdit` + * as the same kind of permission ("allow inline editing"). Stored views carry + * `inlineEdit`, not `editInline`: authored views declare it, and the console's + * list toolbar used to write it. Folding it here puts both keys on one + * vocabulary, so `ListView` can read `editInline` with the spec default and a + * view that has inline editing keeps it. + * + * The console's toolbar no longer writes `inlineEdit` (objectui#5144, ruling + * E). It persisted a user's edit mode into this permission key, so switching + * the toggle off stored `inlineEdit: false` and the fold then read the view as + * not offering inline editing. The toggle is session state now. + * + * Two rules, each the same as a fold above: + * - The value carries over: `inlineEdit: true` folds to `editInline: true`, + * and `inlineEdit: false` to `editInline: false`. + * - An explicit `userActions.editInline` wins. It is the spec key, so a + * stored `inlineEdit` only fills the gap. + * + * One departure, the `data` → `objectName` one: `inlineEdit` is KEPT. The other + * folds delete because the legacy key has one meaning and one home. + * `inlineEdit` has a second use. It is the spec's own `ListView.inlineEdit`, + * and `ListView` seeds the grid's edit MODE from it on each load. The toolbar + * toggle then flips that mode for the session. `editInline` decides whether + * the toggle is offered at all. Deleting `inlineEdit` would open every such + * view out of edit mode. + */ +function foldsInlineEditIntoEditInline(s: Record): boolean { + if (typeof s.inlineEdit !== 'boolean') return false; + return !(isRecord(s.userActions) && typeof s.userActions.editInline === 'boolean'); +} + /** * Legacy `sharing.visibility` → the spec's `ViewSharing.type`. The spec models * two ownership kinds; objectui's four-value audience enum collapses onto them: @@ -384,6 +425,11 @@ const PER_VIEW_CONFIG_ALIASES: Record>> * stays absent, because the defaults are per-toggle (search/sort/filter/ * rowHeight/group default ON, hideFields/rowColor default OFF) and belong to * the renderer, not to the vocabulary bridge. + * - the view's `inlineEdit` → `userActions.editInline` (objectui#5144), with + * the value carried over and an explicit `editInline` winning. `inlineEdit` + * stays on the result, because `ListView` seeds the grid's edit mode from + * it. See {@link foldsInlineEditIntoEditInline}. An absent pair stays + * absent here as well; `ListView` reads that as off, the spec's default. * - `aria: { label, describedBy }` → the spec's `AriaProps` * (`{ ariaLabel, ariaDescribedBy }`), and `sharing: { visibility, enabled }` * → the spec's `ViewSharing` (`{ type }`) — #2890 scope A step 5. `aria.live` @@ -458,6 +504,7 @@ export function normalizeListViewSchema(schema: T): T { const legacyFilters = s.filters; const foldFilter = Array.isArray(legacyFilters); const legacyFlags = Object.keys(SHOW_FLAG_TO_USER_ACTION).filter((k) => typeof s[k] === 'boolean'); + const foldInlineEdit = foldsInlineEditIntoEditInline(s); const foldDescription = typeof s.showDescription === 'boolean'; const aria = isRecord(s.aria) ? s.aria : undefined; const foldAria = !!aria && Object.keys(ARIA_KEY_ALIASES).some((k) => aria[k] !== undefined); @@ -502,7 +549,7 @@ export function normalizeListViewSchema(schema: T): T { }) .filter((entry): entry is NonNullable => entry !== undefined); if ( - !foldColumns && !foldRowHeight && !foldFilter && !legacyFlags.length && + !foldColumns && !foldRowHeight && !foldFilter && !legacyFlags.length && !foldInlineEdit && !foldDescription && !foldAria && !foldSharing && !defaultViewKind && !foldColumnIdentity && !perViewFolds.length && !foldObjectName ) { @@ -527,13 +574,16 @@ export function normalizeListViewSchema(schema: T): T { if (!Array.isArray(next.filter)) next.filter = legacyFilters; delete next.filters; } - if (legacyFlags.length) { + if (legacyFlags.length || foldInlineEdit) { const ua: Record = { ...(isRecord(next.userActions) ? next.userActions : {}) }; for (const flag of legacyFlags) { const key = SHOW_FLAG_TO_USER_ACTION[flag]; if (typeof ua[key] !== 'boolean') ua[key] = s[flag]; delete next[flag]; } + // objectui#5144. Gap-fill only, and `inlineEdit` is not deleted: see + // `foldsInlineEditIntoEditInline`. + if (foldInlineEdit) ua.editInline = s.inlineEdit; next.userActions = ua; } if (foldDescription) { diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 1111bb549b..32589d7af3 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -2886,7 +2886,9 @@ export function viewItemObjectName(item: any): string | undefined { * * `updateViewConfig` has exactly ONE production caller — `ObjectView`'s * `persistViewPatch`, invoked only for the toolbar-driven density / sort / - * hiddenFields / columnState / inlineEdit toggle. That single call site is + * hiddenFields / columnState toggles. (The inline-edit toggle wrote + * `inlineEdit` through it until objectui#5144; the console now keeps that + * toggle session-only and writes nothing for it.) That single call site is * NOT itself the explicit "create/save a view" path (that goes through * {@link ObjectStackAdapter.createView} or the ADR-0034 metadata seam, * `viewEnvelope` in app-shell) — but it fires for a toggle on EITHER kind of @@ -2963,11 +2965,14 @@ function isPersonalizationOverlayRow(item: any, spec: any): boolean { * One per `persistViewPatch` call site in app-shell's `ObjectView` — the ONLY * production writer of these rows — read off the tree rather than recalled: * `rowHeight` (the density toggle, spec-canonical since #2890), `sort`, - * `hiddenFields`, `columnState` and `inlineEdit`. Nothing else in such a row - * is an opinion the user expressed; anything else it carries is a COPY of the - * source view as it stood at write time, because `persistViewPatch` USED TO - * send `{ ...baseViewDef, ...patch }` and this adapter persists what it is - * given. + * `hiddenFields` and `columnState`. Plus `inlineEdit`, which no console call + * site writes since objectui#5144 (triage's ruling E: the inline-edit toggle + * is session-only). It stays owned so an overlay the old toggle wrote is still + * read; the ruling keeps that data rather than migrating it. Nothing else in + * such a row is an opinion the user expressed; anything else it carries is a + * COPY of the source view as it stood at write time, because + * `persistViewPatch` USED TO send `{ ...baseViewDef, ...patch }` and this + * adapter persists what it is given. * * That copy was the defect the maintainer ruled on (objectstack#7494, comment * 5261754173): an overlay written by a mere column drag froze the view's @@ -3000,9 +3005,10 @@ function isPersonalizationOverlayRow(item: any, spec: any): boolean { * ⛔ Do not grow this list to make some other key "stick" through an overlay. * A key that belongs to the view belongs in the view; the overlay is a patch, * and a patch that carries the whole document is what this list exists to - * stop. Adding a sixth entry is only correct alongside a sixth - * `persistViewPatch` call site — {@link narrowPersonalizationOverlay} is what - * a reader checks that against. + * stop. Adding an entry is only correct alongside a `persistViewPatch` call + * site that writes it — {@link narrowPersonalizationOverlay} is what a reader + * checks that against. `inlineEdit` is the one entry with no such call site, + * kept for reading rows written before objectui#5144. */ export const VIEW_OVERLAY_OWNED_KEYS = Object.freeze([ 'rowHeight', diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 0fadbc0a6e..0b0faa7c24 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -305,7 +305,13 @@ export interface ListViewProps { onSearchChange?: (search: string) => void; /** Called when the user toggles fields via the Hide Fields popover. */ onHiddenFieldsChange?: (hidden: string[]) => void; - /** Called when the user toggles inline record editing in View settings. */ + /** + * Called when the user toggles inline record editing. Wiring it is also what + * offers the wide toolbar's toggle. The toggle switches the grid's edit MODE, + * which `ListView` keeps in its own state. A host should not persist it into + * the view's `inlineEdit`: that key is the author's permission, and it folds + * into `userActions.editInline` (objectui#5144). + */ onInlineEditChange?: (next: boolean) => void; /** Called when the user resizes/reorders columns in the underlying grid. */ onColumnStateChange?: (state: { order?: string[]; widths?: Record }) => void; @@ -1771,16 +1777,17 @@ export const ListView = React.forwardRef(({ ); const [showHideFields, setShowHideFields] = React.useState(false); - // Inline-edit State (initialized from schema). Kept local — like hiddenFields - // — so the toolbar toggle flips the grid immediately. The parent persists via - // onInlineEditChange (debounced) and doesn't update the `inlineEdit` prop - // synchronously, so reading `schema.inlineEdit` directly would make the button - // appear dead until a full reload. + // Inline-edit MODE: session state, seeded from the view's `inlineEdit` on each + // load (objectui#5144, ruling E). The toolbar toggle flips it here and reports + // it through `onInlineEditChange`. The console writes nothing back, because + // `inlineEdit` is the author's permission key, so the mode is not remembered + // across loads. Whether the toggle is offered at all is `inlineEditOffered` + // below. const [inlineEdit, setInlineEdit] = React.useState(() => !!schema.inlineEdit); React.useEffect(() => { setInlineEdit(!!schema.inlineEdit); }, [schema.inlineEdit]); - // Setter that also notifies parent for persistence (debounced upstream). + // Setter that also notifies the host. const updateInlineEdit = React.useCallback( (next: boolean) => { setInlineEdit(next); @@ -1917,29 +1924,40 @@ export const ListView = React.forwardRef(({ * and the Studio designer keep today's behavior — the same fail-open the * bulk gate above relies on. * - * ## Gap 2 — consuming the declared `userActions.editInline` + * ## Gap 2 — the declared `userActions.editInline`, with the spec's default + * + * `ListViewSchema.userActions.editInline` is spec-declared with + * `.default(false)`: "the list is read-only unless the author opts in". It is + * read here with that default (`=== true`), so a view that declares nothing + * is not offered inline editing. That is the same reading the ADR-0047 + * interface page takes. + * + * A view's `inlineEdit` reaches this read through the fold, not around it + * (objectui#5144). `normalizeListViewSchema` folds a boolean `inlineEdit` + * into `userActions.editInline` when the view declares no `editInline` of its + * own. So: + * - a view with `inlineEdit: true` offers the toggle; + * - a view with `inlineEdit: false` reads off; + * - an explicit `editInline` wins over `inlineEdit`, either way; + * - a view with neither key reads off. * - * `ListViewSchema.userActions.editInline` is spec-declared and, on this - * toolbar, was read by nothing: an author could not switch inline editing off - * even unconditionally. It is read here as an explicit opt-OUT (`!== false`). + * Both keys are the author's permission. This gate decides whether the toggle + * is offered. Whether the grid is in edit mode is the `inlineEdit` state above: + * session state, seeded from the view's `inlineEdit` on each load, and flipped + * by the toggle. The console no longer persists the toggle into the view + * (objectui#5144, ruling E). That write made the toggle one-way, because + * switching it off stored `inlineEdit: false`, which this gate then read as + * "not offered". A view with `editInline: true` and no `inlineEdit` offers the + * toggle and opens out of edit mode. * - * That default is deliberate and it does NOT enforce the spec's - * `.default(false)`. Enforcing it would take the toggle away from every - * existing console list view in one release, since nothing folds a legacy key - * into `editInline` and no stored view declares it — the console's own - * channel for this capability is the view's `inlineEdit` property, which the - * host relays as `onInlineEditChange`. This is `toolbarFlags`' stated rule - * for exactly this block (defaults "matching what these flags have always - * done"; `hideFields`/`rowColor` keep their historical OFF because flipping - * them "would grow two buttons on every existing view") applied in the - * direction that would REMOVE one. So: an explicit `false` is honoured, an - * explicit `true` is honoured, and absence defers to the host channel that - * already governs this surface. `InterfaceListPage` — the other consumer of - * this key — reads the absent case as OFF (`=== true`), because the - * ADR-0047 interface page has no such host channel to defer to. + * ⚠️ The remedy for a view that relied on the old default: declare + * `userActions.editInline: true`. Two costs are recorded with the ruling. The + * edit mode is not remembered across loads. A view or overlay that already + * stores `inlineEdit: false`, written by the old toggle, still reads off; + * that is existing data, and it is not migrated. */ const inlineEditOffered = React.useMemo(() => { - if ((schema.userActions as Record | undefined)?.editInline === false) { + if ((schema.userActions as Record | undefined)?.editInline !== true) { return false; } return ( @@ -3501,8 +3519,8 @@ export const ListView = React.forwardRef(({ ...(schema.conditionalFormatting ? { conditionalFormatting: schema.conditionalFormatting } : {}), // [#4647] The MODE, not just its toggle. Gating only the toggle would // leave the issue's own consequence reachable by a different door: a - // stored view carrying `inlineEdit: true` (the console persists it - // per view) drops a read-only principal straight into editable cells + // stored view carrying `inlineEdit: true` (authored, or left by the + // console's old toggle) drops a read-only principal straight into editable cells // with no toggle to press, and "Save all" still earns the 403. The // toggle can only ever be the cheapest entrance to this state; the // state is what needs the grant. @@ -4760,8 +4778,8 @@ export const ListView = React.forwardRef(({
)} - {/* Inline edit — toggle record editing for this (grid) view. Persists - `inlineEdit` on the view via onInlineEditChange. + {/* Inline edit — toggle record editing for this (grid) view, for the + session (objectui#5144, ruling E). Reported via onInlineEditChange. [#4647] `inlineEditOffered` carries BOTH the `can(obj,'update')` permission gate this affordance was missing and the declared `userActions.editInline` switch — see its definition above. */} diff --git a/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx b/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx index 8ae42654b0..9e15b0d6e4 100644 --- a/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx @@ -304,16 +304,25 @@ function registerRecordingGrid() { }); } +/** + * The author's opt-in (objectui#5144). `userActions.editInline` is read with the + * spec's `.default(false)`, so a view that declares nothing is offered no inline + * editing at all. A permission-gate case opts in first. Without it, "the + * principal loses the toggle" would pass for every principal. + */ +const OPTED_IN = { userActions: { editInline: true } } as Partial; + /** `permissions: null` renders with NO PermissionProvider at all. */ function renderInlineEdit( permissions: ObjectPermissionConfig | null, schemaOverride?: Partial, + onInlineEditChange: (next: boolean) => void = vi.fn(), ) { const view = ( ); return render( @@ -346,27 +355,27 @@ describe('ListView – inline-edit toggle vs the principal permission gate (#464 }); it('a principal WITH update keeps the inline-edit toggle', async () => { - renderInlineEdit(makeUpdatePermissions(true)); + renderInlineEdit(makeUpdatePermissions(true), OPTED_IN); await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); expect(screen.queryByTestId('toolbar-inline-edit-toggle')).not.toBeNull(); }); it('a principal WITHOUT update loses it', async () => { - renderInlineEdit(makeUpdatePermissions(false)); + renderInlineEdit(makeUpdatePermissions(false), OPTED_IN); await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); expect(screen.queryByTestId('toolbar-inline-edit-toggle')).toBeNull(); }); it('with NO PermissionProvider the toggle survives (fail-open preserved)', async () => { - renderInlineEdit(null); + renderInlineEdit(null, OPTED_IN); await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); expect(screen.queryByTestId('toolbar-inline-edit-toggle')).not.toBeNull(); }); it('a read-only principal cannot reach edit MODE through a stored inlineEdit:true view', async () => { - // The door that stays open if only the toggle is gated: the console - // persists `inlineEdit` per view, so the mode can be entered with no - // toggle press at all. + // The door that stays open if only the toggle is gated: a view can store + // `inlineEdit: true`, so the mode can be entered with no toggle press at + // all. renderInlineEdit(makeUpdatePermissions(false), { inlineEdit: true } as Partial); await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); expect(screen.getByTestId('grid-stub')).toHaveTextContent('false'); @@ -443,12 +452,15 @@ describe('ListView – the declared userActions.editInline switch (#4647 gap 2)' }); it('`editInline: false` also withholds edit MODE from a stored inlineEdit:true view', async () => { + // objectui#5144: the explicit spec key wins over the stored `inlineEdit`, + // so the fold leaves `editInline: false` in place. The toggle goes as well. renderInlineEdit(makeUpdatePermissions(true), { inlineEdit: true, userActions: { editInline: false }, } as Partial); await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); expect(screen.getByTestId('grid-stub')).toHaveTextContent('false'); + expect(screen.queryByTestId('toolbar-inline-edit-toggle')).toBeNull(); }); it('`editInline: true` offers the toggle', async () => { @@ -459,17 +471,121 @@ describe('ListView – the declared userActions.editInline switch (#4647 gap 2)' expect(screen.queryByTestId('toolbar-inline-edit-toggle')).not.toBeNull(); }); - it('an ABSENT editInline defers to the host channel, keeping existing views intact', async () => { - // The deliberate divergence from the spec's `.default(false)`, pinned so a - // future change to it is a decision rather than an accident: enforcing that - // default would take the toggle off every stored console view at once, - // since nothing folds a legacy key into `editInline`. `toolbarFlags`' own - // rule for this block — defaults matching what the flags have always done — - // applied in the direction that would REMOVE an affordance. + it('an ABSENT editInline reads OFF, the spec default (objectui#5144)', async () => { + // This case used to pin the opposite: "an ABSENT editInline defers to the + // host channel, keeping existing views intact". That was the interim + // reading, kept because nothing folded the console's channel into + // `editInline`, so enforcing `.default(false)` would have taken the toggle + // off every stored console view. objectui#5144 is the maintainer's B-fold: + // `normalizeListViewSchema` now folds a stored `inlineEdit` into + // `editInline` (the stored-view cases below), so the spec default can be + // read as written. A view with neither key is not offered inline editing. renderInlineEdit(makeUpdatePermissions(true), { userActions: { search: false }, } as Partial); await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); + expect(screen.queryByTestId('toolbar-inline-edit-toggle')).toBeNull(); + expect(screen.getByTestId('grid-stub')).toHaveTextContent('false'); + }); + + it('…and the COMPACT entry reads off for an absent editInline too', async () => { + // The second render site of the same verdict. The entry's checkbox sits in + // a section whose `defaultOpen` is `!!inlineEdit`, so with `inlineEdit` + // absent it would be missing for every input. The section's TITLE renders + // whether or not the section is open, so the title is what this case and + // its control below look for. + renderInlineEdit(makeUpdatePermissions(true), { + compactToolbar: true, + } as Partial); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); + fireEvent.click(screen.getByTestId('view-settings-trigger')); + const content = await screen.findByTestId('view-settings-content'); + expect(content).not.toHaveTextContent('Record editing'); + }); + + it('…control: the same compact popover offers the entry once the view opts in', async () => { + renderInlineEdit(makeUpdatePermissions(true), { + compactToolbar: true, + ...OPTED_IN, + } as Partial); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); + fireEvent.click(screen.getByTestId('view-settings-trigger')); + const content = await screen.findByTestId('view-settings-content'); + expect(content).toHaveTextContent('Record editing'); + }); +}); + +/** + * objectui#5144 — a view's stored `inlineEdit`, read through the fold. + * + * Stored views carry `inlineEdit` (authored, or written by the console's old + * toolbar toggle) and no `userActions.editInline`. `normalizeListViewSchema` + * folds the first into the second, so every case here declares `inlineEdit` + * and leaves `editInline` out. The fold's own table is pinned in + * `@object-ui/core`'s `normalize-list-view.inlineEditFold-5144.test.ts`. This + * block pins what the toolbar does with it. + * + * Since triage's ruling E the console no longer persists the toggle, so it + * writes no new `inlineEdit` (pinned in `@object-ui/app-shell`'s + * `ObjectView.inlineEditSessionOnly-5144.test.tsx`). + */ +describe('ListView – a stored inlineEdit folds into editInline (objectui#5144)', () => { + let prevGrid: ReturnType; + + beforeEach(() => { + mockDataSource.find.mockClear(); + gridEditableCalls = []; + prevGrid = ComponentRegistry.get('object-grid'); + registerRecordingGrid(); + }); + + afterEach(() => { + cleanup(); + if (prevGrid) ComponentRegistry.register('object-grid', prevGrid); + else ComponentRegistry.unregister('object-grid'); + }); + + it('a stored `inlineEdit: true` folds to on: the toggle is offered and the grid opens editable', async () => { + renderInlineEdit(makeUpdatePermissions(true), { inlineEdit: true } as Partial); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); + expect(screen.queryByTestId('toolbar-inline-edit-toggle')).not.toBeNull(); + expect(screen.getByTestId('grid-stub')).toHaveTextContent('true'); + }); + + it('a stored `inlineEdit: false` reads off: no toggle and no edit mode', async () => { + // A recorded cost of triage's ruling E (objectui#5144): a view or overlay + // the old toolbar toggle left at `inlineEdit: false` stays off. It is + // existing data, and the maintainer's ruling rejects migrating it. The + // remedy is declaring `userActions.editInline: true` (the case below). + renderInlineEdit(makeUpdatePermissions(true), { inlineEdit: false } as Partial); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); + expect(screen.queryByTestId('toolbar-inline-edit-toggle')).toBeNull(); + expect(screen.getByTestId('grid-stub')).toHaveTextContent('false'); + }); + + it('an explicit `editInline: true` wins over a stored `inlineEdit: false`: offered, out of edit mode', async () => { + // The remedy the changeset names. The explicit spec key keeps the toggle + // offered, and the stored `inlineEdit` still decides the mode. + renderInlineEdit(makeUpdatePermissions(true), { + inlineEdit: false, + userActions: { editInline: true }, + } as Partial); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toBeInTheDocument()); expect(screen.queryByTestId('toolbar-inline-edit-toggle')).not.toBeNull(); + expect(screen.getByTestId('grid-stub')).toHaveTextContent('false'); + }); + + it('the toggle reports to the host: switching off reports `false` and leaves edit mode', async () => { + // The toggle flips the session's edit mode and reports it through + // `onInlineEditChange`. Since ruling E (objectui#5144) the console writes + // nothing for it, so the next load is seeded from the view's own + // `inlineEdit` again. + const onInlineEditChange = vi.fn(); + renderInlineEdit(makeUpdatePermissions(true), { inlineEdit: true } as Partial, onInlineEditChange); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toHaveTextContent('true')); + fireEvent.click(screen.getByTestId('toolbar-inline-edit-toggle')); + await waitFor(() => expect(screen.getByTestId('grid-stub')).toHaveTextContent('false')); + expect(onInlineEditChange).toHaveBeenCalledTimes(1); + expect(onInlineEditChange).toHaveBeenLastCalledWith(false); }); }); diff --git a/packages/plugin-list/src/__tests__/ListView.test.tsx b/packages/plugin-list/src/__tests__/ListView.test.tsx index ba26acdd0d..5cea7c40a0 100644 --- a/packages/plugin-list/src/__tests__/ListView.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.test.tsx @@ -2654,6 +2654,10 @@ describe('ListView — inline-edit toggle drives grid editability', () => { objectName: 'contacts', viewType: 'grid', fields: ['name', 'email'], + // objectui#5144: inline editing is offered only where the view opts in + // (the spec's `.default(false)`). This case is about the toggle, so the + // view opts in and starts out of edit mode. + userActions: { editInline: true }, }; renderWithProvider( @@ -2684,6 +2688,8 @@ describe('ListView — inline-edit toggle drives grid editability', () => { objectName: 'contacts', viewType: 'grid', fields: ['name', 'email'], + // objectui#5144: the view opts in, as in the case above. + userActions: { editInline: true }, }; renderWithProvider( diff --git a/packages/plugin-list/src/components/ViewSettingsPopover.tsx b/packages/plugin-list/src/components/ViewSettingsPopover.tsx index 53d23814b6..04974e1eac 100644 --- a/packages/plugin-list/src/components/ViewSettingsPopover.tsx +++ b/packages/plugin-list/src/components/ViewSettingsPopover.tsx @@ -73,7 +73,12 @@ export interface ViewSettingsPopoverProps { hiddenFields?: Set; updateHiddenFields?: (next: Set) => void; - /** Record editing — toggle inline cell editing (persists `inlineEdit` on the view). */ + /** + * Record editing — toggle the grid's inline-edit mode. `setInlineEdit` hands + * the change to the host, which decides what it does with it; the console + * keeps it session-only and writes nothing to the view (objectui#5144, + * ruling E). + */ showInlineEdit?: boolean; inlineEdit?: boolean; setInlineEdit?: (next: boolean) => void; diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index 80b144c54a..2b53115c2a 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -2930,7 +2930,20 @@ export const ObjectView: React.FC = ({ // folds in LAST. Spread rather than `??` on purpose: this slot is a // merge of toggle sets, not a winner-takes-all pick, and a named // view that toggles one action must not blank the rest. - ...currentNamedViewConfig?.userActions, + // + // objectui#5144 — through the same fold as the two layers above. + // A named view carries no `show*` flag (its strict record refuses + // them), but it can carry the spec's `inlineEdit`, which the fold + // turns into `userActions.editInline`. Spread raw, a host view's or + // the node's folded `inlineEdit` outranked the named view's own, + // the inverse of the named-first `inlineEdit` rung below. The fold + // is handed the two members it reads into `userActions` here, by + // name rather than the whole view, so the fence's read census + // (`objectViewHostSurface.test.tsx`) still sees each one. + ...(normalizeListViewSchema({ + userActions: currentNamedViewConfig?.userActions, + inlineEdit: currentNamedViewConfig?.inlineEdit, + }) as { userActions?: object }).userActions, }, compactToolbar: currentNamedViewConfig?.compactToolbar ?? activeView?.compactToolbar ?? (schema as any).compactToolbar, // objectui#11013 — the host `views` entry is no longer read for diff --git a/packages/plugin-view/src/__tests__/ObjectView.namedViewEditInlineFold-5144.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.namedViewEditInlineFold-5144.test.tsx new file mode 100644 index 0000000000..2fef8dd5f8 --- /dev/null +++ b/packages/plugin-view/src/__tests__/ObjectView.namedViewEditInlineFold-5144.test.tsx @@ -0,0 +1,113 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#5144 — on a host's `renderListView`, the named view's `inlineEdit` + * outranks the host's, for whether inline editing is offered too. + * + * `normalizeListViewSchema` folds a view's `inlineEdit` into + * `userActions.editInline`, and `ListView` offers inline editing only where + * that reads `true`. The relay that builds the `list-view` node merges + * `userActions` across three layers: the node, the host's `views` entry, and + * the named view, most specific last. It folded the first two and spread the + * named view's raw. So once the fold existed, a host-layer `inlineEdit` became + * a folded `editInline` that outranked the named view's own `inlineEdit`, the + * inverse of the named-first `inlineEdit` rung on the same node. The seat's + * decision on the card folds the named layer like the other two. + * + * Each case reads what `ListView` reads: the node handed to `renderListView`, + * through the fold. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { normalizeListViewSchema } from '@object-ui/core'; +import { ObjectView } from '../ObjectView'; +import type { ObjectViewSchema } from '@object-ui/types'; + +vi.mock('@object-ui/react', async (importOriginal) => { + const React = await import('react'); + return { + ...(await importOriginal>()), + SchemaRenderer: () => null, + SchemaRendererContext: React.createContext(null), + subscribeDataChanges: () => () => {}, + notifyDataChanged: () => {}, + }; +}); +vi.mock('@object-ui/plugin-grid', async (importOriginal) => ({ + ...(await importOriginal>()), + ObjectGrid: () => null, +})); +vi.mock('@object-ui/plugin-form', async (importOriginal) => ({ + ...(await importOriginal>()), + ObjectForm: () => null, +})); + +const dataSource = (): any => ({ + find: vi.fn().mockResolvedValue({ data: [], total: 0 }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'task', fields: {} }), +}); + +type Layer = Record; + +/** + * The `editInline` and `inlineEdit` `ListView` reads off the node a host + * receives, for a named view and a host `views` entry. + */ +function read(named: Layer, host: Layer | null): { editInline: unknown; inlineEdit: unknown } { + const seen: Record[] = []; + render( + }) => { + seen.push(schema); + return
; + }} + />, + ); + expect(seen.length).toBeGreaterThan(0); + // The named view was selected: its label is on the node. + expect(seen[0].label).toBe('Open work'); + const node = normalizeListViewSchema(seen[0]) as { userActions?: Layer; inlineEdit?: unknown }; + return { editInline: node.userActions?.editInline, inlineEdit: node.inlineEdit }; +} + +beforeEach(() => { + cleanup(); +}); + +describe('objectui#5144 — the named view`s inlineEdit outranks the host`s on renderListView', () => { + it('named `inlineEdit: true` over host `inlineEdit: false` reads offered', () => { + expect(read({ inlineEdit: true }, { inlineEdit: false })).toEqual({ editInline: true, inlineEdit: true }); + }); + + it('named `inlineEdit: false` over host `inlineEdit: true` reads off', () => { + expect(read({ inlineEdit: false }, { inlineEdit: true })).toEqual({ editInline: false, inlineEdit: false }); + }); + + it('control: a named view that says nothing leaves the host`s `inlineEdit` in force', () => { + // Proves the host layer reaches this node at all, so the two cases above + // read a precedence and not a host layer the relay never saw. + expect(read({}, { inlineEdit: true })).toEqual({ editInline: true, inlineEdit: true }); + }); + + it('control: inside the named view, an explicit `userActions.editInline` still wins over its `inlineEdit`', () => { + expect(read({ inlineEdit: true, userActions: { editInline: false } }, null)).toEqual({ editInline: false, inlineEdit: true }); + }); +});