Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 3. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
peterzimon
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUI and layout updates to the member email editor and welcome email modal. Editor styling changes: added bottom padding, input text sizing for Koenig, plus-button alignment, isolation for selected cards, and figcaption/nested-editor typography adjustments. Modal restructuring: replaced the flat header with a sticky top header, moved From/Reply-to/Subject into a header composition with Subject as a TextField (including validation), replaced fixed preview heights with clamp/flexible sizing, added dark background classes, and set scrolling={false} on the modal. No logic, prop signatures, or exported APIs changed. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsx`:
- Line 56: Update the incorrect Tailwind/selector string in the class list
inside member-email-editor (the selector
'[&_.koenig-lexical_input]:text-[1.4rem]') to match the real DOM class name;
replace it with '[&_.koenig-lexical-editor-input]:text-[1.4rem]' so the style
applies to the Koenig lexical editor input element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d8685e0-8399-4579-a538-cc880c11d3d7
📒 Files selected for processing (1)
apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsx
apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (2)
315-329: Consider addingscrolling={false}to the feature-flag-off Modal for consistency.The feature-flag-on Modal (line 325) has
scrolling={false}, but the feature-flag-off Modal (lines 225-234) does not. Both branches implement their own internal scrolling (line 235 and line 51), so having the Modal also scroll could cause double-scrollbar issues in the feature-flag-off branch.If the current behavior in the feature-flag-off branch is working as intended, this can be ignored.
💡 Suggested fix for consistency
<Modal afterClose={() => { updateRoute('memberemails'); }} dirty={isDirty} footer={false} header={false} + scrolling={false} testId='welcome-email-modal' width={672} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx` around lines 315 - 329, The feature-flag-off branch's Modal component (the same Modal used around the welcome-email modal where updateRoute and isDirty are passed) is missing the prop scrolling={false}, which can cause double-scrollbar issues because the inner content already handles scrolling; add scrolling={false} to that Modal instantiation so both branches match the feature-flag-on Modal's behavior and prevent nested scrolling problems.
51-53: Verify behavior on small viewports.The height clamp
h-[clamp(0px,calc(100dvh-320px),82vh)]could result in zero or near-zero height if the viewport height is ≤320px. While this is unlikely in a desktop admin panel context, consider if a minimum height (e.g.,200pxinstead of0px) would be safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx` around lines 51 - 53, The clamped height on the scrollable container in the WelcomeEmailModal component can collapse to 0px on very small viewports; update the class on the div (the element with "flex h-[clamp(0px,calc(100dvh-320px),82vh)] min-h-0 grow flex-col overflow-y-auto") to use a safer minimum (for example change the clamp's minimum from 0px to 200px or another sensible min-height) so the container never becomes effectively zero-height on small screens while preserving the max (82vh) and offset calculation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx`:
- Around line 315-329: The feature-flag-off branch's Modal component (the same
Modal used around the welcome-email modal where updateRoute and isDirty are
passed) is missing the prop scrolling={false}, which can cause double-scrollbar
issues because the inner content already handles scrolling; add
scrolling={false} to that Modal instantiation so both branches match the
feature-flag-on Modal's behavior and prevent nested scrolling problems.
- Around line 51-53: The clamped height on the scrollable container in the
WelcomeEmailModal component can collapse to 0px on very small viewports; update
the class on the div (the element with "flex
h-[clamp(0px,calc(100dvh-320px),82vh)] min-h-0 grow flex-col overflow-y-auto")
to use a safer minimum (for example change the clamp's minimum from 0px to 200px
or another sensible min-height) so the container never becomes effectively
zero-height on small screens while preserving the max (82vh) and offset
calculation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8bd1b54-8a6c-4a98-8da4-ce65840b890a
📒 Files selected for processing (1)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (1)
331-331: Prefer a shared dark-mode token instead of repeating#151719.Line 331 and Line 346 duplicate the same hardcoded dark color. Consider a shared utility/token class to keep theme updates centralized.
Also applies to: 346-346
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx` at line 331, Two identical hardcoded dark-mode colors ('dark:bg-[`#151719`]') appear in welcome-email-modal.tsx; replace both occurrences with a shared dark-mode token/utility (e.g., a Tailwind theme token or CSS variable) to centralize theme updates. Create or use an existing shared token like a Tailwind color key or CSS var (e.g., --color-dark-bg) and update the className entries in the WelcomeEmailModal component (search for the string 'dark:bg-[`#151719`]' in the file) to reference that token (e.g., dark:bg-<token> or dark:bg-[var(--color-dark-bg)]), and add the new token to the central theme/styles configuration so both instances use the same source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx`:
- Around line 79-80: The container class list in WelcomeEmailModal (the string
starting with 'flex mx-auto w-full rounded-b-lg bg-white... px-6') sets border
colors but omits a border width utility, so border-red-500 never appears; add a
base border utility (e.g., 'border') to that class string so lexical validation
classes like 'border-red-500' can render, ensuring the class list in
welcome-email-modal.tsx includes 'border' alongside the existing utilities.
---
Nitpick comments:
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx`:
- Line 331: Two identical hardcoded dark-mode colors ('dark:bg-[`#151719`]')
appear in welcome-email-modal.tsx; replace both occurrences with a shared
dark-mode token/utility (e.g., a Tailwind theme token or CSS variable) to
centralize theme updates. Create or use an existing shared token like a Tailwind
color key or CSS var (e.g., --color-dark-bg) and update the className entries in
the WelcomeEmailModal component (search for the string 'dark:bg-[`#151719`]' in
the file) to reference that token (e.g., dark:bg-<token> or
dark:bg-[var(--color-dark-bg)]), and add the new token to the central
theme/styles configuration so both instances use the same source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba906881-9f24-4112-9a42-b93593743f60
📒 Files selected for processing (1)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx (1)
79-80:⚠️ Potential issue | 🟡 MinorAdd a base border utility so lexical error border can render.
At Line 79,
EmailPreviewBodysets border colors but no border width utility. The error class applied at Line 393 (border-red-500) won’t visibly render without a baseborderclass.Suggested fix
- 'flex mx-auto w-full rounded-b-lg bg-white shadow-sm transition-[max-width,height,padding] duration-300 ease-out motion-reduce:transition-none dark:border-grey-900 dark:bg-grey-975 dark:shadow-none grow max-w-[780px] px-6', + 'flex mx-auto w-full rounded-b-lg border border-grey-200 bg-white shadow-sm transition-[max-width,height,padding] duration-300 ease-out motion-reduce:transition-none dark:border-grey-900 dark:bg-grey-975 dark:shadow-none grow max-w-[780px] px-6',#!/bin/bash # Verify border-width utility is missing where border color utilities are present rg -n -C3 "const EmailPreviewBody|rounded-b-lg|dark:border-grey-900|border-red-500" apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx` around lines 79 - 80, EmailPreviewBody sets border-color utilities but lacks a base border width, so the error class (border-red-500 applied later around line 393) will not be visible; update the class list in the EmailPreviewBody component (the array containing 'flex mx-auto w-full rounded-b-lg bg-white ... dark:border-grey-900 dark:bg-grey-975 ...') to include a base border utility (e.g., 'border' or 'border-2') so border-color classes like border-red-500 can render.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx`:
- Around line 79-80: EmailPreviewBody sets border-color utilities but lacks a
base border width, so the error class (border-red-500 applied later around line
393) will not be visible; update the class list in the EmailPreviewBody
component (the array containing 'flex mx-auto w-full rounded-b-lg bg-white ...
dark:border-grey-900 dark:bg-grey-975 ...') to include a base border utility
(e.g., 'border' or 'border-2') so border-color classes like border-red-500 can
render.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f26c0d90-8f81-485d-87fc-4bae311aa007
📒 Files selected for processing (2)
apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsxapps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/admin-x-settings/src/components/settings/membership/member-emails/member-email-editor.tsx
EvanHahn
left a comment
EvanHahn
left a comment
There was a problem hiding this comment.
LGTM other than one comment.
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
Outdated
Show resolved
Hide resolved
2 participants
ref https://linear.app/ghost/issue/NY-1128/welcome-emails-editor-visual-refinements
ref https://linear.app/ghost/issue/NY-1121/need-a-little-padding-at-the-bottom-of-the-editor-when-a-new-node-is
ref https://linear.app/ghost/issue/NY-1120/caption-styling-on-image-and-bookmark-is-bigger-than-it-should-be-non