upgrade DropdowmMenuCopyButton by closing dropdown conditionally#346
upgrade DropdowmMenuCopyButton by closing dropdown conditionally#346
Conversation
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughReplaced inline two-step confirm logic in NoteDropdown with a hook-based mouse-position tracking approach; added content/button refs and copy-initiation callback wiring. Introduced a new TwoStepDropdownMenuItem component and utilities (mouse-tracking hook and isMouseOverElement). DropdownMenuItemCopyButton now supports onCopyInitiated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CopyBtn as DropdownMenuItemCopyButton
participant Track as MouseTrackingHook
participant Dropdown as NoteDropdown
User->>CopyBtn: Click "Copy"
activate CopyBtn
CopyBtn->>Track: onCopyInitiated() -- enable tracking
activate Track
CopyBtn->>CopyBtn: Write URL to clipboard
CopyBtn->>CopyBtn: set success state, schedule reset
CopyBtn->>Dropdown: onCopyComplete()
deactivate CopyBtn
rect `#E8F3FF`
Note over Track,Dropdown: Track checks mousePositionRef
Track->>Dropdown: isMouseOverElement(buttonRef/contentRef)?
end
alt Mouse outside button/content
Dropdown->>Dropdown: Dispatch Escape -> close dropdown
else Mouse inside button/content
Dropdown->>Dropdown: Keep dropdown open
end
deactivate Track
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsx (1)
33-53: Optional: Simplify the ref pattern.The
delayInMsRefpattern is used butdelayInMsappears to be constant throughout the hook's lifecycle. You can simplify by using the parameter directly in the timeout.Apply this diff if you'd like to simplify:
function useBooleanStateWithAutoRevertToFalse({ delayInMs }: { delayInMs: number }) { const [state, setState] = useState(false); - const delayInMsRef = useRef(delayInMs); - delayInMsRef.current = delayInMs; useEffect(() => { if (!state) { return; } const timeoutHandle = setTimeout(() => { setState(false); - }, delayInMsRef.current); + }, delayInMs); return () => { clearTimeout(timeoutHandle); }; - }, [state]); + }, [state, delayInMs]); return [state, setState] as const; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/NoteManager/components/NoteDropdown.tsx(5 hunks)src/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsx(2 hunks)src/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
When reviewing Tailwind CSS classes, ensure they follow Tailwind 4.x conventions and suggest modern 4.x alternatives for deprecated patterns.
Files:
src/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsxsrc/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsxsrc/components/NoteManager/components/NoteDropdown.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-22T20:36:10.440Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 330
File: src/components/Outline/OutlineLink.tsx:17-21
Timestamp: 2025-10-22T20:36:10.440Z
Learning: Project targets React 19+; prefer passing refs as a regular prop in function components and avoid React.forwardRef in new code. File: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/NoteManager/components/NoteDropdown.tsx
📚 Learning: 2025-10-22T20:36:10.440Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 330
File: src/components/Outline/OutlineLink.tsx:17-21
Timestamp: 2025-10-22T20:36:10.440Z
Learning: This repo targets React 19+; prefer passing refs as a normal prop to function components and avoid React.forwardRef in new code. File context: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/NoteManager/components/NoteDropdown.tsx
🧬 Code graph analysis (1)
src/components/NoteManager/components/NoteDropdown.tsx (1)
src/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsx (1)
DropdownMenuItemCopyButton(6-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: visual-tests
src/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsx
Outdated
Show resolved
Hide resolved
Visual Regression Test Report ✅ PassedGithub run id: 19539241593 🔗 Artifacts: Download |
chmurson
force-pushed
the
upgrade-dropdownmenuitem-copybutton
branch
from
253aee8 to
b675944
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsx (1)
21-29: Consider adding accessibility announcements for state transitions.When the component transitions to confirmation state, users with screen readers may not be aware of the visual change from
childrentoconfirmationSlot. Consider adding anaria-live="polite"region oraria-describedbypointing to a descriptive element to announce the state change.Example approach:
return ( - <DropdownMenuItem + <DropdownMenuItem + aria-live="polite" + aria-label={isConfirmation ? "Confirm action" : undefined} onClick={handleOnClick} className={cn(isConfirmation ? "text-destructive hover:bg-destructive/20 hover:text-destructive" : "")} > {!isConfirmation && children} {isConfirmation && confirmationSlot} </DropdownMenuItem> );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/NoteManager/components/NoteDropdown.tsx(5 hunks)src/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsx(2 hunks)src/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/NoteManager/components/NoteDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
When reviewing Tailwind CSS classes, ensure they follow Tailwind 4.x conventions and suggest modern 4.x alternatives for deprecated patterns.
Files:
src/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsxsrc/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: visual-tests
🔇 Additional comments (3)
src/components/NoteManager/components/SimpleComponents/DropdownMenuItemCopyButton.tsx (1)
30-44: Async clipboard handling implemented correctly.The previous review concern about awaiting the clipboard operation has been properly addressed. The implementation now:
- Makes
handleClickasync- Awaits
navigator.clipboard.writeText()before setting success state- Properly handles errors with try-catch
Calling
onCopyInitiated()before the await (line 36) is appropriate given its name—it signals when the copy operation begins, whileonCopyComplete(line 22) signals when the entire flow finishes.src/components/NoteManager/components/SimpleComponents/TwoStepDropdownMenuItem.tsx (2)
11-19: Click handling logic is correct.The two-step flow is well-implemented:
- First click: prevents default behavior and enables confirmation state
- Second click: executes the actual action
The
preventDefault()andstopPropagation()on lines 13-14 correctly intercept the first click to show the confirmation UI without triggering the menu item's default behavior.
32-52: Hook implementation is solid.The
useBooleanStateWithAutoRevertToFalsehook correctly:
- Uses a ref for
delayInMsto avoid re-running the effect when only the delay changes- Sets up a timeout when state becomes
true- Cleans up the timeout on unmount or state change
- Returns a properly-typed tuple with
as constThe ref pattern (lines 34-35) is a good practice here since
delayInMsis hardcoded to 2000ms in the component. If this hook were extracted for reuse with dynamic delays, you might consider addingdelayInMsto the effect dependencies to restart the timeout when the delay changes—but for the current usage, this implementation is appropriate.
1 participant
Context
DropdownMenuItemCopyButton closes the dropdown after it is clicked and 2s delays passes.
What
The component - DropdownMenuItemCopyButton (actually not this component by its parent) - adds new condition before closing the dropdown which is - it checks whether mouse is over dropdown content or button that triggers the opening of the dropdown.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.