Skip to content

Conversation

francinelucca
Copy link
Member

Reverts #6516

@Copilot Copilot AI review requested due to automatic review settings October 11, 2025 01:34
@francinelucca francinelucca requested a review from a team as a code owner October 11, 2025 01:34
Copy link

changeset-bot bot commented Oct 11, 2025

⚠️ No Changeset found

Latest commit: e1a8b6f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the staff Author is a staff member label Oct 11, 2025
Copy link
Contributor

@Copilot 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

This PR reverts a previous change (#6516) that added accessibility functionality for preventing actions when aria-disabled is set on SegmentedControl components. The revert removes the disabled state handling from SegmentedControl components and restores tooltip feature flag gating.

  • Reverts disabled/aria-disabled prop support and associated action prevention logic
  • Restores feature flag gating for tooltip functionality in SegmentedControlIconButton
  • Removes CSS styling for disabled states and story examples

Reviewed Changes

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

Show a summary per file
File Description
packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx Removes disabled props, restores feature flag for tooltip behavior
packages/react/src/SegmentedControl/SegmentedControlIconButton.stories.tsx Removes disabled props from stories
packages/react/src/SegmentedControl/SegmentedControlButton.tsx Removes disabled props and aria-disabled handling
packages/react/src/SegmentedControl/SegmentedControl.tsx Removes disabled state checking in onClick handlers
packages/react/src/SegmentedControl/SegmentedControl.test.tsx Updates tests for feature flag gating, adds test for non-tooltip behavior
packages/react/src/SegmentedControl/SegmentedControl.module.css Removes CSS styling for aria-disabled states
packages/react/src/SegmentedControl/SegmentedControl.examples.stories.tsx Removes entire file with disabled button examples
packages/react/src/SegmentedControl/SegmentedControl.dev.stories.tsx Removes disabled and aria-disabled story examples
packages/react/src/FeatureFlags/DefaultFeatureFlags.ts Adds back primer_react_segmented_control_tooltip feature flag
.changeset/strong-mangos-rest.md Removes changeset file

Comment on lines +52 to +54
<span className={clsx(classes.Content, 'segmentedControl-content')}>
{isElement(Icon) ? Icon : <Icon />}
</span>
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

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

Code duplication: The same span with identical structure and content exists on line 70. Consider extracting this into a reusable component or variable to reduce duplication.

Copilot uses AI. Check for mistakes.

Comment on lines 166 to +169
? (event: React.MouseEvent<HTMLButtonElement>) => {
const isDisabled = child.props.disabled === true || child.props['aria-disabled'] === true
if (!isDisabled) {
onChange(index)
isUncontrolled && setSelectedIndexInternalState(index)
child.props.onClick && child.props.onClick(event)
}
onChange(index)
isUncontrolled && setSelectedIndexInternalState(index)
child.props.onClick && child.props.onClick(event)
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

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

Missing closing brace: The conditional block starting at line 166 is missing its closing brace, making the code structure unclear and potentially causing syntax errors.

Copilot uses AI. Check for mistakes.

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Oct 11, 2025
Copy link
Contributor

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

@primer-integration
Copy link

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/4410

@github-actions github-actions bot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Oct 11, 2025
@github-actions github-actions bot added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: failing Changes in this PR cause breaking changes in gh/gh labels Oct 11, 2025
@francinelucca
Copy link
Member Author

FYI @khiga8 @TylerJDev looks like this broke the release so taking it out

@TylerJDev
Copy link
Member

@francinelucca, is this even with the changes listed in #6952 (comment)?

@primer-integration
Copy link

🟢 ci completed with status success.

@francinelucca
Copy link
Member Author

@francinelucca, is this even with the changes listed in #6952 (comment)?

still seeing errors even when pulling in those changes https://github.com/github/github-ui/actions/runs/18450776886/job/52564004666?pr=4409

@francinelucca francinelucca merged commit 930b401 into main Oct 13, 2025
47 checks passed
@francinelucca francinelucca deleted the revert-6516-kh-segmented-control branch October 13, 2025 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants