From d4f54256f0dd8c239e094282c6b86c8a1106c2ec Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 05:29:26 +0000 Subject: [PATCH] fix(plugin-kanban): run an authored onCardClick ONCE per card click ObjectKanban handed the host's function to useNavigationOverlay as its onRowClick and then called it again itself. handleClick gives onRowClick full priority -- it calls it and returns -- so for a host supplying onCardClick and no onRowClick, one card click ran that one function twice. Drops the wrapper's second call. Of the two it was the poorer: handleClick forwards onRowClick(record, event), carrying the modifier payload a host needs for Cmd/Ctrl/middle-click, while the deleted call passed the record alone. `onRowClick ?? onCardClick` is untouched, so precedence is unchanged -- what changed is that the loser of that expression no longer also fires, which is the one breaking case and is called out in the changeset. ObjectKanbanSchema.onCardClick therefore declares the second, optional parameter the surviving channel delivers. `event` is `any` rather than HandleClickModifiers: that interface lives in @object-ui/react, which depends on @object-ui/types and is named in no dependency field of it. BaseSchema's own onClick / onChange / onSubmit spell this the same way. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .../9341-kanban-card-click-fires-once.md | 56 +++++ packages/plugin-kanban/src/ObjectKanban.tsx | 30 ++- .../cardClickFiresOnce-9341.test.tsx | 224 ++++++++++++++++++ ...ndlerKeyDispositionsMeasured-7804.test.tsx | 61 +++-- .../kanban-handler-slots-7664.test.tsx | 19 +- packages/types/src/objectql.ts | 31 ++- 6 files changed, 398 insertions(+), 23 deletions(-) create mode 100644 .changeset/9341-kanban-card-click-fires-once.md create mode 100644 packages/plugin-kanban/src/__tests__/cardClickFiresOnce-9341.test.tsx diff --git a/.changeset/9341-kanban-card-click-fires-once.md b/.changeset/9341-kanban-card-click-fires-once.md new file mode 100644 index 0000000000..3e7289707c --- /dev/null +++ b/.changeset/9341-kanban-card-click-fires-once.md @@ -0,0 +1,56 @@ +--- +'@object-ui/types': minor +'@object-ui/plugin-kanban': minor +--- + +An `onCardClick` supplied to an `object-kanban` board runs **once** per card +click instead of twice, and the published declaration of the key grows the +second parameter the surviving call actually delivers (objectui#9341, maintainer +ruling on the card, 2026-09-13). + +**This carries a breaking behaviour change** — the `ObjectView` case below. It +ships `minor` because the repo has a single `fixed` group of 40 packages, so a +`major` on either package majors all forty; the scope is stated here rather than +encoded in the bump. + +## `@object-ui/plugin-kanban` — one click, one call + +`ObjectKanban` handed the host's function to `useNavigationOverlay` as its +`onRowClick` **and** called it again itself on the next line. `handleClick` +gives `onRowClick` full priority — it calls it and returns — so for a host that +supplied `onCardClick` and no `onRowClick`, one card click ran that one function +twice: a duplicate navigation, a duplicate analytics event or a double-open, +depending on what the handler did. The wrapper's second call is gone. + +Of the two calls the deleted one was the poorer. `handleClick` forwards +`onRowClick(record, event)`, so a host can implement Cmd/Ctrl/middle-click; the +wrapper's call passed the record only. `onRowClick ?? onCardClick` is untouched, +so which handler wins is exactly what it was. + +⚠️ **The breaking case, to check before upgrading.** A board embedded in an +`ObjectView` gets `onRowClick` from that parent, so the parent's handler already +won. What also happened was that the document's own `onCardClick` ran anyway — +once, through the wrapper. It now runs **zero** times there: the winner of +`onRowClick ?? onCardClick` answers the click outright. A host that relied on an +authored `onCardClick` firing alongside a parent `onRowClick` must move that +work into the parent's handler. Pinned as a reading, not left to be discovered, +in `packages/plugin-kanban/src/__tests__/cardClickFiresOnce-9341.test.tsx`. + +## `@object-ui/types` — `ObjectKanbanSchema.onCardClick` declares two parameters + +```ts +onCardClick?: (card: any, event?: any) => void; // was: (card: any) => void +``` + +The surviving channel delivers `(record, event)`, so the one-parameter +declaration described only the call that was deleted. Growing an optional second +parameter is source-compatible in both directions — an existing one-argument +handler still type-checks, and code that stores the member in a one-argument +slot still compiles — and that claim is handed to `tsc` rather than asserted, in +the same pin file. + +`event` is `any`, not `HandleClickModifiers`: that interface lives in +`@object-ui/react`, which depends on `@object-ui/types` and is named in no +dependency field of it. `BaseSchema`'s own `onClick` / `onChange` / `onSubmit` +already spell this exact situation the same way. What arrives at runtime is the +DOM click event `KanbanImpl` forwards, typed `React.MouseEvent` there. diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 6cd4f65a5f..575cdbe032 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -252,7 +252,18 @@ export interface ObjectKanbanComponentProps { /** Loading state propagated from a parent. Respected only when `data` is also provided. */ loading?: boolean; onRowClick?: (record: any) => void; - onCardClick?: (record: any) => void; + /** + * ⚠️ TWO parameters, and the second one is not decoration: this prop is the + * `onCardClick` arm of `externalClick` below, which is handed to + * `useNavigationOverlay` as its `onRowClick` and invoked as + * `onRowClick(record, event)` — the modifier payload a host needs for + * Cmd/Ctrl/middle-click. Spelled `any` because `packages/types` declares the + * published twin of this key and may not name `HandleClickModifiers` (it lives + * in `@object-ui/react`, which depends on `@object-ui/types`), and the two + * faces must not disagree. `KanbanImpl` types the same channel as + * `React.MouseEvent`, which is what actually arrives. + */ + onCardClick?: (record: any, event?: any) => void; } export const ObjectKanban: React.FC = ({ @@ -1329,9 +1340,24 @@ export const ObjectKanban: React.FC = ({ // objectui#8307 — the lane headers count rows that came back, so when // the fetch saturated its window they must say `77+`, not `77`. countsAreWindowed, + // ⛔ Calls `handleClick` and NOTHING ELSE. An authored `onCardClick` + // already travels this one line: it is the `onCardClick` arm of + // `externalClick` above, which is `handleClick`'s `onRowClick`, and + // that arm has FULL PRIORITY inside the hook — it is called and the + // hook returns. A second `onCardClick?.(card)` here therefore ran the + // SAME function again, twice per card click (objectui#9341, measured + // 2 by objectui#9338's pin before it was relaxed). + // + // Of the two calls the DELETED one was the poorer: `handleClick` + // forwards `onRowClick(record, event)`, so the host can implement + // Cmd/Ctrl/middle-click, while the second call passed the record + // only. Dropping it also leaves `onRowClick ?? onCardClick` untouched + // — a board inside an `ObjectView` still gives the parent's handler + // priority, and now gives it OUTRIGHT rather than also running the + // authored one. `ObjectGallery` has written exactly this shape, with + // no second call, all along. onCardClick: (card: any, event?: any) => { navigation.handleClick(card, event); - onCardClick?.(card); }, onCardMove: handleCardMove, }} diff --git a/packages/plugin-kanban/src/__tests__/cardClickFiresOnce-9341.test.tsx b/packages/plugin-kanban/src/__tests__/cardClickFiresOnce-9341.test.tsx new file mode 100644 index 0000000000..aa322e21e1 --- /dev/null +++ b/packages/plugin-kanban/src/__tests__/cardClickFiresOnce-9341.test.tsx @@ -0,0 +1,224 @@ +/** + * 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. + */ + +/** + * One card click runs an authored `onCardClick` EXACTLY ONCE, and the call it + * survives as carries the modifier event (objectui#9341, maintainer ruling on + * the card, 2026-09-13). + * + * ## The defect + * + * `ObjectKanban` handed the same function to `useNavigationOverlay` as its + * `onRowClick` and then called it again itself: + * + * const externalClick = onRowClick ?? onCardClick; + * const navigation = useNavigationOverlay({ …, onRowClick: externalClick }); + * … + * onCardClick: (card, event) => { + * navigation.handleClick(card, event); + * onCardClick?.(card); // ⛔ the second call + * } + * + * `handleClick` gives `onRowClick` FULL PRIORITY — it calls it and RETURNS — so + * for a host that supplies `onCardClick` and no `onRowClick`, that one function + * is both the value of `externalClick` and the one the next line calls. One + * click, two calls. Measured 2 by objectui#9338's pin, which relaxed its own + * control to "it RAN" so that this repair would not redden a file that is not + * about it; that control is tightened back to the exact count in the same diff + * as this file. + * + * ## The ruling, and why the two calls are NOT interchangeable + * + * RULING: drop the wrapper's second call. `externalClick` keeps its + * `onCardClick` arm. + * + * `handleClick` forwards `onRowClick(record, event)`, so a host can implement + * Cmd/Ctrl/middle-click; the wrapper's second call passed the record ONLY. The + * surviving channel is therefore the RICHER one, which is leg 2's subject — and + * the reason the published `ObjectKanbanSchema.onCardClick` declaration grows a + * second optional parameter in this same change. + * + * The same shape one surface over is already the ruling's spelling: + * `ObjectGallery` (`packages/plugin-list/src/ObjectGallery.tsx`) writes + * `onRowClick: props.onRowClick ?? props.onCardClick` into the identical hook + * and has NO second call. + * + * ## ⚠️ The consequence the card's text did not state — measured in leg 3 + * + * The ruling's rationale says precedence is left alone, so a board inside an + * `ObjectView` "behaves exactly as today". That is true of the PRECEDENCE + * EXPRESSION and FALSE of the authored handler's call count: when a parent + * supplies `onRowClick` AND the document carries `onCardClick`, the authored + * one ran ONCE before this change (the wrapper called it) and runs ZERO times + * after it — the parent's handler wins OUTRIGHT, which is what + * `onRowClick ?? onCardClick` has always said and what nothing enforced. Leg 3 + * pins that reading rather than leaving it to be discovered. + * + * ## Every control here can fire + * + * A count of 1 is only a reading if the instrument can report something else. + * Leg 1 carries the sibling `onCardMove` spy at 0 on the SAME render, so the 1 + * is about this key and not a recorder that counts every key once; leg 4 drives + * the same handler TWICE and reads 2, so the counter demonstrably moves. + * + * ⚠️ No leg here depends on a throw. React 19 REPORTS a handler error out of + * the dispatch rather than rethrowing it, so an `expect(...).toThrow()` leg on + * this path could never fire. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, waitFor } from '@testing-library/react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import type { ObjectKanbanSchema } from '@object-ui/types'; +import '../index'; + +/** Every props object the board implementation was rendered with, in order. */ +const recorded = vi.hoisted(() => ({ impl: [] as Array> })); + +// The lazy board chunk is a prop recorder: the question is how many times the +// handler the board was HANDED runs the authored one, not what the board draws. +vi.mock('../KanbanImpl', () => ({ + default: (props: Record) => { + recorded.impl.push(props); + return null; + }, +})); + +const STATIC_COLUMNS = [{ id: 'todo', title: 'To Do', cards: [{ id: '1', title: 'One' }] }]; + +/** + * Author a document on the PRODUCTION path — `SchemaRenderer` resolves the + * registry key and spreads every non-metadata schema key as a React prop — and + * return the props the board implementation was handed last. + */ +async function boardPropsFor(schemaKeys: Record) { + const before = recorded.impl.length; + const { unmount } = render( + + + , + ); + await waitFor(() => expect(recorded.impl.length).toBeGreaterThan(before)); + const received = recorded.impl[recorded.impl.length - 1]; + unmount(); + return received; +} + +/** The handler the board implementation was handed, driven as a card click. */ +type BoardClick = (card: unknown, event?: unknown) => void; + +describe('objectui#9341 — an authored `onCardClick` runs ONCE per card click', () => { + it('⭐ ONE click, ONE call — with the sibling `onCardMove` spy at 0 on the same render', async () => { + const onCardClick = vi.fn(); + const onCardMove = vi.fn(); + const card = { id: '1', title: 'One' }; + + // ONE document, ONE render. The sibling key is the firing control: a + // recorder that counted every authored handler once would show 1 here too, + // and `onCardMove` reaches nothing on this arm (objectui#7804), so a 0 + // beside the 1 is what makes the 1 a reading about THIS key. + const props = await boardPropsFor({ onCardClick, onCardMove }); + (props.onCardClick as BoardClick)(card); + + expect( + { cardClick: onCardClick.mock.calls.length, cardMove: onCardMove.mock.calls.length }, + 'the authored `onCardClick` must run exactly once per card click', + ).toEqual({ cardClick: 1, cardMove: 0 }); + }); + + it('⭐ the surviving call carries the MODIFIER EVENT — the whole reason it is the one kept', async () => { + const onCardClick = vi.fn(); + const card = { id: '1', title: 'One' }; + // What `KanbanImpl` forwards is the DOM click event + // (`onClick={(e) => onCardClick?.(card, e)}`); only the three modifier + // fields are ever read, so a structural stand-in measures the channel. + const event = { metaKey: true, ctrlKey: false, button: 0 }; + + const props = await boardPropsFor({ onCardClick }); + (props.onCardClick as BoardClick)(card, event); + + expect( + onCardClick.mock.calls.map((args) => ({ arity: args.length, record: args[0], event: args[1] })), + 'the kept channel is `handleClick`’s `onRowClick(record, event)` — the dropped one passed the record only', + ).toEqual([{ arity: 2, record: card, event }]); + }); + + it('⚠️ a parent `onRowClick` wins OUTRIGHT — the authored `onCardClick` no longer also runs', async () => { + const onRowClick = vi.fn(); + const onCardClick = vi.fn(); + const card = { id: '1', title: 'One' }; + + // Both reach `ObjectKanban` as React props on this path — `SchemaRenderer` + // spreads every non-metadata key and `ObjectKanbanRenderer` forwards its + // rest — which is the shape an `ObjectView`-embedded board has, with the + // parent supplying `onRowClick`. + const props = await boardPropsFor({ onRowClick, onCardClick }); + (props.onCardClick as BoardClick)(card); + + // ⛔ NOT a regression smuggled in: `externalClick = onRowClick ?? onCardClick` + // is untouched by this card and has always said the parent's handler wins. + // What changed is that the loser no longer ALSO fires. + expect( + { rowClick: onRowClick.mock.calls.length, cardClick: onCardClick.mock.calls.length }, + 'precedence is `onRowClick ?? onCardClick`; exactly one handler answers one click', + ).toEqual({ rowClick: 1, cardClick: 0 }); + }); + + it('CONTROL — the counter moves: two clicks read 2', async () => { + const onCardClick = vi.fn(); + const card = { id: '1', title: 'One' }; + + const props = await boardPropsFor({ onCardClick }); + (props.onCardClick as BoardClick)(card); + (props.onCardClick as BoardClick)(card); + + expect(onCardClick.mock.calls.length, 'a count leg whose count cannot move measures nothing').toBe(2); + }); +}); + +/* ── The published face, judged by `tsc -p tsconfig.test.json` ───────────── */ + +/** + * ⭐ The published-surface half of this card, MEASURED rather than asserted in + * prose. `ObjectKanbanSchema.onCardClick` grew a second optional parameter + * because the surviving channel delivers `(record, event)`; the prediction + * offered with the ruling was that such a widening is source-compatible. These + * four lines are that prediction handed to `tsc`, and the last one is the + * control that keeps the other three from being vacuously true. + * + * ⛔ `event` is `any`, not `HandleClickModifiers`: that interface lives in + * `@object-ui/react`, which depends on `@object-ui/types` and is named in no + * dependency field of it — `check:phantom-deps` rejects the import and it would + * close a cycle. `BaseSchema`'s own `onClick` / `onChange` / `onSubmit` + * already spell this exact situation the same way. + */ +type Equal = + (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; +type Expect = T; +type Assignable = [From] extends [To] ? true : false; +type CardClick = NonNullable; + +export type assertion9341PublishedSignatureIsSourceCompatible = [ + /** WIDENING direction: a host's existing one-argument handler still fits. */ + Expect void, CardClick>>, + /** The reverse: code that stores the member in a one-argument slot still compiles. */ + Expect void>>, + /** The arity the channel actually delivers is now declared. */ + Expect void>>, + /** CONTROL — `Assignable` can answer `false`: a REQUIRED second parameter does not fit. */ + Expect void, (card: any) => void>, false>>, +]; diff --git a/packages/plugin-kanban/src/__tests__/handlerKeyDispositionsMeasured-7804.test.tsx b/packages/plugin-kanban/src/__tests__/handlerKeyDispositionsMeasured-7804.test.tsx index 3ccca1db02..ee649c30e9 100644 --- a/packages/plugin-kanban/src/__tests__/handlerKeyDispositionsMeasured-7804.test.tsx +++ b/packages/plugin-kanban/src/__tests__/handlerKeyDispositionsMeasured-7804.test.tsx @@ -34,8 +34,11 @@ * untouched and arrives at the board implementation BY IDENTITY. * - `onCardClick` — RUNTIME SLOT. `ObjectKanban` replaces the schema key with * its own wrapper, but `SchemaRenderer` also spreads the authored key as a - * React PROP, `ObjectKanbanComponentProps` declares that prop, and the - * wrapper CALLS it. The authored function runs. + * React PROP, `ObjectKanbanComponentProps` declares that prop, and + * `ObjectKanban` forwards it into `useNavigationOverlay` as `onRowClick`, + * where `handleClick` gives it full priority and calls it. The authored + * function runs. (Until objectui#9341 the wrapper ALSO called it directly, + * which is why it ran twice — see the correction at the end of this block.) * - `onCardMove` — the reading is `'retired'` and it does NOT land here. * `ObjectKanban` replaces the schema key with `handleCardMove` and declares * NO `onCardMove` prop (its rest parameter is discarded), so neither @@ -101,9 +104,20 @@ * output), but the dead-read leg of suite 2 also failed — on its lit CONTROL, * not on its subject. `onCardMove` was already measured dead; `onCardClick` ran * TWICE rather than once. That count is a defect in `ObjectKanban`'s click - * wiring, not in this card's subject, so the control now asserts that the - * authored handler RAN and the count is reported on its own card instead of - * being pinned here. + * wiring, not in this card's subject, so the control asserted that the authored + * handler RAN and the count was reported on its own card instead of being + * pinned here. + * + * ⭐ THAT CARD LANDED (objectui#9341, maintainer ruling 2026-09-13): the + * wrapper's second, one-argument call is gone, the authored handler runs ONCE + * through `handleClick`, and the relaxed control here is TIGHTENED BACK to the + * exact count in that same diff — the handoff this relaxation was written for. + * Two readings in suite 2 and suite 3 moved with it, each for the same reason + * and each noted at the assertion: the surviving call carries the modifier + * event, so it is `(record, event)` rather than `(record)`, and the TypeScript + * face declares the second parameter. The exact-count reading and the modifier + * channel have their own file, `cardClickFiresOnce-9341.test.tsx`; what is + * here stays the LIT CONTROL for the dead-read leg beside it. */ import { describe, it, expect, vi } from 'vitest'; @@ -255,7 +269,14 @@ describe('suite 2 — the channels, per key, on the one surviving registration ( // spy can only have arrived through it. expect(props.onCardClick).not.toBe(onCardClick); (props.onCardClick as (c: unknown, e?: unknown) => void)(card); - expect(onCardClick).toHaveBeenCalledWith(card); + // ⭐ Reads the WHOLE call list, not one matching call (objectui#9341). The + // old spelling was `toHaveBeenCalledWith(card)`, which passed on the base + // tree because the SECOND, one-argument call matched it — the very call + // that card deleted. What survives is `handleClick`'s + // `onRowClick(record, event)`, so the authored handler is reached once and + // with the event slot present. This is the channel, still measured, now + // measured as it actually is. + expect(onCardClick.mock.calls).toEqual([[card, undefined]]); }); it('⭐ `onCardMove` reaches NOTHING — driven, not inferred, with `onCardClick` as the lit control', async () => { @@ -280,17 +301,20 @@ describe('suite 2 — the channels, per key, on the one surviving registration ( ); (props.onCardClick as (c: unknown, e?: unknown) => void)(card); - // ⚠️ The control asserts that `onCardClick` RAN, deliberately not how many - // times. Measured on the base tree it runs TWICE per click on this path: - // `ObjectKanban` passes the same function to `useNavigationOverlay` as - // `onRowClick` — whose `handleClick` forwards to it and returns — and then - // calls it again itself. That is a separate defect on a separate card; a - // count pinned here would make fixing it red on a file that is not about - // it, and the control needs only to be able to fire. + // ⭐ TIGHTENED BACK TO THE EXACT COUNT (objectui#9341, the card this control + // was relaxed FOR). It read `cardClickRan: … > 0` while the base tree ran + // the authored handler TWICE per click — `ObjectKanban` passed the same + // function to `useNavigationOverlay` as `onRowClick`, whose `handleClick` + // forwards to it and returns, and then called it again itself. That defect + // is fixed in the same diff that restores this count, so the relaxation has + // no subject any more; leaving it would let the double fire return in + // silence. The exact count owns its own file + // (`cardClickFiresOnce-9341.test.tsx`) — here it is still just the LIT + // CONTROL for the dead-read reading beside it. expect( - { cardMove: onCardMove.mock.calls.length, cardClickRan: onCardClick.mock.calls.length > 0 }, + { cardMove: onCardMove.mock.calls.length, cardClick: onCardClick.mock.calls.length }, 'the lit control `onCardClick` must run — a run where NEITHER fires measures nothing', - ).toEqual({ cardMove: 0, cardClickRan: true }); + ).toEqual({ cardMove: 0, cardClick: 1 }); }); it("the prop channel is the difference: `ObjectKanbanComponentProps` declares `onCardClick` and no `onCardMove`", () => { @@ -367,7 +391,12 @@ describe('suite 3 — the disposition is legible on BOTH faces, and they agree ( onCardMove: member('onCardMove'), onQuickAdd: member('onQuickAdd'), }).toEqual({ - onCardClick: '(card: any) => void', + // ⚠️ Two parameters since objectui#9341: the call that survives the + // double-fire repair is `handleClick`'s `onRowClick(record, event)`, and + // the one-argument spelling this pin used to read described only the + // deleted call. A signature pin read off disk, so it moves WITH the + // declaration and cannot drift from it. + onCardClick: '(card: any, event?: any) => void', // ⚠️ `undefined` is the reading, not a gap in the regex — the firing // control below reads a member this interface has always had. A // `?: never` tombstone here is what the measurement asks for and what diff --git a/packages/plugin-kanban/src/__tests__/kanban-handler-slots-7664.test.tsx b/packages/plugin-kanban/src/__tests__/kanban-handler-slots-7664.test.tsx index 4f19a40ca8..4d7b8d6763 100644 --- a/packages/plugin-kanban/src/__tests__/kanban-handler-slots-7664.test.tsx +++ b/packages/plugin-kanban/src/__tests__/kanban-handler-slots-7664.test.tsx @@ -85,10 +85,18 @@ * `createElement` call), and `ObjectKanbanComponentProps` DECLARES * `onCardClick` — there is no `onCardMove` prop. So on the `'object-kanban'` key an * authored `onCardClick` is not merely overridden: `ObjectKanban`'s own - * wrapper CALLS it. Suite 2 invokes the function the board was handed and + * wrapper RUNS it. Suite 2 invokes the function the board was handed and * measures that the authored one runs, with the identity check from suite 1 as * the control that the wrapper is genuinely interposed. * + * ⚠️ Since objectui#9341 the wrapper reaches it INDIRECTLY — it calls + * `useNavigationOverlay`'s `handleClick`, which gives the same function full + * priority as its `onRowClick` and calls it with `(record, event)`. The wrapper + * used to ALSO call it directly with the record alone, which ran one authored + * handler twice per click; that second call is gone. Nothing about the CHANNEL + * this file measures changed: the prop still reaches, and the authored function + * still runs. + * * ⇒ On every channel measured, `onCardClick` is at least as live as * `onCardMove`. Its #6124 disposition is RUNTIME SLOT, not `?: never`. * @@ -274,7 +282,14 @@ describe("ObjectKanban's own onCardClick wrapper CALLS the authored handler (obj // have arrived through it. expect(props.onCardClick).not.toBe(onCardClick); (props.onCardClick as (c: unknown, e?: unknown) => void)(card); - expect(onCardClick).toHaveBeenCalledWith(card); + // ⭐ objectui#9341 — this leg's SUBJECT is unchanged (the wrapper still + // runs the authored handler), but the call it runs it through moved. Two + // calls used to reach the spy; `toHaveBeenCalledWith(card)` matched the + // one-argument one, which is the call that card deleted as the poorer of + // the pair. The survivor is `useNavigationOverlay`'s + // `onRowClick(record, event)`, which carries the modifier payload. Read as + // the whole call list so the count is part of the reading. + expect(onCardClick.mock.calls).toEqual([[card, undefined]]); }); }); diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index 1cc1fbd6a6..17dcc2d75d 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -3291,7 +3291,16 @@ export interface ObjectKanbanSchema extends BaseSchema { * spelling. Kept callable here because the function REACHES the board and * RUNS: `SchemaRenderer` spreads every non-metadata schema key as a React * prop, `ObjectKanbanComponentProps` declares an `onCardClick` prop, and - * `ObjectKanban`'s own click wrapper calls it. + * `ObjectKanban` forwards that prop into `useNavigationOverlay` as its + * `onRowClick` — where `handleClick` gives it FULL PRIORITY and calls it. + * + * ⭐ CORRECTED, NOT QUIETLY EDITED (objectui#9341). This paragraph used to + * end "and `ObjectKanban`'s own click wrapper calls it", which was true and + * was ALSO the defect: the wrapper called the authored function a SECOND + * time, on top of the `handleClick` call above, so one card click ran it + * twice. The wrapper's call is gone; the CHANNEL and every word of the + * argument below survive, because the prop still reaches the hook and the + * hook still runs it. Only the identity of the call SITE moved. * * ⚠️ It is NOT the function the board implementation receives — `ObjectKanban` * substitutes its own wrapper on the schema it hands down, because that @@ -3303,8 +3312,24 @@ export interface ObjectKanbanSchema extends BaseSchema { * parameter — so an authored one reaches nothing, which is a `'retired'` * reading that `check:handler-key-reads` refuses while `KanbanRenderer` still * reads the key. It keeps its `KNOWN_UNDECLARED_READS` row on objectui#7804. - */ - onCardClick?: (card: any) => void; + * + * ⚠️ TWO PARAMETERS since objectui#9341, and the second is what the surviving + * channel actually delivers: `handleClick` forwards `onRowClick(record, + * event)` so a host can implement Cmd/Ctrl/middle-click. The call this + * declaration used to describe — one argument — is the one that was deleted; + * declaring one here would have described only the dropped call. + * + * ⛔ `event` is `any` rather than `HandleClickModifiers`, and that is a + * MEASURED constraint, not a shortcut: that interface lives in + * `@object-ui/react`, which depends on THIS package and which this package's + * manifest does not name in any dependency field — so naming it here is a + * phantom dependency (`check:phantom-deps`) and closes a cycle. Re-declaring its three fields inline would + * put a second copy of one contract on a published face. `event?: any` is the + * spelling `BaseSchema`'s own `onClick` / `onChange` / `onSubmit` already use + * for exactly this situation, one file over. What actually arrives is the DOM + * click event `KanbanImpl` forwards, typed `React.MouseEvent` there. + */ + onCardClick?: (card: any, event?: any) => void; /** * Quick Add handler.