refactor: standardize disabled state and cursor: default #728
Open
paanSinghCoder wants to merge 2 commits intomainfrom
Open
refactor: standardize disabled state and cursor: default #728paanSinghCoder wants to merge 2 commits intomainfrom
paanSinghCoder wants to merge 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis pull request standardizes the disabled state styling across the Raystack UI component library. The changes include: (1) normalizing disabled element opacity to Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/raystack/components/checkbox/checkbox.module.css (1)
49-56:⚠️ Potential issue | 🟡 MinorMissing
pointer-events: nonemay cause unintended hover effects on disabled checkbox.The disabled checkbox state only applies
opacity: 0.5but doesn't disable pointer events. This means:
- The disabled checkbox will still show
cursor: pointer(inherited from line 20)- The hover state override (lines 53-56) will be triggered, showing visual feedback
Other components in this PR (tabs, breadcrumb, sidebar) use
pointer-events: noneto prevent interaction on disabled elements.🐛 Proposed fix
.checkbox[data-disabled] { opacity: 0.5; + pointer-events: none; } - -.checkbox[data-disabled]:hover { - background: var(--rs-color-background-base-primary); - border-color: var(--rs-color-border-base-primary); -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/checkbox/checkbox.module.css` around lines 49 - 56, The disabled checkbox style currently only sets opacity but leaves pointer interactions enabled, so update the .checkbox[data-disabled] rule to include pointer-events: none and ensure it also resets cursor to default (to override the .checkbox cursor: pointer rule), and remove or no-op the .checkbox[data-disabled]:hover rule (or keep it but ensure it does nothing) so disabled checkboxes do not show hover/border changes; modify the selectors .checkbox[data-disabled] and .checkbox[data-disabled]:hover accordingly to mirror the other components' disabled behavior.packages/raystack/components/radio/radio.module.css (1)
37-41:⚠️ Potential issue | 🟠 MajorDisabled radio items still present a pointer cursor.
Because
.radioitemkeepscursor: pointer(Line 18) and disabled styles here don’t override it, disabled radios still look interactive on hover.Suggested fix
.radioitem[data-disabled], .radioitem[data-disabled]:hover { background: var(--rs-color-background-neutral-secondary); border-color: var(--rs-color-border-base-secondary); + cursor: default; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/radio/radio.module.css` around lines 37 - 41, The disabled radio styles for .radioitem[data-disabled] do not override the interactive cursor set on .radioitem, so update the CSS for .radioitem[data-disabled] and .radioitem[data-disabled]:hover to explicitly set cursor to default or not-allowed (e.g., cursor: default or cursor: not-allowed) and ensure hover rules also include that cursor value so disabled radios no longer show the pointer; modify the selectors referencing .radioitem[data-disabled] and .radioitem[data-disabled]:hover accordingly.packages/raystack/components/button/button.module.css (1)
101-106:⚠️ Potential issue | 🟠 MajorDisabled cursor behavior is inconsistent across button variants.
Line 105 sets
cursor: defaultonly for outline buttons, but shared disabled states (Lines 32-36) still inheritcursor: pointerfrom Line 11 for other variants. This breaks the “standardized disabled cursor” goal.Suggested fix
.button:disabled, .button-disabled { opacity: 0.5; pointer-events: initial; + cursor: default; } @@ .button-outline:disabled, .button-outline.button-disabled { opacity: 0.5; background-color: transparent; - cursor: default; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/button/button.module.css` around lines 101 - 106, Shared disabled styles currently leave cursor as pointer for non-outline variants; update the disabled selector logic so the standardized disabled cursor (cursor: default) applies to all button variants rather than only .button-outline. Modify the shared disabled selectors (e.g., .button:disabled and .button.button-disabled or whatever shared disabled rules are present) to include cursor: default, or consolidate by adding a broad selector that targets all disabled buttons and button-disabled classes in addition to .button-outline:disabled/.button-outline.button-disabled so every variant uses cursor: default when disabled.
🧹 Nitpick comments (2)
packages/raystack/components/calendar/calendar.module.css (1)
36-39: Consider removing color override from.navButton:disabledfor consistency.The
.navButton:disabledrule retains an explicitcoloroverride alongsideopacity: 0.5. Other disabled states in this PR rely solely on opacity for visual indication. This could be cleaned up in a follow-up for full consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/calendar/calendar.module.css` around lines 36 - 39, Remove the explicit color override from the .navButton:disabled rule and rely only on opacity: 0.5 for the disabled visual treatment; update the .navButton:disabled CSS selector to drop the color property so it matches other disabled states that use opacity alone and avoids overriding inherited/semantic colors.packages/raystack/components/switch/switch.module.css (1)
36-42: Consider addingopacity: 0.5for consistency with other disabled components.The switch component uses background color overrides for its disabled state rather than the
opacity: 0.5pattern used elsewhere in this PR (tabs, sidebar, checkbox, etc.). If this is intentional for design reasons (e.g., to maintain clear toggle state visibility), consider adding a brief comment to document this deviation.💡 Optional: Add opacity for consistency
.switch[data-disabled] { background: var(--rs-color-background-neutral-primary); + opacity: 0.5; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/switch/switch.module.css` around lines 36 - 42, The disabled switch CSS currently overrides background but doesn't apply the project's standard disabled opacity; update the .switch[data-disabled] rule (and .switch[data-disabled][data-checked] rule) to include opacity: 0.5 so the switch matches other disabled components, and if this background override is intentional instead of opacity, add a short comment above those selectors documenting the design rationale; locate the selectors .switch[data-disabled] and .switch[data-disabled][data-checked] in switch.module.css to make the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/raystack/components/button/button.module.css`:
- Around line 101-106: Shared disabled styles currently leave cursor as pointer
for non-outline variants; update the disabled selector logic so the standardized
disabled cursor (cursor: default) applies to all button variants rather than
only .button-outline. Modify the shared disabled selectors (e.g.,
.button:disabled and .button.button-disabled or whatever shared disabled rules
are present) to include cursor: default, or consolidate by adding a broad
selector that targets all disabled buttons and button-disabled classes in
addition to .button-outline:disabled/.button-outline.button-disabled so every
variant uses cursor: default when disabled.
In `@packages/raystack/components/checkbox/checkbox.module.css`:
- Around line 49-56: The disabled checkbox style currently only sets opacity but
leaves pointer interactions enabled, so update the .checkbox[data-disabled] rule
to include pointer-events: none and ensure it also resets cursor to default (to
override the .checkbox cursor: pointer rule), and remove or no-op the
.checkbox[data-disabled]:hover rule (or keep it but ensure it does nothing) so
disabled checkboxes do not show hover/border changes; modify the selectors
.checkbox[data-disabled] and .checkbox[data-disabled]:hover accordingly to
mirror the other components' disabled behavior.
In `@packages/raystack/components/radio/radio.module.css`:
- Around line 37-41: The disabled radio styles for .radioitem[data-disabled] do
not override the interactive cursor set on .radioitem, so update the CSS for
.radioitem[data-disabled] and .radioitem[data-disabled]:hover to explicitly set
cursor to default or not-allowed (e.g., cursor: default or cursor: not-allowed)
and ensure hover rules also include that cursor value so disabled radios no
longer show the pointer; modify the selectors referencing
.radioitem[data-disabled] and .radioitem[data-disabled]:hover accordingly.
---
Nitpick comments:
In `@packages/raystack/components/calendar/calendar.module.css`:
- Around line 36-39: Remove the explicit color override from the
.navButton:disabled rule and rely only on opacity: 0.5 for the disabled visual
treatment; update the .navButton:disabled CSS selector to drop the color
property so it matches other disabled states that use opacity alone and avoids
overriding inherited/semantic colors.
In `@packages/raystack/components/switch/switch.module.css`:
- Around line 36-42: The disabled switch CSS currently overrides background but
doesn't apply the project's standard disabled opacity; update the
.switch[data-disabled] rule (and .switch[data-disabled][data-checked] rule) to
include opacity: 0.5 so the switch matches other disabled components, and if
this background override is intentional instead of opacity, add a short comment
above those selectors documenting the design rationale; locate the selectors
.switch[data-disabled] and .switch[data-disabled][data-checked] in
switch.module.css to make the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2c9de43-bc3b-416b-b97a-627862180fa3
📒 Files selected for processing (16)
packages/raystack/components/breadcrumb/breadcrumb.module.csspackages/raystack/components/button/button.module.csspackages/raystack/components/calendar/calendar.module.csspackages/raystack/components/checkbox/checkbox.module.csspackages/raystack/components/combobox/combobox.module.csspackages/raystack/components/icon-button/icon-button.module.csspackages/raystack/components/input-field/input-field.module.csspackages/raystack/components/menu/cell.module.csspackages/raystack/components/radio/radio.module.csspackages/raystack/components/search/search.module.csspackages/raystack/components/select/select.module.csspackages/raystack/components/sidebar/sidebar.module.csspackages/raystack/components/switch/switch.module.csspackages/raystack/components/tabs/tabs.module.csspackages/raystack/components/text-area/text-area.module.csspackages/raystack/components/toggle/toggle.module.css
💤 Files with no reviewable changes (2)
- packages/raystack/components/search/search.module.css
- packages/raystack/components/toggle/toggle.module.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
refactor: standardize disabled state and cursor: default for all componentes
Type of Change
How Has This Been Tested?
[Describe the tests that you ran to verify your changes]
Checklist:
Screenshots (if appropriate):
[Add screenshots here]
Related Issues
[Link any related issues here using #issue-number]