Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/kanban-calendar-nav-width-converge.md
Original file line numberDiff line numberDiff line change
@@ -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.
139 changes: 139 additions & 0 deletions packages/plugin-calendar/src/ObjectCalendar.navWidthDefault.test.tsx
Original file line numberDiff line numberDiff line change
@@ -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<any>();
const Real = actual.RecordDetailDrawer;
return {
...actual,
RecordDetailDrawer: (props: any) => {
drawerProps = props;
return <Real {...props} />;
},
};
});

/**
* 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<string, unknown>) {
render(
<ObjectCalendar
schema={{
type: 'object-calendar',
objectName: 'events',
calendar: { startDateField: 'starts_at', titleField: 'name' },
data: {
provider: 'value',
items: [{ id: '1', name: 'On the calendar', starts_at: eventInCurrentMonth() }],
},
...(navigation ? { navigation } : {}),
} as never}
/>,
);
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');
});
});
24 changes: 22 additions & 2 deletions packages/plugin-calendar/src/ObjectCalendar.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -446,7 +446,24 @@ export const ObjectCalendar: React.FC<ObjectCalendarComponentProps> = ({
// 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,
Expand DownExpand Up@@ -760,7 +777,10 @@ export const ObjectCalendar: React.FC<ObjectCalendarComponentProps> = ({
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;
Expand Down
134 changes: 134 additions & 0 deletions packages/plugin-kanban/src/ObjectKanban.navWidthDefault.test.tsx
Original file line numberDiff line numberDiff line change
@@ -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<any>();
const Real = actual.RecordDetailDrawer;
return {
...actual,
RecordDetailDrawer: (props: any) => {
drawerProps = props;
return <Real {...props} />;
},
};
});

const cards = [{ id: '1', title: 'On the board', status: 'todo' }];

async function openDrawer(navigation?: Record<string, unknown>) {
render(
<ObjectKanban
schema={{
type: 'object-kanban',
objectName: 'contacts',
groupBy: 'status',
columns: [{ id: 'todo', title: 'To Do' }],
data: cards,
...(navigation ? { navigation } : {}),
} as never}
/>,
);
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');
});
});
27 changes: 25 additions & 2 deletions packages/plugin-kanban/src/ObjectKanban.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -690,7 +690,27 @@ export const ObjectKanban: React.FC<ObjectKanbanComponentProps> = ({
...(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
Expand DownExpand Up@@ -992,7 +1012,10 @@ export const ObjectKanban: React.FC<ObjectKanbanComponentProps> = ({
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;
Expand Down
Loading