Skip to content

fix(web): commit event duplicates immediately instead of opening the form - #2755

Merged
tyler-dane merged 2 commits into
mainfrom
claude/event-duplication-ux-21ecd2
Aug 11, 2026
Merged

fix(web): commit event duplicates immediately instead of opening the form#2755
tyler-dane merged 2 commits into
mainfrom
claude/event-duplication-ux-21ecd2

Conversation

@tyler-dane

Copy link
Copy Markdown
Contributor

Summary

Duplicating an event (Mod+D on a focused grid event, right-click > Duplicate, or the event form's action menu) previously created a grid draft with an identical time slot to the source and force-opened the edit form — even though every field was already filled in. Two problems followed: the draft rendered directly on top of the source with no visual distinction, and the form was an unwanted extra step.

Duplication now commits the copy immediately as a real saved event through the existing optimistic create mutation, skips the draft/form entirely, and focuses the new card so Shift+Arrow can reposition it right away. It's undoable via the existing Mod+Z stack. Saved events already participate in the app's overlap "deck" layout (grid/layout/timed-deck.layout.ts), so the duplicate and original become visually distinguishable as a side effect of no longer being drafts — no new layout code needed.

Falls back to the old draft/form path only when no writable calendar can be resolved for the copy (duplicating from a read-only calendar with no default target calendar set).

Simplicity

  • New logic lives in one shared helper (commitDuplicateEvent in events/mutations/duplicate-event.ts) called from both the grid keyboard shortcut and the form/context-menu hook, rather than duplicating the commit sequence.
  • Reused the existing duplicateGridEventDraft adapter, parseGridEventDraft, and the create mutation as-is — no new mutation or store.
  • Narrowed duplicateGridEventDraft's return type to Extract<GridEventDraft, { kind: "create" }> | null so the new helper doesn't need a redundant runtime kind guard just to satisfy the discriminated union.
  • A /simplify pass (4 parallel review angles: reuse, simplification, efficiency, altitude) found nothing else worth changing at this diff's scope; a couple of lower-value suggestions (extracting a 3-line calendarId-fallback pattern shared with useSaveEventForm, and lazily reading the default-target-calendar hook) were skipped as not worth the added abstraction/API surface for a negligible cost.

Automated validation

Manually verified in a live browser dev server (bun dev:web, anon/local session):

  • Focused-event Mod+D creates a real second event immediately, no form opens, the two events fan visually via the deck layout.
  • Focus lands on the new duplicate; Shift+Arrow nudges only the new card (verified schedule updates, e.g. 12–1 PM → 12:15–1:15 PM).
  • Two Mod+Z presses cleanly undo the nudge then the create, leaving only the original.
  • Re-tested the exact race condition a review round flagged: opening the form by clicking an event (title input focused), then Mod+D from inside the form — focus correctly lands on the new duplicate's card, not the source, and the form closes with no console errors.
  • No new console/network errors introduced (only pre-existing ERR_CONNECTION_REFUSED noise from the backend not running in this dev-only session).

Independent review

Two independent read-only review rounds via feature-dev:code-reviewer, plus a 4-angle /simplify pass:

  • Round 1 confirmed one real issue: an uncoordinated focus race between useCloseEventForm's refocus-to-source-event and the new commit's focus-the-new-card call, reachable via the form's own Duplicate action. Fixed by having useDuplicateEvent discard the closed form's draft directly (draftActions.discard()) instead of going through useCloseEventForm, since the commit path owns focus itself once it succeeds.
  • /simplify found and fixed one dead branch (a draft.kind !== "create" runtime check that could never be false) by narrowing the adapter's return type instead; three other findings (a small duplicated calendarId-fallback pattern, an unconditional-hook efficiency nit, and a broader useCloseEventForm API redesign suggestion) were judged not worth the added scope/risk and skipped with reasoning.
  • Round 2 (fresh reviewer, full final diff) specifically re-verified the type-narrowing change is sound at all call sites and that the draftActions.discard() swap drops no other side effect — no findings.

Test plan

  • bun run type-check — clean
  • bun run lint — clean (13 pre-existing warnings elsewhere, none in touched files)
  • TZ=UTC NODE_ENV=test bun test src/events/mutations/duplicate-event.test.ts src/views/Day/hooks/shortcuts/useDayEventNudgeShortcuts.test.tsx src/views/Forms/EventForm/EventForm.test.tsx src/components/ContextMenu/ContextMenuItems.test.tsx src/events/grid-event-draft.adapter.test.ts (run from packages/web) — 107 pass, 0 fail

…form

Duplicating an event previously created an identical-time draft stacked
invisibly on the source and force-opened the edit form, even though every
field was already filled in. Duplication now commits straight through the
existing create mutation and focuses the new card, so it's immediately
visible (saved events participate in the deck overlap layout) and
repositionable with Shift+Arrow; Mod+Z still undoes the whole thing.
…ly behavior

CI caught a second existing test (useWeekShortcutOwner.test.tsx) asserting
the old open-the-form behavior; the day-view equivalent was updated in the
prior commit but this one was missed.
@tyler-dane
tyler-dane merged commit 55930e5 into main Aug 11, 2026
20 checks passed
@tyler-dane
tyler-dane deleted the claude/event-duplication-ux-21ecd2 branch August 11, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant