Repository navigation
Graduate merged forwarded refs - #8548
llastflowers wants to merge 9 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: de5c3c24-5ce5-4887-b09b-888953781bde
🦋 Changeset detectedLatest commit: 7b3007e 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.
🔵 Needs a closer look
Cross-component ref lifecycle changes affect focus and positioning and warrant human confirmation without runtime validation.
0 open findings
What changed in this PR
Makes merged forwarded refs unconditional across Primer React, preserving consumer refs alongside internal component refs.
Changes:
- Removes the merged-ref feature flag and fallback paths.
- Updates anchor replacement tracking and applies complete tab-list props in UnderlinePanels.
- Simplifies ref tests and adds a patch changeset.
| File | Description |
|---|---|
| packages/react/src/TooltipV2/Tooltip.tsx | Always merges trigger and forwarded refs. |
| packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx | Removes flag-dependent test variants. |
| packages/react/src/TextInput/TextInput.tsx | Uses merged input refs unconditionally. |
| packages/react/src/TextInput/TextInput.test.tsx | Simplifies forwarded-ref tests. |
| packages/react/src/PageHeader/PageHeader.tsx | Preserves root and title refs without gating. |
| packages/react/src/PageHeader/PageHeader.test.tsx | Simplifies forwarded-ref tests. |
| packages/react/src/FilteredActionList/FilteredActionList.tsx | Always merges input and scroll-container refs. |
| packages/react/src/FilteredActionList/FilteredActionList.test.tsx | Simplifies ref tests. |
| packages/react/src/FeatureFlags/DefaultFeatureFlags.ts | Removes the merged-ref flag. |
| packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx | Applies all returned tab-list props. |
| packages/react/src/experimental/Tabs/useTabList.ts | Always merges tab-list refs. |
| packages/react/src/experimental/Tabs/Tabs.test.tsx | Simplifies tab-list ref tests. |
| packages/react/src/Dialog/Dialog.tsx | Uses merged autofocus-button refs unconditionally. |
| packages/react/src/Dialog/Dialog.test.tsx | Removes flag variants from autofocus testing. |
| packages/react/src/Checkbox/Checkbox.tsx | Always merges checkbox refs. |
| packages/react/src/Checkbox/Checkbox.test.tsx | Simplifies forwarded-ref tests. |
| packages/react/src/ButtonGroup/ButtonGroup.tsx | Always merges group refs. |
| packages/react/src/ButtonGroup/ButtonGroup.test.tsx | Simplifies forwarded-ref tests. |
| packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx | Tracks anchor replacements and always merges overlay refs. |
| packages/react/src/AnchoredOverlay/AnchoredOverlay.test.tsx | Simplifies overlay-ref tests. |
| packages/react/src/ActionList/List.tsx | Always merges list refs. |
| packages/react/src/ActionList/ActionList.test.tsx | Simplifies forwarded-ref tests. |
| .changeset/merged-forwarded-refs.md | Documents the ref-handling patch release. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: de5c3c24-5ce5-4887-b09b-888953781bde
tay1orjones
left a comment
There was a problem hiding this comment.
LGTM, the CI tests on the integration PR look like flakes to me 🤔
|
Integration test results from github/github-ui PR: |
Part of https://github.com/github/primer/issues/7263
Changelog
New
Changed
useTabList.Removed
primer_react_merged_forwarded_refsfeature flag and its fallback paths.Rollout strategy
@primer/reactand remove the client feature flag allowlist entry before deleting the DevPortal flag.Testing & Reviewing
npm test -- --runnpm run type-checknpm run lintnpm run lint:cssnpm run format:diffnpm run build