Styled visibility settings for HTML card #1426
Conversation
|
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/TabView.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. packages/koenig-lexical/src/components/ui/VisibilitySettings.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 subtle modifications to two UI components in the Koenig Lexical package. The Changes
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (3)
packages/koenig-lexical/src/components/ui/VisibilitySettings.jsx (1)
91-91: LGTM! Consider adding aria-label to hr elements.The changes improve the visual hierarchy and separation between visibility setting groups. The use of fragments and conditional horizontal rules enhances readability.
Consider adding an aria-label to the hr elements for better screen reader context:
- <hr className="not-kg-prose my-2 block border-t-grey-300 dark:border-t-grey-900" /> + <hr className="not-kg-prose my-2 block border-t-grey-300 dark:border-t-grey-900" aria-label="Visibility settings section divider" />Also applies to: 105-111
packages/koenig-lexical/src/components/ui/TabView.jsx (2)
13-13: Consider extracting spacing values to CSS custom properties.The conditional spacing logic (
px-6vsmx-6) could be more maintainable using CSS custom properties. This would centralize spacing values and make future adjustments easier.-<div className={`no-scrollbar flex gap-5 border-b border-grey-300 dark:border-grey-900 ${tabs.length > 1 ? 'w-full px-6' : 'mx-6'}`}> +<div className={`no-scrollbar flex gap-5 border-b border-grey-300 dark:border-grey-900 ${tabs.length > 1 ? 'w-full px-[--tab-spacing]' : 'mx-[--tab-spacing]'}`}>Add to your CSS:
:root { --tab-spacing: 1.5rem; /* equivalent to Tailwind's spacing-6 */ }
17-18: Enhance accessibility for single-tab scenarios.When there's only one tab, consider removing the tab semantics since it's not actually interactive. This would improve screen reader experience.
<button key={tab.id} className={`-mb-px appearance-none whitespace-nowrap pb-3 pt-4 text-sm font-semibold transition-all ${ tabs.length > 1 ? 'cursor-pointer border-b-2' : 'cursor-default' } ${ activeTab === tab.id ? 'border-black text-black dark:border-white dark:text-white' : 'border-transparent text-grey-600 hover:border-grey-500 dark:text-white' }`} data-testid={`tab-${tab.id}`} + role={tabs.length === 1 ? 'presentation' : 'tab'} type="button" - onClick={() => handleTabChange(tab.id)} + onClick={() => tabs.length > 1 && handleTabChange(tab.id)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/koenig-lexical/src/components/ui/TabView.jsx(1 hunks)packages/koenig-lexical/src/components/ui/VisibilitySettings.jsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Node 20.11.1
🔇 Additional comments (2)
packages/koenig-lexical/src/components/ui/VisibilitySettings.jsx (1)
106-106: LGTM! Semantic HTML improvement.Good choice changing from
divtopfor the group label. This improves semantic structure while maintaining visual styling through utility classes.packages/koenig-lexical/src/components/ui/TabView.jsx (1)
Line range hint
1-55: Overall implementation looks solid!The component handles both single and multi-tab scenarios gracefully while maintaining dark mode support and responsive design. The conditional styling provides appropriate visual feedback based on the interaction state.
1 participant
Ref https://linear.app/ghost/issue/PLG-327/enable-segmented-visibility-for-web