-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Feature] Highlight the selected options under Rich inputs. #2415
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
[Feature] Highlight the selected options under Rich inputs. #2415
Conversation
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/ui/src/components/rich-input.tsx(17 hunks)
🔇 Additional comments (4)
libs/ui/src/components/rich-input.tsx (4)
251-251: LGTM!The heading buttons (H1, H2, H3) correctly apply
bg-secondarystyling when their respective heading levels are active, properly including the{ level: N }parameter in the active state check.Also applies to: 264-264, 277-277
289-289: LGTM!The paragraph button correctly applies
bg-secondarystyling when paragraph formatting is active.
302-302: LGTM!The text alignment buttons (Left, Center, Right, Justify) correctly apply
bg-secondarystyling when their respective alignment states are active, properly using the{ textAlign: "..." }object parameter in the active state check.Also applies to: 315-315, 328-328, 341-341
354-354: LGTM!The list buttons (Bullet List, Numbered List) correctly apply
bg-secondarystyling when their respective list formats are active.Also applies to: 367-367
- when selecting the rich input option earlier only aria-pressed="true" was changing and not datastate now updated the code to manually update the data-state
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui/src/components/rich-input.tsx (1)
284-294: Add disabled prop to Paragraph toggle
The Paragraph toggle lacks adisabledcheck. Mirror other toolbar buttons by adding
disabled={!editor.can().chain().focus().setParagraph().run()}
to the<Toggle>in libs/ui/src/components/rich-input.tsx (lines 284–294).
🧹 Nitpick comments (1)
libs/ui/src/components/rich-input.tsx (1)
154-154: The manualdata-stateattributes are redundant.Radix UI's Toggle component automatically sets
data-stateto"on"or"off"based on thepressedprop you're already providing. Manually addingdata-stateduplicates this built-in behavior without adding value, and could cause confusion if the two ever drift out of sync during maintenance.Remove the
data-stateattributes from all Toggle components and rely on the component's built-in behavior:<Toggle size="sm" type="button" pressed={editor.isActive("bold")} disabled={!editor.can().chain().toggleBold().run()} - data-state={editor.isActive("bold") ? "on" : "off"} onPressedChange={() => editor.chain().focus().toggleBold().run()} >Apply similar changes to all 17 Toggle components (lines 154, 167, 180, 193, 206, 225, 238, 251, 264, 277, 289, 302, 315, 328, 341, 354, 367).
Also applies to: 167-167, 180-180, 193-193, 206-206, 225-225, 238-238, 251-251, 264-264, 277-277, 289-289, 302-302, 315-315, 328-328, 341-341, 354-354, 367-367
This PR adds the requested feature #2388 .
When user selects any of the rich inputs (Bold, italic etc. ) visual feedback is shown for the selected item.
Screenshot of working feature for reference.

Summary by CodeRabbit