-
-
Notifications
You must be signed in to change notification settings - Fork 76
Updated CTA card icon in card menu #1423
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
Ref https://linear.app/ghost/issue/PLG-320/improve-cta-card-kg-menu-preview-meta - This card will replace the Email CTA card, so it can use the same icon - Changed default background color to grey and sponsor label to true - Removed hasBackground property as this isn't needed - Removed prefilled content in nested editor - Updated card spacing
WalkthroughThis pull request introduces modifications to the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 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 (
|
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: 0
🧹 Nitpick comments (1)
packages/koenig-lexical/src/components/ui/cards/CtaCard.jsx (1)
223-224
: Simplified padding logic based on color and sponsor label.The padding conditions have been streamlined to only depend on:
- Background color being 'none'
- Presence of sponsor label
However, the current implementation might benefit from a more maintainable approach.
Consider extracting the padding logic into a separate function for better maintainability:
+const getPaddingClasses = (color, hasSponsorLabel) => { + if (color !== 'none') return ''; + return hasSponsorLabel ? 'pb-3' : 'py-3'; +}; <div className={clsx( 'w-full rounded-lg border', CTA_COLORS[color], - { - 'py-3': color === 'none' && !hasSponsorLabel, - 'pb-3': color === 'none' && hasSponsorLabel - } + getPaddingClasses(color, hasSponsorLabel) )} data-cta-layout={layout}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/kg-default-nodes/lib/nodes/call-to-action/CallToActionNode.js
(2 hunks)packages/kg-default-nodes/test/nodes/call-to-action.test.js
(0 hunks)packages/koenig-lexical/src/components/ui/cards/CtaCard.jsx
(1 hunks)packages/koenig-lexical/src/nodes/CallToActionNode.jsx
(3 hunks)packages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx
(0 hunks)
💤 Files with no reviewable changes (2)
- packages/kg-default-nodes/test/nodes/call-to-action.test.js
- packages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx
🔇 Additional comments (4)
packages/kg-default-nodes/lib/nodes/call-to-action/CallToActionNode.js (2)
13-14
: Property default values updated to match new design.The changes align with the PR objectives by:
- Setting
hasSponsorLabel
default totrue
- Setting
backgroundColor
default to'grey'
34-44
: Improved property initialization with nullish coalescing.The switch from logical OR (
||
) to nullish coalescing (??
) is a good improvement as it:
- Only applies defaults for
null
/undefined
values- Preserves falsy values like empty strings and
false
packages/koenig-lexical/src/nodes/CallToActionNode.jsx (2)
1-1
: Icon updated to match new design.The changes align with the PR objectives by replacing
CalloutCardIcon
withEmailCtaCardIcon
consistently across:
- Import statement
- Static
kgMenu
configurationgetIcon
methodAlso applies to: 18-18, 33-33
47-47
: Wrapper style now adapts based on background color.The conditional logic improves the component's flexibility:
- Uses 'wide' style when no background color is set
- Uses 'regular' style when a background color is present
Ref https://linear.app/ghost/issue/PLG-320/improve-cta-card-kg-menu-preview-meta
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
hasBackground
property from CTA components.