diff --git a/.changeset/calendar-inert-inputs-4454-4493.md b/.changeset/calendar-inert-inputs-4454-4493.md new file mode 100644 index 000000000..691cf358f --- /dev/null +++ b/.changeset/calendar-inert-inputs-4454-4493.md @@ -0,0 +1,53 @@ +--- +'@object-ui/plugin-calendar': minor +--- + +`calendar-view` has no declared-but-inert inputs left: `allowCreate` works, `colorMapping` is retired (objectui#4454, objectui#4493) + +Two of this widget's registry inputs were declared and read by nobody — the one +state ADR-0049's enforce-or-remove framing says must not persist. Measurement +answered them in opposite directions. + +**`allowCreate` is enforced.** The handler it would gate was already built in the +renderer — `handleAddClick`, dispatching `{ type: 'create', payload: {} }` on the +widget's own `onAction` channel — and simply never passed. `CalendarView` renders +its **New event** button behind `onAddClick`, so on the SDUI path that button +never existed and the handler was unreachable: both halves of one feature were +present and had never been introduced to each other. An authored +`allowCreate: true` now supplies the handler to `onAddClick`, and clicking the +button dispatches the create action. + +The wiring goes through the declared `onAddClick` hatch rather than around it via +a second prop. That key is already one of the renderer's function-typed host +hatches (objectui#4453), so a React host could switch the affordance on today and +that path is untouched — a host handler still replaces the action dispatch rather +than running alongside it, the same precedence `onEventClick` keeps. An authored +`onAddClick` string is still dropped, so turning the affordance on cannot +reintroduce that card's uncaught handler crash. + +Only the boolean `true` turns it on. Absent, `false`, and the off-type spellings +JSON invites (`'true'`, `1`, an object) all resolve to the absent-key answer, which +on this prop is literally what makes the button not render. Every node that never +authored the key renders exactly as before. + +**`colorMapping` is removed.** It had no read site anywhere: the renderer's event +mapping takes the colour straight off the record (`color: record[colorField]`), +and `CalendarView` resolves a colour from `event.color`. An author who wrote the +documented `colorMapping: { meeting: 'blue' }` got no mapping, no warning and no +error — the raw field value was used as the colour, which for a picklist value +like `meeting` is not a colour at all. It is retired rather than implemented +because no measured app authors it, and a capability with no pull behind it is not +worth building. The `content/docs/plugins/plugin-calendar.mdx` schema-API line +documenting it is removed in the same change. + +Retiring it is not a behaviour change — the key never had a read site to lose. It +becomes an ordinary unknown authored key, dropped at the renderer boundary like +any other. + +**Grade.** Minor, not patch: measured both ways against the emitted bundle, the +published registry surface moves — `calendar-view`'s `inputs` array loses a member +(`colorMapping`: 1 emitted declaration before, 0 after), so the authorable +vocabulary this widget publishes narrows by one key, and a second declared input +starts producing a user-visible affordance. The emitted `.d.ts` is byte-identical +either way; the vocabulary lives in the runtime registry metadata, not in the type +surface. diff --git a/content/docs/plugins/plugin-calendar.mdx b/content/docs/plugins/plugin-calendar.mdx index a95312159..a2332e035 100644 --- a/content/docs/plugins/plugin-calendar.mdx +++ b/content/docs/plugins/plugin-calendar.mdx @@ -137,7 +137,6 @@ const schema = { endDateField?: string, allDayField?: string, colorField?: string, - colorMapping?: Record, currentDate?: string, allowCreate?: boolean, onEventClick?: (event: any) => void, @@ -148,6 +147,28 @@ const schema = { } ``` +### Allowing event creation + +`allowCreate: true` adds the header's **New event** button. Clicking it +dispatches the standard create action — `{ type: 'create', payload: {} }` — on +the same action channel every other `calendar-view` gesture uses, so the host +decides what a "create" means (open a form, navigate, call an API). + +```tsx +const schema = { + type: 'calendar-view', + data: events, + allowCreate: true, +} +``` + +Only the boolean `true` turns the button on. Omitting the key, `false`, and any +non-boolean value all render the calendar without the button — the default. + +A React host can supply the affordance directly instead, with or without +`allowCreate`, by passing its own `onAddClick` handler; an explicit handler +replaces the action dispatch rather than running alongside it. + ## ObjectCalendar Component Calendar component designed for use with ObjectQL data sources. diff --git a/packages/plugin-calendar/src/calendar-view-renderer.inertInputs.test.tsx b/packages/plugin-calendar/src/calendar-view-renderer.inertInputs.test.tsx new file mode 100644 index 000000000..1fa81d27b --- /dev/null +++ b/packages/plugin-calendar/src/calendar-view-renderer.inertInputs.test.tsx @@ -0,0 +1,400 @@ +/** + * 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. + */ + +/** + * `plugin-calendar:calendar-view` — the two declared-but-inert inputs, decided + * by measurement in OPPOSITE directions (objectui#4454, objectui#4493). + * + * Both keys were the same defect on paper — an authorable input declared in the + * registry with nothing reading it, the one state ADR-0049's enforce-or-remove + * framing says must not persist — and the measurement separated them: + * + * - `allowCreate` (objectui#4454) is ENFORCED. The handler it would gate was + * already BUILT in the renderer (`handleAddClick`, dispatching + * `{ type: 'create' }` on the component's own `onAction` channel) and simply + * never passed, so the intent existed and the wiring was the missing line. + * - `colorMapping` (objectui#4493) is REMOVED. Nothing anywhere read it — not + * the renderer, not `CalendarView`, which resolves a colour from + * `event.color` — and no measured app authors it, so the declaration is + * retired rather than given an implementation nobody asked for. + * + * ## Why `onAddClick` and not a new prop + * + * `CalendarView` renders its "New event" button behind `{onAddClick && …}` and + * declares `onAddClick?: () => void`. Post-objectui#4453 that key is one of the + * renderer's DECLARED function-typed host hatches (`HOST_CALLBACKS`), so the + * affordance already had a live host path. `allowCreate` therefore goes THROUGH + * that hatch — an authored `true` supplies the renderer's own handler as the + * hatch's value — rather than around it via a second, parallel prop. One key, + * one declared type, one answer. + * + * ## The absent-key answer is load-bearing + * + * `undefined` is what makes the button not render, and it is the answer for + * every input that is not the boolean `true`: absent, `false`, and the off-type + * spellings an author can write in JSON (`'true'`, `1`). That is the same rule + * every other resolver at this boundary follows, and it is what keeps + * today's behaviour byte-identical for everyone who never authored the key. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer } from '@object-ui/react'; +// Module scope: the registration side effect this file renders through +// (AGENTS.md 测试纪律 — never inside a hook). +import './index'; + +/** The text `SchemaErrorBoundary` renders when a widget throws during RENDER. */ +const ERROR_BOUNDARY_MARKER = 'failed to render'; + +/** + * The add affordance's accessible name — `CalendarView`'s own + * `t('calendar.newEvent')`, whose English default is "New event". + */ +const ADD_BUTTON_NAME = 'New event'; + +/** Two records in the CURRENT month, so the default month view shows them. */ +function currentMonthRecords() { + const now = new Date(); + const day = (n: number) => + new Date(now.getFullYear(), now.getMonth(), n, 10, 0, 0, 0).toISOString(); + return [ + { id: 'r1', title: 'Computed Standup', start: day(10) }, + { id: 'r2', title: 'Computed Review', start: day(12) }, + ]; +} + +function calendarRegion(): Element | null { + return document.body.querySelector('[role="region"][aria-label="Calendar"]'); +} + +async function expectCalendarRendered() { + await waitFor(() => expect(calendarRegion()).not.toBeNull()); + expect(document.body.textContent ?? '').not.toContain(ERROR_BOUNDARY_MARKER); +} + +function addButton(): HTMLElement | null { + return screen.queryByRole('button', { name: ADD_BUTTON_NAME }); +} + +describe('calendar-view: `allowCreate` drives the add affordance (objectui#4454)', () => { + it('authored `allowCreate: true` on the NODE renders the add affordance, and clicking it dispatches `create`', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + const onAction = vi.fn(); + try { + render( + , + ); + + await expectCalendarRendered(); + + // Before the fix: `handleAddClick` was built and never passed, so + // `CalendarView` saw no `onAddClick` and this button did not exist. + const button = addButton(); + expect(button).not.toBeNull(); + + fireEvent.click(button as HTMLElement); + + // The behaviour the flag turns on is the renderer's OWN action channel — + // the same `onAction` every other gesture on this widget dispatches to. + expect(onAction).toHaveBeenCalledWith({ type: 'create', payload: {} }); + } finally { + errors.mockRestore(); + } + }); + + it('authored `allowCreate: true` in the `props` CONTAINER does the same', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + const onAction = vi.fn(); + try { + render( + , + ); + + await expectCalendarRendered(); + const button = addButton(); + expect(button).not.toBeNull(); + + fireEvent.click(button as HTMLElement); + expect(onAction).toHaveBeenCalledWith({ type: 'create', payload: {} }); + } finally { + errors.mockRestore(); + } + }); + + it('MUST-NOT-CHANGE: an ABSENT `allowCreate` renders no add affordance', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + await expectCalendarRendered(); + expect(addButton()).toBeNull(); + } finally { + errors.mockRestore(); + } + }); + + it('MUST-NOT-CHANGE: an explicit `allowCreate: false` renders no add affordance', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + await expectCalendarRendered(); + expect(addButton()).toBeNull(); + } finally { + errors.mockRestore(); + } + }); + + it.each([ + ['the string `true`', 'true'], + ['the number 1', 1], + ['an object', { enabled: true }], + ])( + 'off-type `allowCreate` (%s) gets the absent-key answer — no affordance, no crash', + async (_label, raw) => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + await expectCalendarRendered(); + expect(addButton()).toBeNull(); + } finally { + errors.mockRestore(); + } + }, + ); + + it('MUST-NOT-CHANGE: a host `onAddClick` function still works with no `allowCreate` at all', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + const hostAdd = vi.fn(); + try { + render( + , + ); + + await expectCalendarRendered(); + const button = addButton(); + expect(button).not.toBeNull(); + + fireEvent.click(button as HTMLElement); + expect(hostAdd).toHaveBeenCalledTimes(1); + } finally { + errors.mockRestore(); + } + }); + + it('a host `onAddClick` REPLACES the `allowCreate` dispatch, matching the `onEventClick` precedence', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + const hostAdd = vi.fn(); + const onAction = vi.fn(); + try { + render( + , + ); + + await expectCalendarRendered(); + fireEvent.click(addButton() as HTMLElement); + + // Same rule the sibling hatch already pins: a host handler REPLACES the + // `onAction` dispatch rather than running alongside it. + expect(hostAdd).toHaveBeenCalledTimes(1); + expect(onAction).not.toHaveBeenCalled(); + } finally { + errors.mockRestore(); + } + }); + + it('an authored `onAddClick` STRING still cannot reach the component (objectui#4453 stays closed)', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + await expectCalendarRendered(); + const button = addButton(); + expect(button).not.toBeNull(); + + // The renderer's own handler is what is wired, so the click is a normal + // no-op dispatch (no `onAction` host prop here) rather than a + // `onAddClick is not a function` throw. + expect(() => fireEvent.click(button as HTMLElement)).not.toThrow(); + expect(calendarRegion()).not.toBeNull(); + } finally { + errors.mockRestore(); + } + }); +}); + +describe('calendar-view: `colorMapping` is retired (objectui#4493)', () => { + it('the registry no longer declares a `colorMapping` input', () => { + const meta = ComponentRegistry.getMeta('calendar-view', 'plugin-calendar'); + const declared = (meta?.inputs ?? []).map((input) => input.name); + + // The removal itself. `colorMapping` was declared, documented in + // `content/docs/plugins/plugin-calendar.mdx` as + // `colorMapping?: Record< string, string >`, and read by nothing — an + // author who wrote it got no mapping, no warning and no error. + expect(declared).not.toContain('colorMapping'); + + // The inputs that stay, so this pin fails loudly if the removal ever takes + // a neighbour with it. + expect(declared).toEqual([ + 'data', + 'titleField', + 'startDateField', + 'endDateField', + 'allDayField', + 'colorField', + 'view', + 'currentDate', + 'allowCreate', + 'className', + ]); + }); + + it('an authored `colorMapping` changes nothing — it falls into the open tail and is dropped', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + render( + , + ); + + await expectCalendarRendered(); + // Still the real calendar with its computed event: retiring the + // declaration is not a behaviour change, because the key never had a read + // site to lose. It is now an ordinary unknown authored key, dropped at the + // renderer boundary like any other (objectui#4453). + expect(await screen.findByRole('button', { name: 'Computed Standup' })).toBeTruthy(); + } finally { + errors.mockRestore(); + } + }); +}); diff --git a/packages/plugin-calendar/src/calendar-view-renderer.propsContract.test.tsx b/packages/plugin-calendar/src/calendar-view-renderer.propsContract.test.tsx index a09c8ce72..f2739d56e 100644 --- a/packages/plugin-calendar/src/calendar-view-renderer.propsContract.test.tsx +++ b/packages/plugin-calendar/src/calendar-view-renderer.propsContract.test.tsx @@ -558,8 +558,19 @@ describe('calendar-view: the rest of the forward set is consumed or declared (ob zzcanary: 'CANARY-STR', zzcanaryobj: { nested: true }, reference_to: 'contacts', - allowCreate: true, + // Declared and registered here, and read by nobody, when this + // canary was written. `colorMapping`'s declaration has since been + // RETIRED (objectui#4493), which is why it belongs in the tail + // above rather than beside `allowCreate` below: it is now an + // ordinary unknown authored key. colorMapping: { meeting: 'blue' }, + // NOT part of the dropped tail any more. `allowCreate` is a + // CONSUMED declared input as of objectui#4454 — it supplies the + // `onAddClick` hatch, so this node also renders the header's + // "New event" button. Kept on the canary so the combination is + // exercised: turning the affordance on must not disturb anything + // this file pins. + allowCreate: true, props: { zzcanaryprop: 'CANARY-PROP' }, } as never } diff --git a/packages/plugin-calendar/src/calendar-view-renderer.tsx b/packages/plugin-calendar/src/calendar-view-renderer.tsx index 8f5236b65..265572f3b 100644 --- a/packages/plugin-calendar/src/calendar-view-renderer.tsx +++ b/packages/plugin-calendar/src/calendar-view-renderer.tsx @@ -190,6 +190,36 @@ function resolveAuthoredCurrentDate(raw: unknown): Date | undefined { return Number.isNaN(parsed.getTime()) ? undefined : parsed; } +/** + * Resolve the declared `allowCreate` input: does this node get the add + * affordance (objectui#4454)? + * + * The input was declared (`type: 'boolean'`, "Allow creating events by clicking + * on dates") and read by nobody. The handler it would gate — `handleAddClick` + * below, dispatching `{ type: 'create' }` on this widget's own `onAction` + * channel — was already BUILT and never passed, so `CalendarView`, which renders + * its "New event" button behind `{onAddClick && …}`, never saw a handler and the + * button never existed. Both halves of one feature were present and never + * introduced to each other: an authorable key with no read site, and a handler + * with no caller. ADR-0049's enforce-or-remove framing, answered ENFORCE here + * because the intent was already in the tree. + * + * The wiring goes THROUGH the declared `onAddClick` hatch rather than around it + * via a second prop: that key is already one of {@link HOST_CALLBACKS}, so a + * React host can supply the affordance today and that path is not disturbed. + * `allowCreate` supplies the hatch's value for the SDUI path, where an author + * writing JSON can never produce a function. + * + * Only the boolean `true` turns it on. Everything else — absent, `false`, and + * the off-type spellings JSON invites (`'true'`, `1`) — gets `undefined`, which + * is the absent-key answer every other resolver here gives and, on this prop, + * literally the thing that makes the button not render. That is what keeps + * today's behaviour byte-identical for every node that never authored the key. + */ +function resolveAuthoredAllowCreate(raw: unknown): boolean { + return raw === true; +} + // Calendar View Renderer - Airtable-style calendar for displaying records as events ComponentRegistry.register('calendar-view', ({ @@ -220,6 +250,10 @@ ComponentRegistry.register('calendar-view', // The declared `view` input: CONSUMED and narrowed to its declared enum // below (objectui#4453). view: authoredView, + // The declared `allowCreate` input: CONSUMED below and turned into the + // `onAddClick` hatch's value (objectui#4454). Both authoring channels land + // here, the node's own key and a `props: { allowCreate }` container. + allowCreate: authoredAllowCreate, // Everything else. READ for declared keys, NEVER spread: this is the raw // channel the old `{...props}` handed straight to `CalendarView`, and the // reason an authored string could land on a function-typed prop. @@ -276,6 +310,10 @@ ComponentRegistry.register('calendar-view', // The declared `view` input, narrowed to the enum it declares. const view = resolveAuthoredView(authoredView); + // The declared `allowCreate` input, narrowed to the boolean it declares + // (objectui#4454). + const allowCreate = resolveAuthoredAllowCreate(authoredAllowCreate); + // The declared host hatches, each kept only at its declared type. Read out // of `rest`; `rest` itself never reaches `CalendarView`. const hostCallbacks = pickHostCallbacks(rest); @@ -300,12 +338,16 @@ ComponentRegistry.register('calendar-view', }); }; + // Standard "Create" action trigger, gated by the declared `allowCreate` + // input below. It goes through `dispatchAction`, the narrowed `onAction`, + // exactly like `handleEventClick` — so an authored + // `onAction: 'NOT-A-FUNCTION'` cannot turn this newly-live affordance into + // objectui#4453's uncaught handler crash. const handleAddClick = () => { - // Standard "Create" action trigger - dispatchAction?.({ - type: 'create', - payload: {} - }); + dispatchAction?.({ + type: 'create', + payload: {}, + }); }; // The forward set is exactly `CalendarViewProps` — nothing else can reach @@ -334,6 +376,15 @@ ComponentRegistry.register('calendar-view', // string that used to land on this prop now lands nowhere, so the // fallback stands. onEventClick={hostCallbacks.onEventClick ?? handleEventClick} + // The declared `allowCreate` input, finally wired (objectui#4454). + // Same precedence rule as `onEventClick` directly above: a host's own + // handler REPLACES the `onAction` dispatch, and it keeps working with + // no `allowCreate` authored at all — that is the pre-existing hatch + // path this card must not disturb. `undefined` when the flag is not + // the boolean `true`, which is what makes `CalendarView` render no + // "New event" button — today's behaviour, unchanged, for every node + // that never authored the key. + onAddClick={hostCallbacks.onAddClick ?? (allowCreate ? handleAddClick : undefined)} /> ); } @@ -383,13 +434,18 @@ ComponentRegistry.register('calendar-view', defaultValue: 'color', description: 'Field name for event color' }, + // `colorMapping` used to be declared here and is RETIRED (objectui#4493, + // ADR-0049 enforce-or-remove). It had no read site anywhere — not in this + // renderer, whose event mapping takes the colour straight off the record + // (`color: record[colorField]`), and not in `CalendarView`, which resolves + // a colour from `event.color`. An author who wrote the documented + // `colorMapping: { meeting: 'blue' }` got no mapping, no warning and no + // error: the raw field value was used as the colour, which for a picklist + // value like `meeting` is not a colour at all. Removed rather than + // implemented because no measured app authors it — declared authorable + // surface with nothing behind it is the one state that must not persist, + // and a capability nobody pulls on is not worth building. { - name: 'colorMapping', - type: 'object', - label: 'Color Mapping', - description: 'Map field values to colors (e.g., {meeting: "blue", deadline: "red"})' - }, - { name: 'view', type: 'enum', enum: ['month', 'week', 'day'],