Skip to content

RG-2774 Fix ButtonGroup disabled detection with nested children#9260

Open
Hypnosphi wants to merge 1 commit intomasterfrom
air/https-youtrack.jetbrains.com-issue-rg-2774-button-group-cant-det-37caefd8-c
Open

RG-2774 Fix ButtonGroup disabled detection with nested children#9260
Hypnosphi wants to merge 1 commit intomasterfrom
air/https-youtrack.jetbrains.com-issue-rg-2774-button-group-cant-det-37caefd8-c

Conversation

@Hypnosphi
Copy link
Copy Markdown
Contributor

@Hypnosphi Hypnosphi commented Apr 10, 2026

Summary

  • Fix styles.disabled condition in ButtonGroup to recursively traverse nested children (e.g. buttons wrapped in <Tooltip>) when determining whether all buttons are disabled
  • Extract isChildDisabled helper that checks disabled prop on direct button elements and recurses into wrapper elements' children
  • Update story to wrap disabled buttons in <Tooltip> to demonstrate the fix

Fixes https://youtrack.jetbrains.com/issue/RG-2774

Test plan

  • Verify the "all disabled" story section renders with disabled border styling
  • Verify non-disabled groups are unaffected
  • Verify empty ButtonGroup does not incorrectly get disabled styling

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings April 10, 2026 17:46
@Hypnosphi Hypnosphi force-pushed the air/https-youtrack.jetbrains.com-issue-rg-2774-button-group-cant-det-37caefd8-c branch from 085a68c to bfd013e Compare April 10, 2026 17:49
@Hypnosphi Hypnosphi force-pushed the air/https-youtrack.jetbrains.com-issue-rg-2774-button-group-cant-det-37caefd8-c branch from bfd013e to 8359d37 Compare April 10, 2026 17:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes ButtonGroup “all disabled” styling when buttons are wrapped in nested components (e.g., <Tooltip>), by making the disabled detection traverse into nested children.

Changes:

  • Add a recursive isChildDisabled helper and use it to compute styles.disabled for ButtonGroup.
  • Update the Storybook story to wrap disabled buttons in <Tooltip> to demonstrate the nested-children case.
  • Update lockfile metadata (yauzl range for @ring-ui/screenshots) and remove an .idea VCS settings block.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/button-group/button-group.tsx Implements recursive disabled detection to decide when to apply group disabled styling.
src/button-group/button-group.stories.tsx Adjusts the story to include wrapped disabled buttons (Tooltip) to validate the fix visually.
package-lock.json Updates the recorded semver range for yauzl in @ring-ui/screenshots metadata.
.idea/vcs.xml Removes IDE GitHub shared project settings (branch protection patterns).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Hypnosphi Hypnosphi marked this pull request as draft April 10, 2026 18:03
'props' in child &&
(child as ReactElement<ButtonAttrs>).props.disabled,
),
[styles.disabled]: childArray.length > 0 && childArray.every(isChildDisabled),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this.props.children is already ReactNode — exactly what isChildDisabled() needs — it can be passed there as is, without the conversion to an array

Copy link
Copy Markdown
Contributor Author

@Hypnosphi Hypnosphi Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will short circuit on !('props' in child) if there's more than one child. So I will add proper handling for that case

@Hypnosphi Hypnosphi marked this pull request as ready for review April 10, 2026 18:12
@aleksei-berezkin aleksei-berezkin self-requested a review April 10, 2026 18:27
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.

3 participants