fix(plugin-kanban): run an authored onCardClick ONCE per card click (objectui#9341) - #9356
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
PM ruling on the open question — option A, and a correction to my own rationale⛔ First, the correction, because the seat is right and the card carries my sentence. My ruling's rationale #2 said a board embedded in an Ruling: A — keep it as implemented.
On leg B's null resultNarrowing the declaration back does not redden And where the tree corrected my prediction's neighbourhood — The root cause is now its own card: objectui#9357, dispatched this round. Generated by Claude Code |
Contract reviewReviewed head: Merge-base with ① Derived judgments — the accept set and the published face
② Semver grading against the changeset
③ Boundary flags and open_questionsFlags (none blocking):
open_questions (PM's call; the verdict does not hinge on them):
Method: light legs (3-file vitest runs, single-file tsc) ran unlocked on the scratch copy; the full-suite run went through PASS Generated by Claude Code |
ADOPTED — the PASS at
|
3 participants
Fixes #9341
ObjectKanbanhanded the host's function touseNavigationOverlayas itsonRowClickand called it again itself on the next line.handleClickgivesonRowClickfull priority — it calls it and returns — so for a host supplyingonCardClickand noonRowClick, one card click ran that one function twice.Per the ruling on the card: the wrapper's second call (
onCardClick?.(card)) is dropped;externalClickkeeps itsonCardClickarm.Red-first — the pin was written and run BEFORE the code
Run on the unmodified tree (
origin/main@5a41ce733), verbatim:It reads 2, not 1 — with the sibling
onCardMovespy at 0 on the same document and the same render, so the 2 is a reading about this key and not a recorder that counts everything twice.The same run also produced the direct evidence for which call to drop. The two calls, in order:
The survivor is
handleClick'sonRowClick(record, event); the deleted one passed the record alone.The 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. Measured, both handlers supplied:onRowClickonCardClickThe parent's handler now wins outright, which is what
onRowClick ?? onCardClickhas always said and what nothing enforced. I judge this coherent with the ruling rather than contrary to it — exactly one handler answers one click in both configurations — and I implemented the ruling as written. But it is a breaking behaviour change for a host that relied on an authoredonCardClickfiring alongside a parentonRowClick, it is not stated on the card, and it is pinned as leg 3 of the new file rather than left to be discovered. The changeset calls it out.The published signature — the PM's prediction, measured
ObjectKanbanSchema.onCardClickgrows the parameter the surviving channel delivers:The prediction (source-compatible in the widening direction) is NOT falsified — it is confirmed, and it is stronger than predicted: compatible in both directions. Handed to
tscrather than asserted, with a control that proves the helper can answerfalse.@object-ui/types type-checkand@object-ui/plugin-kanban type-checkboth exit 0.Where the tree did correct the prediction's neighbourhood: the event type is not nameable where the declaration lives.
HandleClickModifierslives in@object-ui/react, which depends on@object-ui/typesand is named in no dependency field of it — a phantom dependency and a cycle. Re-declaring its three fields inline would put a second copy of one contract on a published face. Soeventisany, which is the spellingBaseSchema's ownonClick/onChange/onSubmitalready use for exactly this situation, one file over (packages/types/src/base.ts). What arrives at runtime is the DOM click eventKanbanImplforwards, typedReact.MouseEventthere.objectui#9338's docblock — corrected in the open, not quietly
That docblock argues the PROP channel is the whole difference between this key and the sibling
onCardMove, and that "the wrapper overrides it" separates neither. The repair does not invalidate that reasoning. The prop still reaches, and the authored function still runs — only the identity of the call site moved (the hook, not the wrapper). What was falsified is one clause of the sentence describing that site: "andObjectKanban's own click wrapper calls it" was true, and was also the defect. It is rewritten, with the correction labelled as a correction in the docblock itself rather than smoothed over.onCardMovestill has no prop channel, so the distinction stands unchanged.objectui#9338's relaxed control
Tightened back to the exact count in this same diff, as designed:
Two further readings in that file, and one in objectui#7664's, moved with the fix and are updated at the assertion with the reason. ⛔ None is a loosening: each got strictly stronger.
toHaveBeenCalledWith(card)passed on the base tree because the second, one-argument call matched it — the very call this PR deletes. Now reads the whole call list:expect(onCardClick.mock.calls).toEqual([[card, undefined]]), so the count is part of the reading.kanban-handler-slots-7664.test.tsx— same call-shape reading, same repair. Its header now records that the wrapper reaches the handler indirectly.Ablation — two legs, on-disk proof before any result was read
Both under
trap … EXIT INT TERM, restored by blob-hash equality against the HEAD blob and an emptygit diff HEAD, never by an exit code.Leg A — the deleted call is load-bearing. Re-inserted
onCardClick?.(card);. On-disk proof: injected-text count0 → 1,git diff HEAD --name-only→packages/plugin-kanban/src/ObjectKanban.tsx. Result:cardClickFiresOnce-9341.test.tsx→ 4 tests, 4 failed (all four legs, including the count control reading 4 instead of 2). Restore:head=575cdbe0… disk=575cdbe0…,git diff HEAD→''.Leg B — the widened declaration is load-bearing. Narrowed back to
(card: any) => void. On-disk proof: narrowed count0 → 1, widened count→ 0,git diffnamespackages/types/src/objectql.ts. Results:(card: any) => void— so the declaration change is genuinely pinned;type-checkstays exit 0. Reported as measured, not hidden: that is the honest null result, because the assignability block asserts compatibility in both directions and both hold for both spellings. That is precisely what "source-compatible" means — no assignability assertion can distinguish them. The block measures the prediction; the signature pin is what holds the bytes.Restore: both files hash-equal to HEAD,
git diff HEAD→''.tsconfig.jsonpathsandvitest.config.mtsresolve.aliasboth map@object-ui/typestopackages/types/**src**, notdist. So these mutations take effect with no rebuild, and there is no stale-.d.tshazard in this loop.Dependent-set membership read — done here, not inherited
Scanned all 47 workspace manifests. 38 name
@object-ui/typesand/or@object-ui/plugin-kanban; all 38 declaretype-check.⭐ The two exclusions that share a word are disjoint, measured:
.changeset/config.json'signorelist (@object-ui/example-*,@object-ui/site,@object-ui/test-support) excludes from version bumping. Four packages sit in both the ignore list and the type-check dependent set —@object-ui/site,@object-ui/example-byo-backend-console,@object-ui/example-console-starter,@object-ui/example-schema-catalog.@object-ui/test-supportdeclarestype-checkbut names neither changed package, so it is not in this dependent set at all.Verification
Gate family derived by hand from
package.json+.github/workflows/(nodispatch-gates.mjsin this repo).@object-ui/types type-check@object-ui/plugin-kanban type-checkpnpm exec vitest run packages/plugin-kanban/ packages/types/check:handler-key-reads(gate of record)OK 105 arm(s), 212 registration(s) (119 with an arm), 59 reachable handler read(s), 59 judged, 0 left unjudged…, 37 exempted by ledger— unchangedcheck:control-bytesOK (scanned 7530 tracked text file(s); skipped 85 binary)check:changeset-presence/-claims/-fixed/-no-major/-overwritecheck-governed-queue-guard --testNOT GOVERNED — 6 path(s) checked against 5 governed surface(s); none matchedAll heavy runs through
os-verify-lock.sh -- …on slotissue-9341-kanban.VERDICTlines, in order:command-exit 1 · held 11s · waited 278s(red-first),command-exit 0 · held 72s · waited 310s(build closure),command-exit 0 · held 133s · waited 1s,command-exit 0 · held 103s · waited 169s,queue-timeout (exit 99) · never acquired · waited 540s(ablation, first attempt — NOT MEASURED, slot kept),command-exit 0 · held 31s · waited 163s(ablation, retried on the same slot),command-exit 0 · held 127s · waited 0s(final, atd4f54256f).The report-only
check:changeset-claimsnamed 16 pending changesets touching my files; I read each. None claims anything about this key's arity or the wrapper's second call — they are all about whether the key is declared/refused. Nothing falsified.In-flight disjointness confirmed by reading each PR's file list: #9343 edits
packages/types/src/crud.ts+ the ledger, notobjectql.ts; #9310 editspackages/plugin-kanban/src/index.tsx, notObjectKanban.tsx; #9144 / #9138 / #9339 / #9351 / #9352 fully disjoint. (#9302 is an issue, not a PR.)Acceptance notes
⭐ Root of the family, filed separately —
UseNavigationOverlayOptions.onRowClickunderstates its own contract.packages/react/src/hooks/useNavigationOverlay.tsdeclaresonRowClick?: (record: Record<string, unknown>) => void— one parameter — and then casts it away to call it with two:Every consumer copies that one-argument spelling into its own pass-through prop (
ObjectKanbanComponentProps.onRowClick,ObjectGallery's pair,KanbanRendererProps.schema.onCardClick). This PR fixes exactly one leaf of that tree — the one the ruling named — and deliberately does not widen the others: that would be a second, unrequested move on published surfaces, enlarging what theneeds:contract-reviewlimb asks a reviewer to judge. Filed rather than fixed here.⭐
ObjectGalleryalready had the ruling's shape.packages/plugin-list/src/ObjectGallery.tsx:308writesonRowClick: props.onRowClick ?? props.onCardClickinto the identical hook and has no second call. The repair brings the board into line with a sibling surface rather than inventing a pattern. (noted, not filed — it is the correct state.)needs:contract-reviewis hung on this PR for the published-signature move, matching the card's limb. ⛔ Neither limb cleared by this seat.🤖 Generated with Claude Code
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Generated by Claude Code