Add mouse-move discard prompt for keyboard tweaks#447
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/react-grab/src/components/edit-panel/index.tsx">
<violation number="1" location="packages/react-grab/src/components/edit-panel/index.tsx:307">
P2: `navigateActive` arms `shouldPromptOnPointerMove` whenever `hasPendingTweaks()` is true, but this includes pending edits created by pointer-only slider changes (not keyboard). If a user drags a slider, then navigates with Arrow keys, this will incorrectly trigger the discard prompt on subsequent mouse movement. Consider gating this on a flag that tracks whether the pending tweaks originated from keyboard stepping (e.g., check `isCompact()` or introduce a dedicated `hasKeyboardTweaks` signal).</violation>
<violation number="2" location="packages/react-grab/src/components/edit-panel/index.tsx:435">
P2: Pointer-triggered discard prompt is never disarmed after first open, so cancelling once causes the prompt to reopen on every subsequent mouse move.</violation>
<violation number="3" location="packages/react-grab/src/components/edit-panel/index.tsx:435">
P2: Race condition: browsers fire `pointermove` before `mousedown` during a click gesture. When the user clicks outside the panel after a keyboard tweak, this handler opens the discard prompt via `attemptDismiss("pointer")`, but the same click's `mousedown` event then triggers the outside-dismiss logic, potentially discarding the edits immediately without user confirmation. Consider disarming `shouldPromptOnPointerMove` once `attemptDismiss` is called here, or preventing the outside-click dismiss handler from running when a discard prompt is already visible.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/react-grab/src/components/edit-panel/index.tsx">
<violation number="1" location="packages/react-grab/src/components/edit-panel/index.tsx:307">
P2: `navigateActive` arms `shouldPromptOnPointerMove` whenever `hasPendingTweaks()` is true, but this includes pending edits created by pointer-only slider changes (not keyboard). If a user drags a slider, then navigates with Arrow keys, this will incorrectly trigger the discard prompt on subsequent mouse movement. Consider gating this on a flag that tracks whether the pending tweaks originated from keyboard stepping (e.g., check `isCompact()` or introduce a dedicated `hasKeyboardTweaks` signal).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/react-grab/src/components/selection-label/discard-prompt.tsx">
<violation number="1" location="packages/react-grab/src/components/selection-label/discard-prompt.tsx:73">
P2: The new Copy button is not keyboard-activatable with Enter because the prompt’s capture-phase key handler forces Enter to `onConfirm`, causing discard instead of copy.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Additional Suggestions:
- Enter key handling in discard prompt always calls onConfirm() regardless of which button has focus, breaking keyboard accessibility
- commitActive function doesn't pass source: "keyboard" to commit() calls, breaking the mouse-move discard prompt for keyboard-sourced edits in inline controls
- Enter key in discard prompt unconditionally calls props.onConfirm() without checking which button has focus, breaking keyboard accessibility
- Click on arrow-selected element bypasses discard prompt when no mouse movement occurs - the arrow navigation pointer handoff is not checked in handleSingleClick
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 640a121. Configure here.
There was a problem hiding this comment.
Additional Suggestions:
- Enter key handling unconditionally calls onConfirm() regardless of which button has focus, breaking keyboard accessibility when Copy or No buttons are focused
- pointerup event handler doesn't check isPendingDismiss(), allowing click actions to bypass the discard prompt when keyboard selection has a pending dismissal
- commitActive() function calls commit() without passing source option, preventing pointerMovePromptHandoff.arm() from being triggered when edits come from keyboard sources like ValueStepper text input or ColorPicker text input
- Enter key in discard prompt unconditionally calls onConfirm() regardless of which button has focus, breaking keyboard accessibility
- Arrow navigation menu selections don't arm the mouse-move discard prompt, unlike keyboard arrow navigation
There was a problem hiding this comment.
Additional Suggestions:
- handleSubmit() function submits edits but does not close the edit panel, leaving it open when it should close after copying
- The pointerup event handler does not check keyboardSelection.isPendingDismiss(), allowing click actions to bypass the discard prompt when keyboard selection has a pending dismissal
Show the edit panel discard prompt when mouse movement follows keyboard-driven pending tweaks, add a Copy action to the discard prompt, and prompt before discarding an arrow-key selection on mouse handoff via a keyboard-selection controller.
- Gate the selection label's Enter-to-expand on discardPrompt so Enter on the keyboard-selection prompt's Copy/Yes copies/discards instead of opening the Style panel (matches the existing canToggleExpand gate). - Remove the unreachable keyboard-selection cancel/re-arm path, the dead isPendingDismiss/onCancelDismiss renderer/label props plus the renderer fallback (migrate openstory stories to the discardPrompt union), an unreachable pointermove guard, commitActive's unreachable default source, and an unneeded signal in the pointer-move handoff.
210f611 to
8f56117
Compare
|
Triaged the latest Vercel VADE suggestions as false-positives (resolved, not fixed):
|
The two discard prompts differ little in practice, so collapse the StandardSelectionDiscardPrompt | KeyboardSelectionDiscardPrompt union into a single SelectionDiscardPrompt with an isKeyboardSelection flag. Simpler to construct and read; the label-rendering ternaries collapse accordingly.
Edit panel: mouse move with no pending tweaks stays silent, the prompt's Yes reverts the tweak, a fresh keyboard commit re-arms the consumed one-shot handoff, an autoApplied class arms it, and a touch pointermove neither opens the prompt nor consumes the arm. Keyboard selection: Enter on the focused Copy copies without opening the Style panel (guards the Enter-to-expand regression), Enter on Yes discards without copying, and arrow keys are ignored while the prompt is open.

Summary
Verification
pnpm dlx --package @antfu/ni nr buildpnpm testpnpm lintpnpm typecheckpnpm formatSummary by cubic
Adds a one-time discard prompt when the mouse moves after keyboard tweaks or arrow-key selection, with Copy to keep the edit or copy the keyboard-selected element. While shown, detection pauses, Expand is disabled, Enter confirms Copy/Yes, arrow keys are ignored, and clicks prefer the keyboard-selected element.
New Features
EditPanelCopyButton,data-react-grab-discard-button="copy", anddata-react-grab-copy-button.data-react-grab-discard-copy,data-react-grab-discard-yes(and-nofor the standard prompt).Refactors
createPointerMovePromptHandoffandcreateKeyboardSelectionController. ReplacedisPendingDismiss/onCancelDismisswith a singlediscardPrompt?: SelectionDiscardPromptthat uses anisKeyboardSelectionflag across renderer/label and updated stories.discardPromptso Enter triggers Copy/Yes, and threaded commit sources ("keyboard"|"pointer") through steppers/color picker to arm the handoff only for keyboard commits. Removed unreachable guards and dead props. Expanded e2e coverage for discard flows, including touch pointer moves and Enter on Copy/Yes.Written for commit ec80831. Summary will update on new commits.