Wired up button toggle to CTA Card #1415
Conversation
ref https://linear.app/ghost/issue/PLG-318/implement-button-toggle - Wired up a function to toggle the visibility of the button on the CTA card. - Ensures the Button settings expand and collapse when toggling it. - Added tests
|
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/src/components/ui/cards/CtaCard.jsxOops! 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. WalkthroughThe pull request introduces changes to the Call To Action (CTA) card functionality in the Koenig-Lexical editor. The modifications focus on enhancing the interactivity of the CTA card by adding a method to toggle the button's visibility dynamically. The changes span across three files: a UI component, a node component, and an end-to-end test file. The updates include refactoring function parameters, implementing a new toggle function, and adding corresponding test cases to verify the new behavior. Changes
Sequence DiagramsequenceDiagram
participant User
participant CtaCard
participant CallToActionNode
User->>CtaCard: Click toggle button
CtaCard->>CallToActionNode: toggleShowButton()
CallToActionNode-->>CtaCard: Update button visibility
CtaCard-->>User: Reflect button visibility change
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 (2)
packages/koenig-lexical/test/e2e/cards/cta-card.test.js (1)
43-57: Fix typo in comment.The test is well-structured and comprehensive, but there's a typo in the comment.
- // determine if settings are open byy looking for cta-button-color, button-text & button-url + // determine if settings are open by looking for cta-button-color, button-text & button-urlpackages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx (1)
36-41: Add error handling to the toggle function.While the implementation works, it could benefit from proper error handling and validation.
const toggleShowButton = (event) => { editor.update(() => { const node = $getNodeByKey(nodeKey); + if (!node) { + console.error('Failed to find node with key:', nodeKey); + return; + } node.showButton = !node.showButton; }); };Also applies to: 67-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/koenig-lexical/src/components/ui/cards/CtaCard.jsx(1 hunks)packages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx(4 hunks)packages/koenig-lexical/test/e2e/cards/cta-card.test.js(2 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/koenig-lexical/test/e2e/cards/cta-card.test.js (2)
2-2: LGTM!The addition of the
expectimport is necessary for the new test assertions.
32-41: LGTM! Well-structured test case.The test effectively verifies the button toggle functionality, checking both show and hide states.
packages/koenig-lexical/src/nodes/CallToActionNodeComponent.jsx (1)
4-4: LGTM!The new imports are necessary for implementing the button toggle functionality.
Also applies to: 9-9
ref https://linear.app/ghost/issue/PLG-318/implement-button-toggle
Summary by CodeRabbit
Release Notes
New Features
Tests
Improvements