-
Notifications
You must be signed in to change notification settings - Fork 656
ADR-023: Public data-component API for targeting component parts #7576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3dd34d5
a5adc10
1e3e4a6
bd1d401
fdcd038
49af9d4
1883137
5e07cb1
02ddb5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,308 @@ | ||
| # Public `data-component` and `data-part` API for targeting component parts | ||
|
|
||
| 📆 Date: 2026-02-20 | ||
|
|
||
| ## Status | ||
|
|
||
| | Stage | State | | ||
| | -------------- | ----------- | | ||
| | Status | Proposed ❓ | | ||
| | Implementation | | | ||
|
|
||
| ## Context | ||
|
|
||
| Primer React uses [CSS Modules](https://github.com/css-modules/css-modules) for | ||
| styling. CSS Modules generate hashed class names (e.g., | ||
| `prc-ActionList-Item-cBBI`) that are not stable across versions. This makes it | ||
| impossible for consumers to reliably target internal parts of a component for | ||
| style overrides using class selectors. | ||
|
|
||
| Consumers need the ability to target component parts for legitimate use cases: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a good change! Is this something consumers are currently asking for, or are we anticipating future needs and implementing it in advance?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1000% to @TylerJDev comment, I'd please like to see real problems and use cases for this that justify this. See this https://github.com/github/primer/discussions/6393 for why I'm suggesting changes. I think this is unnecessary and could lead to a very fragile system.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is slightly tangential to your post. The root of the problem is some components not being composable at all. For example, SelectPanel and Autocomplete are black boxes that take title, items, etc. but gives you little control over customising the styles of internal components
This is a tricky spot we always find ourselves in. If we don't give folks the ability to customise, the only option is to fork/eject. If we do give some customisations, we are now on the hook to support backward compatibility with those customisations as well. 😢 For me, a balance does not exist in the long term, because it's not uncommon for us to need to change the internal implementation details for accessibility or other fixes. I'd love to hear ideas on how we can solve for this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I am currently doing interviews with folks and have hear this as an issue a few times. I don't have a list of the specific things, but I think especially copilot like copilot chat, ran into a lot of issues where they wanted to change small things but couldn't easily and thus rebuilt the component. @hectahertz are you suggesting that adding data-attr will have a huge performance impact or are you saying teams can kill performance by messing with the components? Can you elaborate on why this would make the system fragile?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So enterprise has some cases where they are trying to style components different. Currently they are wrapping the react component into another react component to add styles. However, it is hard to target specific parts. The span is actually the trailing action part they want to style. .chipInput {
border-radius: var(--borderRadius-full) !important;
& input:placeholder-shown + span {
display: none;
}
}
.chipInput span:last-child {
color: var(--control-fgColor-disabled) !important;
&:hover {
color: var(--fgColor-muted) !important;
cursor: pointer;
}
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lukasoppermann sorry, what component is that? Trying to see if there's a better way that already exists for this specific case For example, with TextInput, the ideal solution that @hectahertz suggested already works: <TextInput trailingVisual={() => <CalendarIcon className="custom-selector" />} />and then you can target it . & input:placeholder-shown + .custom-selector {}I feel this is better than . & input:placeholder-shown + [data-component=TextInput.TrailingVisual] {}because it also let's you do other things like <TextInput trailingVisual={() => <CalendarIcon data-testid="id" />} />
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @siddharthkp For your example, yes, this of course is nice.
However, I think 2 is a matter of opinion and I am happy to shift there, but I see 1 as a problem. Do you see this differently? Another example I remember is a case where folks needed to change the color of the icon in the banner, which I think is not easily possible.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Nope, same page as you
Yes, absolutely. I was trying to point the ideal scenario of how it should be.
Maybe but still very little additional work, so for customisation that goes off the system, I don't feel too bad about it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another example use cases I learned yesterday. Dustin needed to adjust the Dialog position as he was using it to build the command palette. He only made it work with a css hack, as the class is not forwarded to the needed element. He said that having a way to at least temporary move on (and possibly report a bug / feature request to primer) would have helped him a lot.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a lot of folks are (non-optimally) matching on partial css selectors and things like that, so it's definitely already needed |
||
|
|
||
| - Customizing the appearance of specific parts (e.g., hiding a trailing visual, | ||
| changing the font weight of a label) | ||
| - Querying parts of a component in JavaScript (e.g., finding all selected items | ||
| in an action list) | ||
| - Writing integration tests that target stable selectors | ||
|
|
||
| The `data-component` attribute is already used internally across multiple | ||
| components (`Button`, `ActionList`, `PageHeader`, `UnderlineNav`, `Avatar`, and | ||
| others) for internal CSS targeting and DOM queries. However, the naming | ||
| convention is inconsistent: | ||
|
|
||
| | Pattern | Examples | | ||
| | -------------------- | ------------------------------------------------ | | ||
| | `ComponentName.Part` | `ActionList.Description`, `ActionList.Selection` | | ||
| | `PREFIX_Part` | `PH_LeadingAction`, `PH_Title`, `PH_Navigation` | | ||
| | `camelCase` | `buttonContent`, `leadingVisual`, `text` | | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ouch, i think i know why this is so.
But Button's
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I think this is not needed. |
||
| | `PascalCase` | `Avatar`, `IconButton`, `SkeletonText` | | ||
|
|
||
| Because `data-component` is not documented as a public API, values have changed | ||
| without notice and coverage is incomplete — many component parts have no | ||
| `data-component` attribute at all. | ||
|
|
||
| ## Decision | ||
|
|
||
| Establish two **public, stable data attributes** for identifying components and | ||
| their parts in the DOM: | ||
|
|
||
| - **`data-component`** — identifies the root element of a component or | ||
| sub-component. | ||
| - **`data-part`** — identifies an inner structural part within a component. | ||
|
|
||
| ### Naming convention | ||
|
|
||
| All values use PascalCase. The two attributes serve distinct roles: | ||
|
|
||
| ``` | ||
| data-component="ComponentName" → root element of a component or sub-component | ||
| data-part="PartName" → inner part within a component | ||
| ``` | ||
|
|
||
| #### Rules | ||
|
|
||
| 1. **Root components** get `data-component` with their React component name. | ||
|
|
||
| ```html | ||
| <ul data-component="ActionList"></ul> | ||
| ``` | ||
|
|
||
| 2. **Public sub-components** get `data-component` matching the React API. If | ||
| consumers write `<ActionList.Item>`, the DOM element gets | ||
| `data-component="ActionList.Item"`. | ||
|
|
||
| ```html | ||
| <li data-component="ActionList.Item"></li> | ||
| ``` | ||
|
|
||
| Note: a sub-component root uses `data-component`, not `data-part`, because it | ||
| is itself a component — it has its own props, its own identity, and may | ||
| contain its own slots. | ||
|
|
||
| 3. **Inner structural parts** (DOM elements that are not exposed as a | ||
| sub-component but represent a meaningful part of the structure) get | ||
| `data-part` with a PascalCase name describing the part. | ||
|
|
||
| ```html | ||
| <span data-part="Label">monalisa</span> | ||
| <span data-part="Content">...</span> | ||
| <span data-part="LeadingVisual"><img /></span> | ||
| ``` | ||
|
|
||
| Slot names are **scoped to their parent component** — a `Label` slot inside | ||
| `ActionList.Item` is distinct from a `Label` slot inside `Button` because | ||
| they exist within different `data-component` boundaries. | ||
|
|
||
| 4. **State and modifier attributes remain separate.** `data-component` and | ||
| `data-part` identify _what_ an element is. Existing attributes like | ||
| `data-variant`, `data-size`, and `data-loading` describe the _state_ of that | ||
| element. These concerns must not be mixed. | ||
|
|
||
| ```html | ||
| <li data-component="ActionList.Item" data-variant="danger" data-active="true"> | ||
| <span data-part="Label">Delete file</span> | ||
| </li> | ||
| ``` | ||
|
|
||
| ### Relationship to CSS Modules and CSS Layers | ||
|
|
||
| `data-component` and `data-part` complement the existing styling architecture: | ||
|
|
||
| - **CSS Modules** provide scoped class names for internal styling. Components | ||
| continue to use CSS Module classes for their own styles. | ||
| - **CSS Layers** ([ADR-021](./adr-021-css-layers.md)) ensure that consumer | ||
| overrides take precedence over component styles regardless of specificity. | ||
| - **`data-component` and `data-part`** provide the stable selectors that | ||
| consumers use to target components and their parts within those overrides. | ||
|
|
||
| Together, these three mechanisms give consumers a complete override path: | ||
|
|
||
| ```css | ||
| /* Target a component */ | ||
| [data-component='ActionList.Item'] { | ||
| border-radius: 8px; | ||
| } | ||
|
|
||
| /* Target a slot within a component */ | ||
| [data-component='ActionList.Item'] [data-part='Label'] { | ||
| font-weight: 600; | ||
| } | ||
| ``` | ||
|
|
||
| ### Internal CSS usage | ||
|
|
||
| Components may use `data-part` selectors in their own CSS Modules for targeting | ||
| child parts. This replaces ad-hoc patterns like bare `[data-component='text']` | ||
| with the standardized naming: | ||
|
|
||
| ```css | ||
| /* ButtonBase.module.css */ | ||
| & :where([data-part='LeadingVisual']) { | ||
| color: var(--button-leadingVisual-fgColor); | ||
| } | ||
| ``` | ||
|
|
||
| ### Coverage requirements | ||
|
|
||
| Every component must provide: | ||
|
|
||
| - **`data-component`** on the root element of every component and public | ||
| sub-component | ||
| - **`data-part`** on every internal structural element that a consumer might | ||
| reasonably need to target (labels, content wrappers, visual slots, action | ||
| slots) | ||
|
|
||
| Elements that are purely for layout and have no semantic meaning (spacers, | ||
| wrappers that exist only for CSS grid/flex layout) do not require either | ||
| attribute. | ||
|
|
||
| ### Testing requirements | ||
|
|
||
| The presence and values of `data-component` and `data-part` attributes must be | ||
| covered by tests. This can be achieved through: | ||
|
|
||
| - Unit tests that assert the attributes are present on rendered elements | ||
| - Snapshot tests that capture the attribute values | ||
|
|
||
| Changing a `data-component` or `data-part` value is a **breaking change** and | ||
| must follow the standard breaking change process. | ||
|
|
||
| ### Versioning and breaking changes | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice if this section covered changes on the CSS side, as well, with respect to semver. I think this would be the core concern (personally) when it comes to overrides is that we introduce fragility into the system that can either be difficult to test for or lock us in. Some examples that come to mind:
/* Our component styles */
.Foo {
display: flex;
}
/* Downstream override */
.override[data-component="Foo"] {
align-items: center;
}This creates a contract where if
One example of this would be for spacing. Say we initially apply padding on a selector: .Example {
padding: var(--spacing-1);
}And downstream it is overridden: .override[data-component="Example"] {
padding: var(--spacing-2);
}If spacing is applied to a different node (say to a container or a child) then this will cause double spacing instead of continuing to be an override on the padding of the component.
To me, this situation comes up when someone is trying to override a core way the component may work (like they want it to be grid vs flex) and it's a race against overriding all the things that implement this. This ends up being a race because as you add or implement this logic in items, consumers need to create the rule to override it. It also can lead to unexpected breakage since the override competes with how the component works. I think the first scenario is almost always a challenge for overrides (or at least I haven't found a strategy to mitigate this). I assume the second bucket would be where component tokens come into play?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this falls under the customize at your own risk. This is something we need to mention on the docs about those overrides. But I think our guarantee says that we have a stable API for selecting parts. This is where it ends. We could add some examples like this and recommend to not rely on css values being there. So that folks for example could do: /* Downstream override */
.override[data-component="Foo"] {
display: flex;
align-items: center;
}However, we should of course mention any css change we make to a part so that folks can see this in the change logs. @joshblack the way I understand it, this is an issue we have to deal with right? This is the same today, when folks wrap components and overwrite them, or whatever other way they would use. |
||
|
|
||
| Because `data-component` and `data-part` are **public API**, changes to them | ||
| follow [Semantic Versioning](../versioning.md). The table below summarises what | ||
| requires which kind of release: | ||
|
|
||
| | Change | semver bump | | ||
| | ------------------------------------------------------------------ | ----------- | | ||
| | A `data-component` or `data-part` attribute is added to an element | `minor` | | ||
| | A `data-component` or `data-part` value is renamed | `major` | | ||
| | A `data-component` or `data-part` attribute is removed | `major` | | ||
| | An attribute is moved from one element to another | `major` | | ||
| | A `data-component` is changed to `data-part` or vice-versa | `major` | | ||
|
|
||
| **Deprecation path.** Before removing or renaming a value in a major release, | ||
| the old value should be deprecated in at least one prior minor release. During | ||
| the deprecation window the component must emit a development-mode console | ||
| warning (using the existing `warn` / `deprecate` helpers) so consumers have | ||
| time to migrate. | ||
|
|
||
| The [Migration](#migration) table below captures the full set of renames | ||
| planned for the next major release. | ||
|
|
||
| ### Versioning and breaking changes | ||
|
|
||
| Because `data-component` and `data-part` are **public API**, changes to them | ||
| follow [Semantic Versioning](../versioning.md). The table below summarises what | ||
| requires which kind of release: | ||
|
|
||
| | Change | semver bump | | ||
| | ------------------------------------------------------------------ | ----------- | | ||
| | A `data-component` or `data-part` attribute is added to an element | `minor` | | ||
| | A `data-component` or `data-part` value is renamed | `major` | | ||
| | A `data-component` or `data-part` attribute is removed | `major` | | ||
| | An attribute is moved from one element to another | `major` | | ||
| | A `data-component` is changed to `data-part` or vice-versa | `major` | | ||
|
|
||
| **Deprecation path.** Before removing or renaming a value in a major release, | ||
| the old value should be deprecated in at least one prior minor release. During | ||
| the deprecation window the component must emit a development-mode console | ||
| warning (using the existing `warn` / `deprecate` helpers) so consumers have | ||
| time to migrate. | ||
|
|
||
| The [Migration](#migration) table below captures the full set of renames | ||
| planned for the next major release. | ||
|
|
||
| ### Migration | ||
|
|
||
| Existing `data-component` values must be migrated to the new convention. Inner | ||
| parts move from `data-component` to `data-part` with simplified names (since | ||
| they are scoped to their parent component). This migration is a breaking change | ||
| and should be coordinated as part of a major release. | ||
|
|
||
| | Current attr | Current value | New attr | New value | | ||
| | ---------------- | --------------------------------------- | ---------------- | --------------------------- | | ||
| | `data-component` | `buttonContent` | `data-part` | `Content` | | ||
| | `data-component` | `text` (in Button) | `data-part` | `Label` | | ||
| | `data-component` | `leadingVisual` (in Button) | `data-part` | `LeadingVisual` | | ||
| | `data-component` | `trailingVisual` (in Button) | `data-part` | `TrailingVisual` | | ||
| | `data-component` | `trailingAction` (in Button) | `data-part` | `TrailingAction` | | ||
| | `data-component` | `ButtonCounter` | `data-part` | `Counter` | | ||
| | `data-component` | `PH_LeadingAction` | `data-part` | `LeadingAction` | | ||
| | `data-component` | `PH_Breadcrumbs` | `data-part` | `Breadcrumbs` | | ||
| | `data-component` | `PH_LeadingVisual` | `data-part` | `LeadingVisual` | | ||
| | `data-component` | `PH_Title` | `data-part` | `Title` | | ||
| | `data-component` | `PH_TrailingVisual` | `data-part` | `TrailingVisual` | | ||
| | `data-component` | `PH_TrailingAction` | `data-part` | `TrailingAction` | | ||
| | `data-component` | `PH_Actions` | `data-part` | `Actions` | | ||
| | `data-component` | `PH_Navigation` | `data-part` | `Navigation` | | ||
| | `data-component` | `TitleArea` | `data-part` | `TitleArea` | | ||
| | `data-component` | `GroupHeadingWrap` | `data-component` | `ActionList.GroupHeading` | | ||
| | `data-component` | `ActionList.Item--DividerContainer` | `data-part` | `SubContent` | | ||
| | `data-component` | `icon` (in UnderlineTabbedInterface) | `data-part` | `Icon` | | ||
| | `data-component` | `text` (in UnderlineTabbedInterface) | `data-part` | `Label` | | ||
| | `data-component` | `counter` (in UnderlineTabbedInterface) | `data-part` | `Counter` | | ||
| | `data-component` | `multilineContainer` | `data-part` | `Container` | | ||
| | `data-component` | `input` (in TextInput) | `data-part` | `Input` | | ||
| | `data-component` | `AnchoredOverlay` | `data-component` | `AnchoredOverlay` | | ||
| | `data-component` | `ActionBar.VerticalDivider` | `data-component` | `ActionBar.VerticalDivider` | | ||
|
|
||
| Components that currently have no attributes on key parts must also be updated. | ||
|
|
||
| ## Consequences | ||
|
|
||
| ### Positive | ||
|
|
||
| - **Stable selectors for consumers.** Consumers can target any component with | ||
| `[data-component="..."]` and any inner part with `[data-part="..."]` — both | ||
| are immune to CSS Module hash changes and version upgrades. | ||
| - **Clear separation.** `data-component` answers "which component is this?" | ||
| while `data-part` answers "which part of the component is this?" This makes | ||
| the DOM self-documenting and avoids overloading a single attribute. | ||
| - **Consistent naming.** A single convention replaces four inconsistent patterns, | ||
| making the codebase easier to learn and maintain. | ||
| - **Scoped slot names.** Because `data-part` values are scoped to their parent | ||
| `data-component`, names like `Label` or `LeadingVisual` can be reused across | ||
| components without ambiguity. | ||
| - **Enables JavaScript queries.** Consumers and tests can use | ||
| `querySelectorAll('[data-component="ActionList.Item"] [data-part="Label"]')` | ||
| reliably. | ||
| - **Complements CSS Layers.** Together with ADR-021, this gives consumers a | ||
| complete, specificity-safe override mechanism. | ||
|
|
||
| ### Negative | ||
|
|
||
| - **Breaking change for existing consumers.** Anyone currently relying on the | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we have data on how many
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 72 files in github/github-ui, not all of them are primer components, but I'm assuming most of them are |
||
| undocumented `data-component` values (e.g., in CSS overrides or | ||
| `querySelector` calls) will need to update when values are renamed. This must | ||
| be coordinated in a major release. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| ### 1. Stable class names alongside CSS Module classes | ||
|
|
||
| Add a non-hashed class name to every part (e.g., | ||
| `className={clsx(classes.Item, 'ActionList-item')}`). | ||
|
|
||
| **Why not chosen:** Pollutes the global CSS namespace. Risk of collisions with | ||
| consumer or third-party styles. Requires consumers to understand which class | ||
| names are "stable" vs. which are CSS Module hashes. Data attributes are a | ||
| cleaner separation of concerns — class names for styling, data attributes for | ||
| identification. | ||
|
|
||
| ### 2. CSS `::part()` pseudo-element | ||
|
|
||
| The `::part()` CSS pseudo-element allows styling of elements inside a shadow | ||
| DOM. | ||
|
|
||
| **Why not chosen:** Only works with Shadow DOM, which React does not use. Not | ||
| applicable to our architecture. | ||
|
|
||
| ### 3. Do nothing — keep `data-component` as an internal implementation detail | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we look into why they are doing that? I think we should categorize and solve those use cases properly instead.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a two step process:
The problem is that we can't deliver at feature team speed and that the teams need ways to customize components to explore ideas e.g. in flagged ships. If they always have to rebuild everything, it is unlikely they adjust it to use primer and create a PR once they have proven their idea is needed, because for them it brings little benefit compared to focusing on a new feature.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea of this , cuts down on teams having to wait for Primer to deliver on small customization/changes. We can then upstream and cleanup without teams having to be blocked for a week waiting on a Primer release. |
||
|
|
||
| Continue using `data-component` informally without guaranteeing stability. | ||
|
|
||
| **Why not chosen:** Consumers are already depending on these attributes for | ||
| overrides (as seen in SelectPanel story CSS). Without a stability guarantee, | ||
| any refactor can silently break consumer overrides. Formalizing the API | ||
| acknowledges the reality and provides a proper contract. | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consumers could be exposed a non-hashed classname, which would be a slightly tighter customization point than a data-attribute.
classes are better optimized for this than data attributes are. (edit - looks like that was discussed below. I don't have a strong feeling - but it always feels less ergonomic to use data selectors for child elements)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the ergonomic argument, but than again, we don't expect folks to change everything about components a lot on a daily basis.
On the upside, it is less likely that we get conflicts with other classes, if we use a data prop. vs a non-hashed class. (Yes, we could prefix everything with
Primer-to avoid this as well).I personally am open do either option, I feel having a bit of friction in here may not be bad, as this is supposed to be a stable escape hatch, but not the happy case.