Skip to content

Export getResponsiveAttributes from experimental entrypoint#8144

Draft
rickyzhangca wants to merge 2 commits into
mainfrom
rickyzhangca-export-get-responsive-attributes
Draft

Export getResponsiveAttributes from experimental entrypoint#8144
rickyzhangca wants to merge 2 commits into
mainfrom
rickyzhangca-export-get-responsive-attributes

Conversation

@rickyzhangca

@rickyzhangca rickyzhangca commented Jul 9, 2026

Copy link
Copy Markdown

Exports the getResponsiveAttributes helper from the @primer/react/experimental entrypoint. It was previously only available as an internal util, even though it is the helper referenced by the useResponsiveValue @deprecated notice and ADR-018 as the sanctioned way to bridge responsive prop values to CSS via data-* attributes.

Consumers migrating off the deprecated useResponsiveValue hook currently have no supported way to import this helper. Placing it in experimental matches where the sibling responsive primitive Hidden already lives.

To keep the source location consistent with its new public status, the helper is moved out of src/internal/utils into src/utils (and its 4 internal importers — PageHeader, PageLayout, SegmentedControl, Stack — are repointed), matching the precedent set when useMergedRefs was promoted from internal to public, rather than re-exporting an internal/ path from a public entrypoint.

Changelog

New

  • Export getResponsiveAttributes from @primer/react/experimental.

Changed

  • Move getResponsiveAttributes from src/internal/utils to src/utils (no behavior change; internal import paths updated).

Removed

  • None

Rollout strategy

  • Minor release

Testing & Reviewing

Additive and low-risk. The helper's implementation is unchanged (its unit tests moved alongside it and still pass). The experimental exports snapshot was updated to include the new symbol, and a minor changeset was added. Scoped exports.test.ts passes (3/3), the moved/changed files lint clean, and type-check reports no src/ errors.

@rickyzhangca rickyzhangca requested a review from a team as a code owner July 9, 2026 18:51
@rickyzhangca rickyzhangca requested review from Copilot and joshblack July 9, 2026 18:51
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7bf9678

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 Minor

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 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

⚠️ 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. If this doesn't work, you can also use the original workflow here. 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.

@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 Jul 9, 2026

Copilot AI left a comment

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.

Pull request overview

Exports the getResponsiveAttributes helper via the @primer/react/experimental entrypoint so consumers have a supported migration path off the deprecated useResponsiveValue hook.

Changes:

  • Re-export getResponsiveAttributes from packages/react/src/experimental/index.ts.
  • Update the experimental exports snapshot to include getResponsiveAttributes.
  • Add a changeset for a minor release of @primer/react.
Show a summary per file
File Description
packages/react/src/experimental/index.ts Adds getResponsiveAttributes to the experimental entrypoint exports.
packages/react/src/tests/snapshots/exports.test.ts.snap Updates the snapshot to reflect the new experimental export.
.changeset/export-get-responsive-attributes.md Adds a changeset documenting the new export.

Review details

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment thread .changeset/export-get-responsive-attributes.md Outdated
Promotes the getResponsiveAttributes helper from an internal util to the
public @primer/react/experimental entrypoint (placed next to Hidden). The
source file is moved out of src/internal/utils into src/utils to match the
precedent set when useMergedRefs was made public, rather than re-exporting
from internal.

It is the helper referenced by the useResponsiveValue deprecation notice
and ADR-018 for bridging responsive prop values to CSS via data-* attributes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@joshblack joshblack left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this something broadly applicable / helping with migration downstream? The ADR itself is definitely written with primer/react in mind so that's why this stuff is more internal / hasn't been exported before as it was just our convention for doing this thing.

Are we seeing downstream that this would be helpful to adopt for useResponsiveValue migration? 👀

This export is referenced in the `useResponsiveValue` deprecation notice and ADR-018.
@rickyzhangca

rickyzhangca commented Jul 9, 2026

Copy link
Copy Markdown
Author

Is this something broadly applicable / helping with migration downstream? The ADR itself is definitely written with primer/react in mind so that's why this stuff is more internal / hasn't been exported before as it was just our convention for doing this thing.

Are we seeing downstream that this would be helpful to adopt for useResponsiveValue migration? 👀

yes we will use it to replace useResponsiveValue which is being used downstream, as part of the hook deprecation cleanup task

we will migrate them in 4 ways based on the situation

Case When to use Replacement SSR-safty
Native responsive prop The target Primer component's prop already accepts {narrow, regular, wide} (e.g. Stack direction, Dialog variant) Pass the object directly, delete the hook
Show/hide The value only decides whether some content renders (current {narrow: true} gating a block) <Hidden when="narrow">…</Hidden>
Style/value varies The value drives styling that CSS can express (size, gap, spacing, class) getResponsiveAttributes('name', value)data-* attrs + CSS @media
Escape hatch JS behavior genuinely depends on viewport measuring useMedia(...) ⚠️ client-only

e.g. github-models/routes/playground/components/PlaygroundHeader.tsx

before

const isMobile = useResponsiveValue({narrow: true}, false) as boolean
...
<div className={`${defaultClasses} ${isMobile ? 'pb-2' : 'tmp-pb-3'}`}>
  <SomeChild className={isMobile ? 'px-2' : ''} />
</div>

after

<div
  className={clsx(defaultClasses, styles.header)}
  {...getResponsiveAttributes('density', {narrow: 'compact', regular: 'comfortable'})}
>
  ...
  <SomeChild className={styles.headerAction} />
</div>

@joshblack

Copy link
Copy Markdown
Member

@rickyzhangca for that example, I feel like a more tailored migration would be to either use media queries or if they're using tailwind it would be to use the breakpoint prefix for the class name they want to apply, right? e.g. xs:p-3 or whatever our equivalent is there. Let me know what you think 👀

jonrohan
jonrohan previously approved these changes Jul 9, 2026
@jonrohan jonrohan dismissed their stale review July 9, 2026 20:25

Defer

@rickyzhangca rickyzhangca marked this pull request as draft July 9, 2026 21:16
@rickyzhangca

rickyzhangca commented Jul 9, 2026

Copy link
Copy Markdown
Author

Parking this as a draft. Let's prefer @media (--viewportRange-*) in CSS Modules and Hidden over getResponsiveAttributes. Keeping this ready to un-draft if we hit a case that genuinely needs the helper

@rickyzhangca

Copy link
Copy Markdown
Author

if they're using tailwind

curious, what's our stand on tailwind? is it for team to opt-in while we default to css modules?

@joshblack

Copy link
Copy Markdown
Member

@rickyzhangca I think you're spot on, the default is still CSS Modules but tailwind is available to use in github-ui 👍

@joshblack joshblack left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just wanted to approve in advance if we end up finding use-cases 👍

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

Labels

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