-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: update email sending #1942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe InviteViewersModal component was restructured to implement a two-column layout with improved recipient handling and email preview logic. Key changes include derived recipient state management, enhanced form validation, updated Send button behavior with dynamic labeling, and reorganized UI layout separating input controls from email preview. The public API signature remains unchanged. Changes
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ee/features/dataroom-invitations/components/invite-viewers-modal.tsx (1)
280-447: Approve the layout restructure with an optional accessibility suggestion.The two-column layout with live email preview is a significant UX improvement. The responsive design properly hides the preview on small screens, and the character count indicator (line 326) provides helpful feedback.
Optional: Consider using label elements for better accessibility.
Lines 290, 313, and 331 use
<span>for field labels. While the current implementation works, using proper<label>elements would improve accessibility:For example, at line 313-314:
- <span className="text-sm font-medium text-muted-foreground"> - Custom message - </span> + <label htmlFor="custom-message" className="text-sm font-medium text-muted-foreground"> + Custom message + </label> <Textarea + id="custom-message" value={customMessage}Similar changes could be applied to the other form fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ee/features/dataroom-invitations/components/invite-viewers-modal.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ee/features/dataroom-invitations/components/invite-viewers-modal.tsx (2)
components/ui/dialog.tsx (3)
DialogContent(151-151)DialogHeader(152-152)DialogTitle(154-154)components/ui/select.tsx (5)
Select(151-151)SelectTrigger(154-154)SelectValue(153-153)SelectContent(155-155)SelectItem(157-157)
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (3)
ee/features/dataroom-invitations/components/invite-viewers-modal.tsx (3)
47-56: LGTM: Clean email parsing logic.The function correctly handles multiple input formats (comma-separated, newline-separated, or mixed) and deduplicates emails using a Set.
169-178: Good: Clear derived state for recipient handling.The logic correctly derives recipient state for UI display, with proper handling of edited vs. default recipients. The displayRecipients and remainingCount calculations provide good UX for showing recipient lists.
462-470: Good: Clear button state and dynamic labeling.The disabled logic properly prevents sending when no recipients are present or (for group invitations) when no link is selected. The dynamic button label provides clear feedback about how many invitations will be sent.
Note: This logic will work correctly once the critical issue in lines 197-199 is resolved.
| const parsedEmails = hasEditedRecipients | ||
| ? parseRecipientInput(recipientInput) | ||
| : []; | ||
| : defaultRecipients; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Inconsistent recipient logic between UI and send handler.
Lines 197-199 compute parsedEmails differently than currentRecipients (lines 169-172). The UI checks both hasEditedRecipients && recipientInput.length > 0, but the send handler only checks hasEditedRecipients.
Scenario:
- User edits recipients (sets
hasEditedRecipients = true) - User clears all input text
- UI shows default recipients with "Send N invitations" button enabled
- Clicking Send would invoke
parseRecipientInput("")→[], sending 0 invitations
Apply this diff to align the logic:
- const parsedEmails = hasEditedRecipients
+ const parsedEmails = hasEditedRecipients && recipientInput.length > 0
? parseRecipientInput(recipientInput)
: defaultRecipients;🤖 Prompt for AI Agents
In ee/features/dataroom-invitations/components/invite-viewers-modal.tsx around
lines 197-199 (and note currentRecipients logic at 169-172), update the
parsedEmails computation to match the UI check by requiring both
hasEditedRecipients and a non-empty recipientInput (e.g.
recipientInput.trim().length > 0) before calling parseRecipientInput; otherwise
fall back to defaultRecipients — this ensures clearing the input yields
defaultRecipients in both the UI and the send handler and prevents sending zero
invitations.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.