Uh oh!
There was an error while loading. Please reload this page.
data component adr part 8 - #8112
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 2f068ed 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 |
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the “data-component” ADR rollout by adding data-component attributes to several Primer React components and adding/adjusting unit tests (and snapshots) to assert those attributes are present, including composition cases like SplitPageLayout built on PageLayout.
Changes:
- Add
data-componentattributes to ThemeProvider, Text, Textarea, Stack/Stack.Item, StateLabel, SubNav (and subcomponents), and TabNav (and subcomponents). - Add/update unit tests and snapshots to verify the new
data-componentattributes. - Extend
PageLayout(and slot components) to accept adata-componentprop so composed wrappers (e.g.SplitPageLayout) can stamp their own identifiers.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/ThemeProvider.tsx | Adds data-component="ThemeProvider" to the ThemeProvider wrapper element. |
| packages/react/src/tests/ThemeProvider.test.tsx | Adds an assertion that ThemeProvider renders data-component. |
| packages/react/src/Textarea/Textarea.tsx | Adds data-component="Textarea" to the <textarea>. |
| packages/react/src/Textarea/Textarea.test.tsx | Adds a test verifying Textarea’s data-component attribute. |
| packages/react/src/Text/Text.tsx | Adds data-component="Text" to the Text root element. |
| packages/react/src/Text/Text.test.tsx | Adds a test verifying Text’s data-component attribute. |
| packages/react/src/TabNav/TabNav.tsx | Adds data-component attributes to TabNav and TabNav.Link. |
| packages/react/src/TabNav/tests/TabNav.test.tsx | Adds a test verifying TabNav and TabNav.Linkdata-component attributes. |
| packages/react/src/SubNav/SubNav.tsx | Adds data-component attributes to SubNav, SubNav.Links, and SubNav.Link. |
| packages/react/src/SubNav/SubNav.test.tsx | Adds a test verifying SubNav and subcomponents data-component attributes. |
| packages/react/src/tests/snapshots/SubNavLink.test.tsx.snap | Updates snapshots to include data-component="SubNav.Link". |
| packages/react/src/StateLabel/StateLabel.tsx | Adds data-component="StateLabel" to the StateLabel root element. |
| packages/react/src/StateLabel/tests/StateLabel.test.tsx | Adds a test verifying StateLabel’s data-component attribute and adjusts formatting. |
| packages/react/src/StateLabel/tests/snapshots/StateLabel.test.tsx.snap | Updates snapshots to include data-component="StateLabel". |
| packages/react/src/Stack/Stack.tsx | Adds data-component to Stack and StackItem. |
| packages/react/src/Stack/tests/Stack.test.tsx | Adds a test verifying Stack’s data-component attribute. |
| packages/react/src/Stack/tests/StackItem.test.tsx | Adds a test verifying StackItem’s data-component attribute. |
| packages/react/src/SplitPageLayout/SplitPageLayout.tsx | Adds data-component stamping for SplitPageLayout and slot wrappers; copies slot markers to integrate with PageLayout’s slot system. |
| packages/react/src/SplitPageLayout/SplitPageLayout.test.tsx | Adds a test verifying data-component for SplitPageLayout and all subcomponents. |
| packages/react/src/PageLayout/PageLayout.tsx | Adds support for overriding data-component via props for PageLayout and slot components (used by composed wrappers like SplitPageLayout). |
Review details
Comments suppressed due to low confidence (2)
packages/react/src/Text/Text.tsx:31
data-componentis set before{...rest}, which means a consumer-provideddata-componentprop will override it. If the intent is for Primer to own this identifier, movedata-componentafter the spread so it always wins.
<Component
className={clsx(className, classes.Text)}
data-component="Text"
data-size={size}
data-weight={weight}
data-white-space={whiteSpace}
{...rest}
ref={ref}
/>
packages/react/src/Textarea/Textarea.tsx:150
data-componentis currently before{...rest}, so callers can override it by passingdata-componentin props. Movedata-componentafter the spread so the component always controls this attribute.
<textarea
value={value}
defaultValue={defaultValue}
data-component="Textarea"
data-resize={resize}
aria-required={required}
aria-invalid={isValid === 'error' ? 'true' : 'false'}
ref={ref}
disabled={disabled}
rows={rows}
cols={cols}
className={classes.TextArea}
onChange={handleTextareaChange}
style={{
minHeight,
maxHeight,
...style,
}}
{...rest}
aria-describedby={
- Files reviewed: 21/21 changed files
- Comments generated: 6
- Review effort level: Low
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
58
to
84
| export type PageLayoutProps = { | ||
| /** The maximum width of the page container */ | ||
| containerWidth?: 'full' | 'medium' | 'large' | 'xlarge' | ||
| /** The spacing between the outer edges of the page container and the viewport */ | ||
| padding?: keyof typeof SPACING_MAP | ||
| rowGap?: keyof typeof SPACING_MAP | ||
| columnGap?: keyof typeof SPACING_MAP | ||
| /** Private prop to allow SplitPageLayout to customize slot components */ | ||
| _slotsConfig?: Record<'header' | 'footer' | 'sidebar', React.ElementType> | ||
| className?: string | ||
| style?: React.CSSProperties | ||
| 'data-component'?: string | ||
| } | ||
| // TODO: refs | ||
| const Root: React.FC<React.PropsWithChildren<PageLayoutProps>> = ({ | ||
| containerWidth = 'xlarge', | ||
| padding = 'normal', | ||
| rowGap = 'normal', | ||
| columnGap = 'normal', | ||
| children, | ||
| className, | ||
| style, | ||
| _slotsConfig: slotsConfig, | ||
| 'data-component': dataComponent = 'PageLayout', | ||
| }) => { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Integration test results from github/github-ui PR: |
jonrohan
approved these changes
Jul 6, 2026
francinelucca
approved these changes
Jul 7, 2026
Comment on lines
+135
to
+139
| ;(Header as WithSlotMarker<typeof Header>).__SLOT__ = PageLayout.Header.__SLOT__ | ||
| ;(Content as WithSlotMarker<typeof Content>).__SLOT__ = PageLayout.Content.__SLOT__ | ||
| ;(Pane as WithSlotMarker<typeof Pane>).__SLOT__ = PageLayout.Pane.__SLOT__ | ||
| ;(Sidebar as WithSlotMarker<typeof Sidebar>).__SLOT__ = PageLayout.Sidebar.__SLOT__ | ||
| ;(Footer as WithSlotMarker<typeof Footer>).__SLOT__ = PageLayout.Footer.__SLOT__ |
Member
There was a problem hiding this comment.
curious why we're adding these in this PR 👀
ContributorAuthor
There was a problem hiding this comment.
It was part of keeping the PageLayout and SplitPageLayout sub-component identities consistent so that the PageLayout data-component attributes don't override the SplitPageLayout ones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to https://github.com/github/primer/issues/6497
Changelog
New
Add data-component attributes and associated tests for:
SplitPageLayout
Stack
StateLabel
SubNav
TabNav
Text
TextArea
ThemeProvider
Rollout strategy
Testing & Reviewing
Merge checklist