chore: remove primer_react_spinner_synchronize_animations feature flag#7739
chore: remove primer_react_spinner_synchronize_animations feature flag#7739
Conversation
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 detectedLatest commit: 4ee4f42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
There was a problem hiding this comment.
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 fromSpinnerand 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
|
@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. |
Agent-Logs-Url: https://github.com/primer/react/sessions/93f4077c-a9ef-4fe4-9609-5730ffeb62ed Co-authored-by: siddharthkp <1863771+siddharthkp@users.noreply.github.com>
Updated the snapshot in |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/18257 |
|
Integration test results from github/github-ui:
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. |
The
primer_react_spinner_synchronize_animationsfeature flag has been shipped to everyone. This PR removes it from the codebase, treating the behavior as always enabled.Changelog
New
Changed
Spinneranimation synchronization is now always enabled (when user has no reduced-motion preference), no longer gated behind a feature flag.TextInputWithTokenssnapshot to reflect the Spinner now always rendering withanimation-delaystyle attribute.Removed
primer_react_spinner_synchronize_animationsfeature flag removed fromDefaultFeatureFlagsandSpinner.Rollout strategy
Testing & Reviewing
All existing Spinner tests (19) pass without modification. The
TextInputWithTokenssnapshot was updated to accept the new Spinner DOM that now always includesstyle="animation-delay: 0ms;". The feature flag was already shipped to 100% of users, so this is a no-op change in behavior.Merge checklist