Skip to content

chore: remove primer_react_spinner_synchronize_animations feature flag#7739

Open
Copilot wants to merge 3 commits intomainfrom
copilot/remove-primer-react-spinner-flag
Open

chore: remove primer_react_spinner_synchronize_animations feature flag#7739
Copilot wants to merge 3 commits intomainfrom
copilot/remove-primer-react-spinner-flag

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 9, 2026

The primer_react_spinner_synchronize_animations feature flag has been shipped to everyone. This PR removes it from the codebase, treating the behavior as always enabled.

Changelog

New

Changed

  • Spinner animation synchronization is now always enabled (when user has no reduced-motion preference), no longer gated behind a feature flag.
  • Updated TextInputWithTokens snapshot to reflect the Spinner now always rendering with animation-delay style attribute.

Removed

  • primer_react_spinner_synchronize_animations feature flag removed from DefaultFeatureFlags and Spinner.
  • Spinner example removed from feature flags skill documentation.

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

All existing Spinner tests (19) pass without modification. The TextInputWithTokens snapshot was updated to accept the new Spinner DOM that now always includes style="animation-delay: 0ms;". The feature flag was already shipped to 100% of users, so this is a no-op change in behavior.

Merge checklist

The feature flag has been shipped to everyone, so we can remove it
and treat the behavior as always enabled.

- Remove useFeatureFlag import and usage from Spinner.tsx
- Simplify animation sync condition to just check noMotionPreference
- Remove flag from DefaultFeatureFlags.ts
- Remove spinner example from feature-flags SKILL.md

Agent-Logs-Url: https://github.com/primer/react/sessions/b51a6323-d68b-4a4c-8136-0de3e4fe87ec

Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 9, 2026

🦋 Changeset detected

Latest commit: 4ee4f42

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

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

@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 Apr 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@siddharthkp siddharthkp marked this pull request as ready for review April 9, 2026 09:11
@siddharthkp siddharthkp requested a review from a team as a code owner April 9, 2026 09:11
@siddharthkp siddharthkp added skip changeset This change does not need a changelog and removed skip changeset This change does not need a changelog labels Apr 9, 2026
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

Removes the primer_react_spinner_synchronize_animations feature flag now that the Spinner animation-sync behavior is treated as permanently enabled across the codebase.

Changes:

  • Removed useFeatureFlag('primer_react_spinner_synchronize_animations') usage from Spinner and always apply sync delay when motion is allowed.
  • Removed the flag from DefaultFeatureFlags.
  • Removed the Spinner example snippet from the feature-flag skill documentation.
Show a summary per file
File Description
packages/react/src/Spinner/Spinner.tsx Drops the feature-flag gate and always uses animationDelay sync when prefers-reduced-motion allows motion.
packages/react/src/FeatureFlags/DefaultFeatureFlags.ts Removes the obsolete default flag entry.
.github/skills/feature-flags/SKILL.md Removes the Spinner example now that the flag no longer exists.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 1

@siddharthkp
Copy link
Copy Markdown
Member

@copilot The vitest tests for Spinner are failing because the snapshot of the dom has changed. That's intentional, accept the new snapshot and commit it.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 9, 2026

@copilot The vitest tests for Spinner are failing because the snapshot of the dom has changed. That's intentional, accept the new snapshot and commit it.

Updated the snapshot in 4ee4f42. The TextInputWithTokens > renders with a loading indicator snapshot now includes the style="animation-delay: 0ms;" attribute on all Spinner SVGs, which is expected since the sync behavior is always enabled now.

@siddharthkp siddharthkp added the Canary Release Apply this label when you want CI to create a canary release of the current PR label Apr 9, 2026
Copy link
Copy Markdown
Member

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

@primer-integration
Copy link
Copy Markdown

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

@primer-integration
Copy link
Copy Markdown

Integration test results from github/github-ui:

Failed  CI   Failed
Passed  VRT   Passed
Passed  Projects   Passed

CI check runs linting, type checking, and unit tests. Check the workflow logs for specific failures.

Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary Release Apply this label when you want CI to create a canary release of the current PR integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants