diff --git a/.changeset/kanban-calendar-nav-width-converge.md b/.changeset/kanban-calendar-nav-width-converge.md new file mode 100644 index 000000000..7f827c27d --- /dev/null +++ b/.changeset/kanban-calendar-nav-width-converge.md @@ -0,0 +1,22 @@ +--- +--- + +`ObjectKanban` and `ObjectCalendar` internal cleanup, measured as a zero-pixel +change — the convergence of the two renderers #6305 left behind. + +Each carried the house default `min(960px, 60vw)` at **two** sites: the +`navConfig` default (`{ mode: 'drawer', width: 'min(960px, 60vw)' }`) and a +render-site `width={(navigation.width as any) ?? 'min(960px, 60vw)'}`. Both are +gone. `width` is spec-deprecated (`@deprecated [#2578 -> size]`) and +`resolveOverlayWidth` gives an explicit `width` priority OVER `size`, so +spelling it kept the deprecated branch load-bearing on the path most boards and +calendars take. With both omitted, `resolveOverlayWidth` returns `undefined` and +`RecordDetailDrawer`'s own `width` default supplies the identical +`min(960px, 60vw)`. + +The resolved overlay width is therefore unchanged on every viewport, for both a +board/calendar that declares no `navigation` and one that authors +`navigation.width` — each is now pinned by a test. The three renderers agree +again. + +No published behaviour changes. diff --git a/packages/plugin-calendar/src/ObjectCalendar.navWidthDefault.test.tsx b/packages/plugin-calendar/src/ObjectCalendar.navWidthDefault.test.tsx new file mode 100644 index 000000000..f03ca1ff0 --- /dev/null +++ b/packages/plugin-calendar/src/ObjectCalendar.navWidthDefault.test.tsx @@ -0,0 +1,139 @@ +/** + * 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. + */ + +/** + * Pins the drawer width a calendar gets when it declares no `navigation` + * (objectui#6303 — the sibling of `ObjectGantt.navWidthDefault.test.tsx`). + * + * `ObjectCalendar` used to spell `min(960px, 60vw)` in TWO places: the + * `navConfig` default (`{ mode: 'drawer', width: 'min(960px, 60vw)' }`) and, + * further down, a render-site + * `width={(navigation.width as any) ?? 'min(960px, 60vw)'}`. The second is why + * taking only the first would have changed nothing — the old width survived by + * a different route. + * + * `width` is `@deprecated [#2578 -> size]` and `resolveOverlayWidth` gives an + * explicit `width` priority OVER `size`, so spelling it kept the deprecated + * branch load-bearing on the path most calendars take. The default is now + * `{ mode: 'drawer' }` with no render-site fallback: `resolveOverlayWidth` + * returns `undefined` and RecordDetailDrawer's own `width` default supplies the + * identical CSS — a zero-pixel change. + * + * Both halves below are load-bearing and fail for different reasons: + * + * half 1 — the calendar must stop injecting a width of its own (it has to + * hand `undefined` down, or the drawer's default can never apply). + * This half is what catches a re-added `??` fallback at the render + * site, which half 2 alone cannot see: the fallback's value is the + * same string the drawer default produces; + * half 2 — the width the REAL drawer then resolves must still be that value. + * Without this half the calendar would follow a moved drawer default + * invisibly, which is the regression the indirection introduces. + * + * A third case pins the other direction: an AUTHORED `navigation.width` still + * wins. What #6303 removed is the renderer spelling the deprecated key as its + * own default — not the key's acceptance as an authored value. + * + * All three assert the resolved width VALUE — never a `className`, never "it + * renders", either of which passes in both worlds. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { ObjectCalendar } from './ObjectCalendar'; + +/** The width the calendar's drawer has always resolved to. Must not drift. */ +const EXPECTED_WIDTH = 'min(960px, 60vw)'; + +// Record the props ObjectCalendar hands down, then delegate to the REAL drawer +// so half 2 measures the actual resolution rather than a stub's idea of it. +let drawerProps: any = null; +vi.mock('@object-ui/plugin-detail', async (importOriginal) => { + const actual = await importOriginal(); + const Real = actual.RecordDetailDrawer; + return { + ...actual, + RecordDetailDrawer: (props: any) => { + drawerProps = props; + return ; + }, + }; +}); + +/** + * The month grid renders whatever month `currentDate` is on, and `currentDate` + * initialises to `new Date()` — so the event has to sit in the CURRENT month or + * there is nothing on screen to click. Noon avoids a timezone shift moving it + * across a month boundary. + */ +function eventInCurrentMonth() { + const now = new Date(); + return new Date(now.getFullYear(), now.getMonth(), 15, 12, 0, 0).toISOString(); +} + +async function openDrawer(navigation?: Record) { + render( + , + ); + const event = await screen.findByText('On the calendar'); + fireEvent.click(event); + await waitFor(() => expect(drawerProps).not.toBeNull()); + await waitFor(() => expect(screen.getByRole('dialog')).toBeDefined()); +} + +/** + * The drawer prefers a drag-resized width persisted in localStorage over its + * prop, which would mask half 2 — and it is keyed by `objectName`, so a value + * left by any earlier test in the file would be read back here. + */ +function readPanelWidth(): string { + const panel = document.querySelector('[role="dialog"]') as HTMLElement | null; + expect(panel, 'drawer panel').not.toBeNull(); + // The drawer applies the resolved width as an inline style on its panel, as + // BOTH `width` and `max-width`. happy-dom's CSS parser drops the `width` + // longhand when the value is a `min()` expression but keeps `max-width`, so + // the surviving declaration is what we read — it is the same resolved + // string, not a proxy for it. + return panel!.style.maxWidth; +} + +describe('calendar drawer width with no declared `navigation` (objectui#6303)', () => { + beforeEach(() => { + drawerProps = null; + try { window.localStorage.clear(); } catch { /* ignore */ } + }); + afterEach(() => cleanup()); + + it('half 1: the calendar injects no width of its own (so the drawer default applies)', async () => { + await openDrawer(); + expect(drawerProps.width).toBeUndefined(); + }); + + it('half 2: the width the real drawer resolves is still the pinned value', async () => { + await openDrawer(); + expect(readPanelWidth()).toBe(EXPECTED_WIDTH); + }); + + it('an authored `navigation.width` still reaches the drawer unchanged', async () => { + await openDrawer({ mode: 'drawer', width: '720px' }); + expect(drawerProps.width).toBe('720px'); + expect(readPanelWidth()).toBe('720px'); + }); +}); diff --git a/packages/plugin-calendar/src/ObjectCalendar.tsx b/packages/plugin-calendar/src/ObjectCalendar.tsx index 45ccff06a..2592c8aa3 100644 --- a/packages/plugin-calendar/src/ObjectCalendar.tsx +++ b/packages/plugin-calendar/src/ObjectCalendar.tsx @@ -446,7 +446,24 @@ export const ObjectCalendar: React.FC = ({ // Must be called before any early returns to satisfy React hooks rules // When the local navigation mode is an overlay (drawer/modal), ignore the // inherited onRowClick so the local overlay wins over parent page-nav. - const navConfig = (schema as any).navigation ?? { mode: 'drawer', width: 'min(960px, 60vw)' }; + // No width is spelled here on purpose (objectui#6303, converging the calendar + // on the shape #6305 gave ObjectGantt). `width` is `@deprecated [#2578 -> + // size]` in the spec that owns this shape, and `resolveOverlayWidth` gives an + // explicit `width` priority OVER `size` — so spelling it kept the deprecated + // branch load-bearing on the path most calendars take (no declared + // `navigation`), and made the size buckets unreachable there. Omitting both + // leaves `resolveOverlayWidth` returning `undefined`, which is what + // RecordDetailDrawer's own `width` default is for; that default is the + // identical `min(960px, 60vw)`, so this is a zero-pixel change on every + // viewport. The absent width is deliberate, not an oversight — do not + // "restore" it. Pinned by `ObjectCalendar.navWidthDefault.test.tsx`, both + // halves, because the equivalence now depends on the drawer's default too. + // + // Deliberately NOT converged on `size: 'lg'` either: that bucket is + // `min(92vw, 960px)`, which agrees with the above only at viewport >= 1600px + // and is up to 53% wider below it. That move is a real behaviour change and + // stays open on #6303 for a human ruling. + const navConfig = (schema as any).navigation ?? { mode: 'drawer' }; const navIsOverlay = navConfig.mode === 'drawer' || navConfig.mode === 'modal' || navConfig.mode === 'split' || navConfig.mode === 'popover'; const navigation = useNavigationOverlay({ navigation: navConfig, @@ -760,7 +777,10 @@ export const ObjectCalendar: React.FC = ({ recordId={recordId} dataSource={dataSource} objectSchema={objectSchema as any} - width={(navigation.width as any) ?? 'min(960px, 60vw)'} + // No `?? 'min(960px, 60vw)'` fallback on purpose — `undefined` has + // to reach the drawer for its OWN identical default to apply. See + // the `navConfig` comment above (objectui#6303). + width={navigation.width as any} fullPageHref={deriveRecordPageHref(objectName, recordId) ?? undefined} onFieldSave={async (field, value) => { if (!dataSource?.update) return; diff --git a/packages/plugin-kanban/src/ObjectKanban.navWidthDefault.test.tsx b/packages/plugin-kanban/src/ObjectKanban.navWidthDefault.test.tsx new file mode 100644 index 000000000..cacc1f460 --- /dev/null +++ b/packages/plugin-kanban/src/ObjectKanban.navWidthDefault.test.tsx @@ -0,0 +1,134 @@ +/** + * 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. + */ + +/** + * Pins the drawer width a kanban gets when it declares no `navigation` + * (objectui#6303 — the sibling of `ObjectGantt.navWidthDefault.test.tsx`). + * + * `ObjectKanban` used to spell `min(960px, 60vw)` in TWO places: the `navConfig` + * default (`{ mode: 'drawer', width: 'min(960px, 60vw)' }`) and, further down, a + * render-site `width={(navigation.width as any) ?? 'min(960px, 60vw)'}`. The + * second is why taking only the first would have changed nothing — the old + * width survived by a different route. + * + * `width` is `@deprecated [#2578 -> size]` and `resolveOverlayWidth` gives an + * explicit `width` priority OVER `size`, so spelling it kept the deprecated + * branch load-bearing on the path most boards take. The default is now + * `{ mode: 'drawer' }` with no render-site fallback: `resolveOverlayWidth` + * returns `undefined` and RecordDetailDrawer's own `width` default supplies the + * identical CSS — a zero-pixel change. + * + * Both halves below are load-bearing and fail for different reasons: + * + * half 1 — the kanban must stop injecting a width of its own (it has to hand + * `undefined` down, or the drawer's default can never apply). This + * half is what catches a re-added `??` fallback at the render site, + * which half 2 alone cannot see: the fallback's value is the same + * string the drawer default produces; + * half 2 — the width the REAL drawer then resolves must still be that value. + * Without this half the kanban would follow a moved drawer default + * invisibly, which is the regression the indirection introduces. + * + * A third case pins the other direction: an AUTHORED `navigation.width` still + * wins. What #6303 removed is the renderer spelling the deprecated key as its + * own default — not the key's acceptance as an authored value. + * + * All three assert the resolved width VALUE — never a `className`, never "it + * renders", either of which passes in both worlds. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { ObjectKanban } from './ObjectKanban'; + +// Pay the board's lazy chunk at import time, not inside a `findBy` budget +// (AGENTS.md 测试纪律) — every assertion below sits after the Suspense +// boundary, and a card has to be on screen before it can be clicked. The +// specifier must stay byte-identical to the one in `./index`, which is what +// makes the component's own lazy factory resolve immediately. +import './KanbanImpl'; + +/** The width the kanban's drawer has always resolved to. Must not drift. */ +const EXPECTED_WIDTH = 'min(960px, 60vw)'; + +// Record the props ObjectKanban hands down, then delegate to the REAL drawer so +// half 2 measures the actual resolution rather than a stub's idea of it. +let drawerProps: any = null; +vi.mock('@object-ui/plugin-detail', async (importOriginal) => { + const actual = await importOriginal(); + const Real = actual.RecordDetailDrawer; + return { + ...actual, + RecordDetailDrawer: (props: any) => { + drawerProps = props; + return ; + }, + }; +}); + +const cards = [{ id: '1', title: 'On the board', status: 'todo' }]; + +async function openDrawer(navigation?: Record) { + render( + , + ); + const card = await screen.findByText('On the board'); + fireEvent.click(card); + await waitFor(() => expect(drawerProps).not.toBeNull()); + await waitFor(() => expect(screen.getByRole('dialog')).toBeDefined()); +} + +/** + * The drawer prefers a drag-resized width persisted in localStorage over its + * prop, which would mask half 2 — and it is keyed by `objectName`, so a value + * left by any earlier test in the file would be read back here. + */ +function readPanelWidth(): string { + const panel = document.querySelector('[role="dialog"]') as HTMLElement | null; + expect(panel, 'drawer panel').not.toBeNull(); + // The drawer applies the resolved width as an inline style on its panel, as + // BOTH `width` and `max-width`. happy-dom's CSS parser drops the `width` + // longhand when the value is a `min()` expression but keeps `max-width`, so + // the surviving declaration is what we read — it is the same resolved + // string, not a proxy for it. + return panel!.style.maxWidth; +} + +describe('kanban drawer width with no declared `navigation` (objectui#6303)', () => { + beforeEach(() => { + drawerProps = null; + try { window.localStorage.clear(); } catch { /* ignore */ } + }); + afterEach(() => cleanup()); + + it('half 1: the kanban injects no width of its own (so the drawer default applies)', async () => { + await openDrawer(); + expect(drawerProps.width).toBeUndefined(); + }); + + it('half 2: the width the real drawer resolves is still the pinned value', async () => { + await openDrawer(); + expect(readPanelWidth()).toBe(EXPECTED_WIDTH); + }); + + it('an authored `navigation.width` still reaches the drawer unchanged', async () => { + await openDrawer({ mode: 'drawer', width: '720px' }); + expect(drawerProps.width).toBe('720px'); + expect(readPanelWidth()).toBe('720px'); + }); +}); diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 1e99dfa64..ace66b960 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -690,7 +690,27 @@ export const ObjectKanban: React.FC = ({ ...(effectiveSwimlaneField ? { swimlaneField: effectiveSwimlaneField } : {}), }; - const navConfig = (schema as any).navigation ?? { mode: 'drawer', width: 'min(960px, 60vw)' }; + // Default to a right-side drawer so clicking a card opens an editable detail + // panel inline. A schema can override this with its own `navigation` config. + // + // No width is spelled here on purpose (objectui#6303, converging kanban on + // the shape #6305 gave ObjectGantt). `width` is `@deprecated [#2578 -> size]` + // in the spec that owns this shape, and `resolveOverlayWidth` gives an + // explicit `width` priority OVER `size` — so spelling it kept the deprecated + // branch load-bearing on the path most boards take (no declared + // `navigation`), and made the size buckets unreachable there. Omitting both + // leaves `resolveOverlayWidth` returning `undefined`, which is what + // RecordDetailDrawer's own `width` default is for; that default is the + // identical `min(960px, 60vw)`, so this is a zero-pixel change on every + // viewport. The absent width is deliberate, not an oversight — do not + // "restore" it. Pinned by `ObjectKanban.navWidthDefault.test.tsx`, both + // halves, because the equivalence now depends on the drawer's default too. + // + // Deliberately NOT converged on `size: 'lg'` either: that bucket is + // `min(92vw, 960px)`, which agrees with the above only at viewport >= 1600px + // and is up to 53% wider below it. That move is a real behaviour change and + // stays open on #6303 for a human ruling. + const navConfig = (schema as any).navigation ?? { mode: 'drawer' }; // When this kanban is embedded in an ObjectView, the parent provides // `onRowClick`/`onCardClick` and owns the unified record-detail overlay. // We must always forward to the parent in that case — otherwise we'd open @@ -992,7 +1012,10 @@ export const ObjectKanban: React.FC = ({ recordId={recordId} dataSource={dataSource} objectSchema={objectDef as any} - width={(navigation.width as any) ?? 'min(960px, 60vw)'} + // No `?? 'min(960px, 60vw)'` fallback on purpose — `undefined` has + // to reach the drawer for its OWN identical default to apply. See + // the `navConfig` comment above (objectui#6303). + width={navigation.width as any} fullPageHref={deriveRecordPageHref(objectName, recordId) ?? undefined} onFieldSave={async (field, value) => { if (!dataSource?.update) return;