fix: make clipboard work along with wrong position of popup#1493
fix: make clipboard work along with wrong position of popup#1493retrogtx wants to merge 5 commits intoMail-0:stagingfrom
Conversation
WalkthroughThis update refactors clipboard copy functionality in mail display components by introducing a centralized hook with improved error handling and fallback support. It also simplifies popover positioning and styling, removes obsolete code related to unsubscribe and attachments, and updates internal logic for copying sender emails and selected text. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailDisplay
participant useCopyToClipboard
participant Toast
User->>MailDisplay: Click "Copy" (email or text selection)
MailDisplay->>useCopyToClipboard: copyToClipboard(value, [id])
alt Clipboard API available
useCopyToClipboard->>Clipboard API: writeText(value)
Clipboard API-->>useCopyToClipboard: Success/Failure
else Fallback
useCopyToClipboard->>DOM: Create hidden textarea, execCommand('copy')
DOM-->>useCopyToClipboard: Success/Failure
end
alt Success
useCopyToClipboard->>Toast: Show success
else Failure
useCopyToClipboard->>Toast: Show error
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/mail/hooks/use-copy-to-clipboard.ts (1)
31-31: Improve code readability with optional chaining.The static analysis tool correctly identifies that this can be simplified using optional chaining.
Apply this diff to use optional chaining:
- if (navigator.clipboard && navigator.clipboard.writeText) { + if (navigator.clipboard?.writeText) {apps/mail/components/mail/mail-display.tsx (2)
90-92: Simplified positioning logic improves clarity.The changes to use center coordinates and fixed positioning are good improvements for popover placement. However, the extremely high z-index value (2147483647) suggests there might be CSS specificity conflicts that could be resolved more elegantly.
Consider investigating why such a high z-index is needed and whether the underlying CSS conflicts can be resolved at a lower level.
Also applies to: 133-139
148-151: Consider alternative approach to invisible trigger button.The invisible button (h-0 w-0) appears to be a workaround for popover positioning. While functional, this approach is semantically unclear and could be confusing for future maintainers.
Consider whether the popover component API supports programmatic positioning without requiring a trigger element, or if a more semantic approach is possible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/mail/mail-display.tsx(6 hunks)apps/mail/hooks/use-copy-to-clipboard.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/mail/components/mail/mail-display.tsx (1)
apps/mail/hooks/use-copy-to-clipboard.ts (1)
useCopyToClipboard(4-54)
🪛 Biome (1.9.4)
apps/mail/hooks/use-copy-to-clipboard.ts
[error] 31-31: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
apps/mail/hooks/use-copy-to-clipboard.ts (2)
7-24: Excellent fallback implementation for clipboard copying.The fallback mechanism is well-implemented with proper DOM manipulation, styling to hide the textarea, and thorough cleanup. Good error handling and boolean return for success indication.
26-51: Well-designed clipboard hook with robust error handling.The implementation properly handles the modern clipboard API with graceful fallback, provides clear user feedback through toasts, and manages state effectively. The configurable reset delay is a nice touch for customization.
apps/mail/components/mail/mail-display.tsx (2)
58-59: Good integration of the new clipboard hook.Appropriate imports for the enhanced clipboard functionality and popover components.
184-188: Excellent integration of the centralized clipboard functionality.The copy button properly uses the new clipboard hook and manages the selection state correctly. This is a good example of how to integrate the centralized clipboard functionality.
|
this also has a toast btw, not in the video cause of the arrow zoom |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Text Selection Popover Fails to Hide
The text selection popover remains visible after text is deselected or when no text is selected. This occurs because the handleSelectionChange function no longer clears the selectionCoords and selectedText state when window.getSelection() is collapsed or empty. The removal of the setSelectionCoords(null) and setSelectedText('') calls prevents the popover from hiding, leading to a stale UI.
apps/mail/components/mail/mail-display.tsx#L81-L85
Zero/apps/mail/components/mail/mail-display.tsx
Lines 81 to 85 in 2667bff
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
2 participants
clipboard.mp4
Summary by CodeRabbit
New Features
Refactor