Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes update the mail components to improve mail selection handling. In the mail list component, the rendering is updated to conditionally display either a clickable link or a simple div based on a demo flag while preserving key attributes and events. In the mail layout component, a new state variable and callback function are introduced to track and update the selected mail item, and a hook is added to set a default selection when none exists. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailListDemo
participant DemoMailLayout
participant ThreadDemo
User->>MailListDemo: Clicks a mail item (in demo mode: rendered as div)
MailListDemo->>DemoMailLayout: Invokes onSelectMail callback
DemoMailLayout->>DemoMailLayout: Executes handleSelectMail to update selectedMail state
DemoMailLayout->>ThreadDemo: Passes updated mail prop to ThreadDemo
Note right of ThreadDemo: Renders content based on selected mail
Possibly related PRs
Suggested reviewers
Poem
✨ 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:
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: 0
🧹 Nitpick comments (5)
apps/mail/components/mail/mail.tsx (2)
77-77: Added state management for selected mailThis new state variable tracks the currently selected mail item in the demo layout. Consider using a more specific type instead of
anyto improve type safety and provide better developer experience.- const [selectedMail, setSelectedMail] = useState<any>(null); + const [selectedMail, setSelectedMail] = useState<typeof items[0] | null>(null);
109-113: Added auto-selection logic for initial mail displayThis useEffect properly ensures there's always a selected mail when items are available. However, consider adding a condition to prevent overriding user selections.
useEffect(() => { - if (filteredItems.length > 0 && !selectedMail) { + if (filteredItems.length > 0 && !selectedMail && !mail.selected) { handleSelectMail(filteredItems[0]); } }, [filteredItems, selectedMail, handleSelectMail]);apps/mail/components/mail/mail-list.tsx (3)
138-295: Implemented conditional rendering for mail items in demo modeThe implementation correctly handles both demo and non-demo modes, but there's significant code duplication between the two rendering paths. This could lead to maintenance issues if one path is updated but the other is forgotten.
Consider extracting the common elements into a shared component or using a higher-order component pattern to avoid duplication:
return ( <div className="p-1" onClick={onClick ? onClick(message) : undefined}> - {demo ? ( - <div - data-thread-id={message.threadId ?? message.id} - onMouseEnter={handleMouseEnter} - onMouseLeave={handleMouseLeave} - key={message.threadId ?? message.id} - className={cn( - 'hover:bg-offsetLight hover:bg-primary/5 group relative flex cursor-pointer flex-col items-start overflow-clip rounded-lg border border-transparent px-4 py-3 text-left text-sm transition-all hover:opacity-100', - isMailSelected || (!message.unread && 'opacity-50'), - (isMailSelected || isMailBulkSelected || isKeyboardFocused) && - 'border-border bg-primary/5 opacity-100', - isKeyboardFocused && 'ring-primary/50 ring-2', - )} - > - {/* Duplicated content */} - </div> - ) : ( - <Link - href={`/mail/${folder}?threadId=${message.threadId ?? message.id}`} - data-thread-id={message.threadId ?? message.id} - onMouseEnter={handleMouseEnter} - onMouseLeave={handleMouseLeave} - key={message.threadId ?? message.id} - className={cn( - 'hover:bg-offsetLight hover:bg-primary/5 group relative flex cursor-pointer flex-col items-start overflow-clip rounded-lg border border-transparent px-4 py-3 text-left text-sm transition-all hover:opacity-100', - isMailSelected || (!message.unread && 'opacity-50'), - (isMailSelected || isMailBulkSelected || isKeyboardFocused) && - 'border-border bg-primary/5 opacity-100', - isKeyboardFocused && 'ring-primary/50 ring-2', - )} - > - {/* Duplicated content */} - </Link> - )} + {(() => { + const commonProps = { + "data-thread-id": message.threadId ?? message.id, + onMouseEnter: handleMouseEnter, + onMouseLeave: handleMouseLeave, + key: message.threadId ?? message.id, + className: cn( + 'hover:bg-offsetLight hover:bg-primary/5 group relative flex cursor-pointer flex-col items-start overflow-clip rounded-lg border border-transparent px-4 py-3 text-left text-sm transition-all hover:opacity-100', + isMailSelected || (!message.unread && 'opacity-50'), + (isMailSelected || isMailBulkSelected || isKeyboardFocused) && + 'border-border bg-primary/5 opacity-100', + isKeyboardFocused && 'ring-primary/50 ring-2', + ) + }; + + const Content = () => ( + <> + <div + className={cn( + 'bg-primary absolute inset-y-0 left-0 w-1 -translate-x-2 transition-transform ease-out', + isMailBulkSelected && 'translate-x-0', + )} + /> + {/* Rest of the shared content */} + </> + ); + + return demo ? ( + <div {...commonProps}> + <Content /> + </div> + ) : ( + <Link href={`/mail/${folder}?threadId=${message.threadId ?? message.id}`} {...commonProps}> + <Content /> + </Link> + ); + })()} </div> );
303-306: Updated MailListDemo function signature with onSelectMail callbackThe function signature now properly accepts an optional callback for handling mail selection. However, consider improving type safety by using a more specific type than
any.-export function MailListDemo({ items: filteredItems = items, onSelectMail }: { - items?: typeof items; - onSelectMail?: (message: any) => void; -}) { +export function MailListDemo({ items: filteredItems = items, onSelectMail }: { + items?: typeof items; + onSelectMail?: (message: typeof items[0]) => void; +}) {
313-319: Added click handling to Thread componentThe implementation correctly passes the click handler to the Thread component for demo mode. However, the callback usage could be improved using optional chaining as suggested by static analysis.
- onClick={(message) => () => onSelectMail && onSelectMail(message)} + onClick={(message) => () => onSelectMail?.(message)}🧰 Tools
🪛 Biome (1.9.4)
[error] 318-318: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/mail/mail-list.tsx(1 hunks)apps/mail/components/mail/mail.tsx(5 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/mail/components/mail/mail.tsx (2)
apps/mail/components/mail/mail-list.tsx (1)
MailListDemo(303-326)apps/mail/components/mail/thread-display.tsx (1)
ThreadDemo(36-157)
apps/mail/components/mail/mail-list.tsx (3)
apps/mail/lib/utils.ts (2)
getEmailLogo(355-358)formatDate(66-107)apps/mail/lib/email-utils.client.tsx (1)
highlightText(59-77)apps/mail/lib/notes-utils.ts (1)
formatDate(72-89)
🪛 Biome (1.9.4)
apps/mail/components/mail/mail-list.tsx
[error] 318-318: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
apps/mail/components/mail/mail.tsx (3)
87-90: New mail selection handler functionGood implementation of a mail selection handler that centralizes the logic for updating both the selected mail state and the mail ID in the mail state object.
176-179: Updated MailListDemo to handle mail selectionCorrectly passes the selection handler to the mail list component, enabling proper communication between components.
194-194: Updated ThreadDemo to display selected mailGood fallback mechanism that ensures the thread always displays either the selected mail or defaults to the first available mail item.
2 participants
lazy route, gotta take a look into it later #541
Summary by CodeRabbit
New Features
Refactor