Skip to content

Button ReadOnly property#4099

Merged
James-Baloyi merged 1 commit intoshesha-io:releases/0.44from
czwe-01:thulasizwe/f/3925
Oct 30, 2025
Merged

Button ReadOnly property#4099
James-Baloyi merged 1 commit intoshesha-io:releases/0.44from
czwe-01:thulasizwe/f/3925

Conversation

@czwe-01
Copy link
Copy Markdown
Collaborator

@czwe-01 czwe-01 commented Oct 30, 2025

Fix readonly mode on button group configurator

Summary by CodeRabbit

  • Bug Fixes
    • Button group items now properly respond to the read-only state by being disabled when applicable, ensuring consistent user interaction and preventing unintended actions.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 30, 2025

Walkthrough

Modified ButtonGroupItem to apply the native disabled prop when readOnly is true and removed the corresponding inline cursor style override, relying on the component's computed styles instead.

Changes

Cohort / File(s) Summary
Button read-only state handling
shesha-reactjs/src/components/buttonGroupConfigurator/buttonGroupItem.tsx
Added disabled={readOnly} to Button component; removed conditional inline style that set cursor: not-allowed when readOnly, allowing standard computed styles to apply

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify cursor behavior is still visually correct for read-only buttons
  • Ensure the native disabled prop doesn't conflict with existing accessibility or styling mechanisms
  • Check that no unintended side effects occur in parent components that rely on this button's interaction state

Possibly related PRs

Suggested reviewers

  • James-Baloyi
  • AlexStepantsov

Poem

🐰 A button once worried in read-only dreams,
Now disabled it stands, in nature's own schemes.
No cursor tricks needed, no styles applied,
Just a native prop, simple and tried.
One small hop forward, accessibility sighed! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title "Button ReadOnly property" is related to the changeset, as the PR involves modifications to how the readOnly property is handled on a button component in the button group configurator. However, the title is somewhat vague and generic—it mentions the topic but doesn't clearly convey what was actually changed or fixed. A reader scanning commit history wouldn't immediately understand whether this is about adding, removing, fixing, or modifying the readOnly property without additional context. The PR description "Fix readonly mode on button group configurator" is more informative than the title itself. Consider revising the title to be more specific and descriptive about what was changed. For example, "Disable button component when readOnly in button group configurator" or "Fix readonly mode styling on button group items" would more clearly communicate the primary change and help teammates understand the intent without needing to examine the full changeset.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@czwe-01 czwe-01 requested a review from James-Baloyi October 30, 2025 09:47
@czwe-01 czwe-01 changed the title thulasizwe/f/3925 Button ReadOnly property Oct 30, 2025
@James-Baloyi James-Baloyi merged commit 19f9c82 into shesha-io:releases/0.44 Oct 30, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants