Added setting for link color to CTA card #1477
Conversation
Ref https://linear.app/ghost/issue/PLG-377/add-link-color-setting - Switched from two to three tabs in the settings panel, to make the settings easier to digest. - Added link color setting to the CTA card settings panel that allows switching between the default text color and the site accent color.
WalkthroughThe changes introduce a new configurable property, Possibly related PRs
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
packages/koenig-lexical/test/e2e/cards/call-to-action-card.test.jsOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the config "react-app" to extend from. Please check that the name of the config is correct. The config "react-app" was referenced from the config file in "/packages/koenig-lexical/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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 (
|
![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 (1)
packages/kg-default-nodes/lib/nodes/call-to-action/calltoaction-renderer.js (1)
215-216: Consider adding validation for the linkColor propertyWhile the
linkColorproperty has been correctly added to the dataset object, there's no validation for it unlikebuttonColorandbackgroundColorwhich have validation checks. Consider adding validation to ensure only valid values ('text' or 'accent') are used.// Add validation for backgroundColor if (!dataset.backgroundColor || !dataset.backgroundColor.match(/^[a-zA-Z\d-]+|#([a-fA-F\d]{3}|[a-fA-F\d]{6})$/)) { dataset.backgroundColor = 'white'; } + +// Add validation for linkColor +if (!dataset.linkColor || !['text', 'accent'].includes(dataset.linkColor)) { + dataset.linkColor = 'text'; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/kg-default-nodes/lib/nodes/call-to-action/calltoaction-renderer.js(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Node 20.11.1
🔇 Additional comments (2)
packages/kg-default-nodes/lib/nodes/call-to-action/calltoaction-renderer.js (2)
26-26: Good implementation of conditional class for link color stylingThe addition of the
kg-cta-link-accentclass whenlinkColoris set to 'accent' follows the existing pattern of conditional classes in the template. This allows for proper styling of links within the CTA card based on user preference.
180-180: Consistent implementation across template variantsThe same conditional class has been correctly added to the email template, ensuring consistent styling behavior across both web and email renderings of the CTA card. This is a good practice for maintaining visual consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/koenig-lexical/src/styles/components/koenig-lexical.css (1)
6-8: New Rule for CTA Link Color StylingThe rule for
.koenig-lexical .cta-link-colorappliescolor: var(--cta-link-color) !important;to ensure that CTA link colors are enforced. While the use of!importantguarantees priority, please confirm that this approach does not conflict with other styling rules in the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/koenig-lexical/src/components/ui/ButtonGroupBeta.jsx(2 hunks)packages/koenig-lexical/src/components/ui/ColorOptionButtonsBeta.jsx(1 hunks)packages/koenig-lexical/src/components/ui/ColorPickerBeta.jsx(3 hunks)packages/koenig-lexical/src/components/ui/TabView.jsx(2 hunks)packages/koenig-lexical/src/components/ui/cards/CallToActionCard.jsx(10 hunks)packages/koenig-lexical/src/styles/components/koenig-lexical.css(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/koenig-lexical/src/components/ui/TabView.jsx
- packages/koenig-lexical/src/components/ui/cards/CallToActionCard.jsx
🔇 Additional comments (10)
packages/koenig-lexical/src/components/ui/ColorOptionButtonsBeta.jsx (2)
34-34: Consistent dark mode styling for color selection indicatorThis change adds proper dark mode styling for the color selection indicator by using
dark:border-grey-950class, ensuring the border is visible against the dark background.
40-40: Consistent dark mode background for popoverUsing
dark:bg-grey-900for the color options popover ensures consistent styling with other popover components in dark mode. This change improves visual consistency across the UI.packages/koenig-lexical/src/components/ui/ButtonGroupBeta.jsx (2)
10-10: Improved dark mode styling for button group containerThe addition of
dark:bg-grey-900class ensures the button group container has appropriate background styling in dark mode, consistent with the project's dark theme implementation.
39-39: Enhanced active button styling in dark modeThe updated class string properly styles active buttons in dark mode by removing
dark:hover:bg-grey-900and addingdark:border-grey-800 dark:bg-grey-950. This creates better visual distinction for active states in dark mode.packages/koenig-lexical/src/components/ui/ColorPickerBeta.jsx (3)
205-206: Consistent color indicator border in dark modeAdding
dark:border-grey-950ensures the color indicator has appropriate contrast against dark backgrounds, matching the styling used in other color selection components.
221-222: Standardized dark mode background for color picker popoverChanging to
dark:bg-grey-900for the popover background standardizes the dark mode styling across all popovers in the application, improving UI consistency.
274-275: Consistent dark mode styling for custom color indicatorAdding
dark:border-grey-950to this span element ensures consistent styling with the other color indicator elements in dark mode.packages/koenig-lexical/src/styles/components/koenig-lexical.css (3)
3-4: CSS Variable Addition for Light Mode CTA Link ColorThe addition of
--cta-link-color-text: var(--grey-900);on lines 3–4 sets the default text color for CTA links in light mode. This aligns well with the new configurable link color setting introduced in the PR.
46-47: Dark Mode Variable Update for CTA Link ColorSetting
--cta-link-color-text: var(--grey-200);on lines 46–47 correctly adjusts the CTA link text color for dark mode. This update is consistent with maintaining good contrast in dark mode themes.
127-132: CTA Card Underline for LinksThe new rule for
.koenig-lexical-cta-label a(lines 127–132) adds an underline to links within CTA cards, enhancing visual distinction. This change supports the PR’s objective of offering customizable link color settings and styling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/koenig-lexical/test/e2e/cards/call-to-action-card.test.js (2)
319-335: Consider verifying additional image attributes
The test reliably checks that the preview image appears. For more thorough coverage, you could also confirm attributes likealttext or verify dimensions.
366-390: Robust background color tests
Looping through color configurations ensures broad validation. Consider a parameterized test if your test framework supports it for easier maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/koenig-lexical/test/e2e/cards/call-to-action-card.test.js(3 hunks)
🔇 Additional comments (10)
packages/koenig-lexical/test/e2e/cards/call-to-action-card.test.js (10)
122-128: Tests confirm default visibility of button settings
These assertions comprehensively validate that button-related elements are visible by default and properly initialized.
348-348: Design tab click aligns with updated UI
Navigating to the newly introduced design tab is consistent with the PR’s objective of expanding settings options.
357-357: Tab usage is consistent
Switching to the design tab again ensures consistent UI interaction. No issues found.
392-406: Effective popup dismissal check
Verifying the color picker closes on outside click adds valuable coverage for user interactions.
408-429: Comprehensive link color checks
Confirming default and accent color changes viagetComputedStyleis an effective approach. Great coverage for the newlinkColorfeature.
431-435: Verifies accent color as default
Ensuring the button defaults to the site’s accent color matches the intended design.
437-444: Testing black button color
Shows that the button color can be toggled away from accent. Implementation looks correct.
446-453: Verifies grey button color
This test ensures additional styling flexibility. No issues detected.
455-461: Color picker usage is validated
Good approach verifying custom color changes. The helper utility keeps the test concise.
463-475: Button text color updates alongside background
Checkingstyleattributes effectively confirms dynamic text color changes. This is thorough coverage of the styling logic.
Ref https://linear.app/ghost/issue/PLG-377/add-link-color-setting