Added layout toggle to CTA Card #1422
Conversation
This reverts commit 596db58.
ref https://linear.app/ghost/issue/PLG-315/added-layout-handling-on-cta-card - Changed the default layout to minimal from the previous default, which was immersive
ref https://linear.app/ghost/issue/PLG-315/added-layout-handling-on-cta-card - Added layout toggle to CTA Card
|
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/kg-default-nodes/lib/nodes/call-to-action/CallToActionNode.jsOops! Something went wrong! :( ESLint: 8.57.0 Error: Failed to load parser '@babel/eslint-parser' declared in 'packages/kg-default-nodes/.eslintrc.js': Cannot find module '@babel/eslint-parser'
WalkthroughThe pull request introduces modifications to the Call To Action (CTA) node and card components across multiple packages. The primary change is updating the default layout of the CTA from 'immersive' to 'minimal'. This involves adjusting the node class, test cases, and related components to reflect the new default layout. The changes enhance the flexibility of the CTA card by adding new handler functions for layout and background color updates, and introducing comprehensive e2e tests to validate the layout toggling functionality. Changes
Sequence DiagramsequenceDiagram
participant User
participant CtaCard
participant CallToActionNode
User->>CtaCard: Select/Change Layout
CtaCard->>CallToActionNode: handleUpdatingLayout()
CallToActionNode-->>CtaCard: Update Layout Property
CtaCard->>CtaCard: Update UI Based on Layout
Possibly related PRs
Poem
✨ 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/test/nodes/call-to-action.test.js (1)
75-77: Consider adding more layout transition test casesWhile the basic layout setter test is good, consider adding test cases for:
- Invalid layout values
- Layout transitions with other property changes
📜 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(2 hunks)packages/koenig-lexical/src/components/ui/cards/CtaCard.jsx(2 hunks)packages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx(2 hunks)packages/koenig-lexical/test/e2e/cards/cta-card.test.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/koenig-lexical/src/components/ui/cards/CtaCard.jsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Node 20.11.1
🔇 Additional comments (3)
packages/kg-default-nodes/lib/nodes/call-to-action/CallToActionNode.js (1)
6-6: Breaking Change: Default layout changed from 'immersive' to 'minimal'This change modifies the default behavior of the CTA card. Existing code that relies on the default 'immersive' layout may need to be updated.
Let's verify the impact on existing CTA cards:
Also applies to: 36-36
✅ Verification successful
✓ Default layout change is safe and verified
The change from 'immersive' to 'minimal' default layout is properly tested and doesn't impact existing code:
- Test suite explicitly verifies 'minimal' as the default layout
- No instances found of CTA cards being created without explicit layout specification
- Change is consistently applied across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CTA card usage without explicit layout specification ast-grep --pattern 'new CallToActionNode({$$$})'Length of output: 50
Script:
#!/bin/bash # Search for any references to CallToActionNode rg "CallToActionNode" -B 2 -A 2 # Also search for test files specifically fd "test|spec" -e js -x cat {} \; | rg "CallToActionNode" -B 2 -A 2Length of output: 15653
packages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx (1)
73-85: LGTM! Clean implementation of handler functionsThe new handlers follow consistent patterns and properly integrate with Lexical's editor update cycle.
packages/koenig-lexical/test/e2e/cards/cta-card.test.js (1)
209-241: LGTM! Comprehensive e2e test coverage for layout functionalityThe new tests thoroughly verify:
- Default layout behavior
- Layout toggle functionality
- Associated UI changes (text alignment)
ref https://linear.app/ghost/issue/PLG-315/added-layout-handling-on-cta-card
Summary by CodeRabbit
New Features
Tests