Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update refactors form handling in the rule editor to use controlled components, adds contextual tooltips and updates labels in action summary cards, adjusts combobox button responsiveness, modifies onboarding text, updates action display strings, and increments the application version. No exported function signatures are changed except for an added parameter in an internal component. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RuleForm
participant FormContext
participant ActionSummaryCard
participant TooltipExplanation
User->>RuleForm: Interacts with form fields
RuleForm->>FormContext: Updates form state (controlled components)
RuleForm->>ActionSummaryCard: Renders action summary
ActionSummaryCard->>TooltipExplanation: (If applicable) Shows tooltip for action type
TooltipExplanation-->>User: Displays contextual explanation
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📜 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: 3
🧹 Nitpick comments (7)
apps/web/utils/action-display.ts (1)
24-24: Consistency between helper and UI strings
getActionDisplay()now returns “Auto-update reply label”, which matches the updated summary card, 👍.
If you expose the same action name in email notifications or logs, consider centralising the string in a constants file to avoid future drift.apps/web/components/Combobox.tsx (1)
44-45: Popover width now mismatches trigger width on ≥640 px screensThe trigger button lost
sm:w-[500px], but the pop-over still hassm:w-[500px].
At thesmbreakpoint the menu can overflow either side of the narrower button and look detached.Options:
- <PopoverContent className="w-full p-0 sm:w-[500px]"> + <PopoverContent className="w-full p-0">or keep the fixed width on both elements.
Verify on 640-768 px viewports before shipping.
apps/web/app/(app)/[emailAccountId]/assistant/ActionSummaryCard.tsx (2)
68-83: Inline<div>inside summary block may break text wrapping
summaryContentis injected into a<div className="whitespace-pre-wrap">…</div>.
Placing a flex<div>inside that container stopswhitespace-pre-wrapfrom affecting the inner text, so long subject lines will no longer wrap as before.Consider swapping the inner
<div>for a<span className="inline-flex gap-2 items-center">…</span>to preserve wrapping semantics.
172-179: Tooltip text contains single quotes that are not HTML-escapedThe template string embeds
'${NEEDS_REPLY_LABEL_NAME}', which in English will usually be"'Needs Reply'". Rendering inside the tooltip is safe, but if at any point this text is placed into a plain HTMLtitleattribute, unescaped quotes could truncate the string.Low risk today, but worth using
"or back-ticks in wording.apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx (1)
26-33: Minor copy style tweakCurrent sentence reads smoothly; if you want an even crisper CTA:
“Want more control? Update your rules anytime on the Assistant page.”
Pure style – feel free to ignore.
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx (2)
793-805: Consider deduplicating error renderingYou already render a global error list (lines 323-335) and per-section alerts (e.g. here for actions). This can produce duplicate messages. Centralising the error display or scoping it strictly to each section will reduce noise.
808-835:key={i}on mapped actions can cause wrong field pairing after re-orderUsing the array index as React key is fragile when elements are added/removed; field values may leap between cards. Prefer the stable
field.idyou get fromuseFieldArray(oraction.idif present).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/app/(app)/[emailAccountId]/assistant/ActionSummaryCard.tsx(5 hunks)apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx(11 hunks)apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx(1 hunks)apps/web/components/Combobox.tsx(1 hunks)apps/web/utils/action-display.ts(2 hunks)version.txt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/app/(app)/[emailAccountId]/assistant/onboarding/completed/page.tsx (1)
apps/web/components/Typography.tsx (1)
TypographyP(116-116)
apps/web/app/(app)/[emailAccountId]/assistant/RuleForm.tsx (12)
apps/web/components/Input.tsx (2)
Input(34-112)Label(116-132)apps/web/components/Typography.tsx (2)
TypographyH3(112-112)SectionDescription(114-114)apps/web/utils/actions/group.ts (1)
createGroupAction(13-35)apps/web/components/ui/card.tsx (1)
CardBasic(143-143)apps/web/components/Select.tsx (1)
Select(16-48)apps/web/utils/condition.ts (1)
getEmptyCondition(93-122)apps/web/utils/types.ts (1)
isDefined(8-10)apps/web/utils/path.ts (1)
prefixPath(1-4)apps/web/app/(app)/[emailAccountId]/assistant/ConditionSummaryCard.tsx (1)
ConditionSummaryCard(8-102)apps/web/app/(app)/[emailAccountId]/assistant/group/LearnedPatterns.tsx (1)
LearnedPatterns(12-42)apps/web/app/(app)/[emailAccountId]/assistant/ActionSummaryCard.tsx (1)
ActionSummaryCard(15-206)apps/web/utils/actions/rule.ts (1)
deleteRuleAction(365-423)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/web/utils/action-display.ts (1)
13-15: Check for functional parity between “Skip Inbox” and the underlying ARCHIVE actionRenaming the user-facing string is fine, but double-check every UI, test, and analytics query that depended on the literal word “Archive”. In particular, ensure:
- Existing e2e tests that look for “Archive” are updated.
- User docs / tooltips reflect the new wording.
- No translations or i18n keys still reference “Archive”.
A quick search across the codebase for
"Archive"vsActionType.ARCHIVEwill surface any stragglers.apps/web/app/(app)/[emailAccountId]/assistant/ActionSummaryCard.tsx (1)
202-203: Conditional tooltip rendering looks goodNice touch deferring tooltip creation until needed – avoids unnecessary DOM nodes.
version.txt (1)
1-1: Version bump acknowledgedv1.5.4 recorded. Ensure CHANGELOG is updated in the same commit or just after merge.
1 participant
Summary by CodeRabbit
New Features
UI Improvements
Content Updates
Chores