feat: add data-slot attributes across the entire design system - #872
Open
ravisuhag wants to merge 6 commits into
Open
feat: add data-slot attributes across the entire design system#872ravisuhag wants to merge 6 commits into
ravisuhag wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedToo many files! This PR contains 319 files, which is 219 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (319)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ravisuhag
changed the title
Every element these components render now carries a stable, kebab-case, component-prefixed data-slot identifier (e.g. filter-chip-remove, data-view-list-loader-row). This gives consumers a supported styling and testing hook for inner parts that have no classNames key, without targeting hashed CSS-module classes. - Add test-utils/data-slots with getSlot/getAllSlots/expectSlots and slot-contract tests for all three components - Document the convention in the styling guide and list slot names on each component's docs page - Require data-slot coverage in the add-new-component skill checklist First batch of the design-system-wide rollout. Closes the styling half of #846 (filter-chip internals and loader rows are now reachable). Popover.Content/Menu.Content call sites are deliberately skipped: their extra props land on Base UI's positioner, not the popup — those slots ship with the popover/menu batch.
Every element every component renders now carries a stable, kebab-case, component-prefixed data-slot identifier — the full rollout following the first batch (Search, FilterChip, DataView). 76 of 77 components covered; data-table is deliberately excluded (it's slated for deprecation in favor of DataView). Each component gets a __tests__/data-slots.test.tsx asserting its slot contract via the shared expectSlots/getSlot helper, and a docs "### Slots" section listing its slot names. Existing classNames props are untouched — this is purely additive. Popover/Menu/Dialog-style Content wrappers that split props between a Base UI Positioner and Popup get their slot hardcoded directly on the Popup, not routed through forwarded props, so it lands on the right element. Existing internal precedent (menu-trigger's data-slot='menu-subtrigger') is preserved unchanged. Fixes made while integrating parallel batches: - calendar: corrected a test's wrong assumption that DatePicker's error span unmounts when absent (it always mounts, toggled via data-visible, so screen readers hear it immediately on error) - avatar: corrected a test asserting the fallback and loaded image slots simultaneously (Base UI unmounts the fallback once the image loads) - chat: fixed a test rendering Chat.JumpButton outside its required Chat.Messages ancestor - text, headline, label, grid, grid-item: mergeProps' generic props type doesn't include data-slot, so passing it as a fresh object literal fails TypeScript's excess-property check. Extracted the props into a named const first (matching the existing pattern in flex.tsx), which resolves it without a cast.
…ency Checked the data-slot rollout against components.build's data-attributes guide. Two follow-ups: - Renamed Command's `command-input-wrapper` slot to `command-input-container`, matching the "-container" suffix used elsewhere (input-container, toast-viewport) instead of a bare "wrapper" name. - Documented the has-[]/[&_] compound-selector patterns in the styling guide — the main technique the guide highlights for parent-aware and descendant styling via data-slot, which wasn't shown before. No component change needed; existing slots already support it. Also updated the outdated "rolling out component by component" coverage note now that all components ship slots.
Some components' CSS reached into a different component's rendered markup by HTML tag (.container input, .root button, > span) — exactly the fragile pattern data-slot exists to replace, per components.build/data-attributes. Swapped each for the actual data-slot the nested element carries, verified empirically per case (a Base UI render-prop composition like Toolbar.Button wrapping Button isn't always obvious which slot wins without checking): - search: .container input -> [data-slot="search-input"] - filter-chip: .chip/.inputFieldWrapper/.dateField input -> [data-slot="input"] / [data-slot="date-picker-input"] (the chip-level reset needs both, since it resets any input variant inside the chip) - data-view: .display-popover-properties-select > span -> [data-slot="select-value"] - toolbar: .root[data-orientation="vertical"] button -> [data-slot="toolbar-button"] - calendar: .dropdowns > span -> [data-slot="select-value"] data-table's equivalent selector is left as-is — that component is excluded from the data-slot rollout since it's being deprecated. Left in place: selectors that reach into a component's own directly- rendered icon/pseudo-element (own markup, not cross-component), and code-block's :global(.token...) rules (targeting Prism.js's own public class API, not something we control).
…ecate redundant classNames props Follow-up from auditing #846 against the data-slot rollout: every component's classNames-style hook props were checked for redundancy now that data-slot covers the same ground, and three concrete gaps were closed. New: data-column - DataView.List's header cells, body cells, and loader cells now carry data-column={accessorKey} alongside their data-slot, so one column can be targeted without a per-column classNames entry: [data-slot="data-view-list-cell"][data-column="amount"]. Fixed: DataView.Filters classNames.addFilter - Declared in the type since the classNames API existed, but never applied anywhere in filters.tsx — setting it had no effect. Wired it onto the default trigger (Button/IconButton), same as classNames.filterChips already applies to FilterChip. Deprecated (JSDoc @deprecated, non-breaking — removal is a future major version): every classNames key with an exact 1:1 data-slot match — DataView.List, DataView.Timeline, DataView.Filters (filterChips/addFilter), Indicator, Input, EmptyState, Table.SectionHeader, Sidebar.NavigationGroup, Sidebar.Item, and Sidebar.More's root/leadingIcon/text (not menuContent — Menu.Content portals to document.body, so a data-slot selector can't be scoped to one specific instance; that key stays as the only way to target it). DataViewListColumn's per-column classNames is deprecated too, pointing at the new data-column combo, since data-slot alone can't distinguish one column from another. Also closed two pre-existing doc gaps found while cross-referencing props against their mirrored apps/www props.ts: Table never had a Slots section, and DataView.List/Timeline/Sidebar.NavigationGroup's classNames types were undocumented or oversimplified there.
ravisuhag
force-pushed
the
feat/data-slot-attributes
branch
from
July 31, 2026 16:40
56d14b7 to
bbcdb35
Compare
…Sidebar.Trigger Rebase fallout: the sidebar refactor (#867) replaced the boolean collapsible prop with 'icon' | 'hidden' | 'none', so collapsible={false} no longer disables the toggle (canCollapse checks !== 'none'). Switched the test to collapsible='none' and added coverage for the new Sidebar.Trigger's data-slot.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Every element every component renders now carries a stable
data-slotattribute — a kebab-case, component-prefixed identifier (filter-chip-remove,data-view-list-loader-row,dialog-close). This gives consumers a supported hook for styling and testing inner parts without targeting hashed CSS-module classes.Started as a 3-component pilot (Search, FilterChip, DataView) and grew into full coverage across the design system per follow-up discussion.
Related: #846 (closes its styling half — filter-chip internals and loader rows are now reachable). The prop-forwarding half of #848 is separate and unaffected.
Coverage
76 of 77 components get slots.
data-tableis deliberately excluded — it's slated for deprecation in favor ofDataView, so it doesn't get new public API surface.Every component ships with:
data-sloton every element it renders, placed before any{...props}spread so a caller-supplied override always wins__tests__/data-slots.test.tsxasserting the slot contract via the sharedexpectSlots/getSlot/getAllSlotshelper (packages/raystack/test-utils/data-slots.ts), including that conditional slots are absent when their part isn't rendered### Slotssection on its docs page listing slot namesDataView.Listcells also carrydata-column={accessorKey}, so a single column can be targeted without a per-columnclassNamesentry:[data-slot="data-view-list-cell"][data-column="amount"].Convention
Documented in the styling guide (With data-slot, including the
has-[]/[&_]composition patterns) and enforced going forward by theadd-new-componentskill checklist:filter-chip-remove,data-view-list-rowPopover.Content/Menu.Content/Dialog.Popup-style wrappers that split props between a Base UI Positioner and Popup get the slot hardcoded directly on the Popup — not routed through forwarded props, since those land on the positioner insteadmenu-trigger'sdata-slot='menu-subtrigger') is preserved unchanged.container input,.root button) is exactly the fragile patterndata-slotreplaces — the 7 places this existed in-scope are now[data-slot=...]selectors insteadclassNames cleanup (closing the loop on #846)
#846 asked to grow
classNameswith more keys (a filter-chip object, a loader key). Instead we audited everyclassNames-style prop in the design system against the now-completedata-slotcoverage:@deprecatedJSDoc, non-breaking — actual removal is a future major version) every key with an exact 1:1data-slotmatch:DataView.List,DataView.Timeline,DataView.Filters(filterChips/addFilter),Indicator,Input,EmptyState,Table.SectionHeader,Sidebar.NavigationGroup,Sidebar.Item, andSidebar.More'sroot/leadingIcon/text.Sidebar.More.classNames.menuContent—Menu.Contentportals todocument.body, so adata-slotselector can't be scoped to one specific dropdown instance; this prop remains the only way to target it. Same reasoning applies to any other portaled per-instance content.DataView.Filters.classNames.addFilterwas declared in the type but never applied anywhere — setting it silently did nothing. Wired it onto the default trigger.Tablehad no### Slotssection at all, andDataView.List/Timeline/Sidebar.NavigationGroup'sclassNamestypes were undocumented or oversimplified in the docs-site mirror (apps/www/.../props.ts).Fixes made while integrating
A few real issues surfaced while wiring this up across the whole system, fixed as part of this PR:
DatePicker's error span unmounts when there's no error — it always mounts and toggles viadata-visible, so screen readers hear it the instant an error appears. Fixed the test, not the component.Chat.JumpButtonoutside its requiredChat.Messagesancestor. Fixed the test harness.mergePropstypes its parameter againstHTMLAttributes, which doesn't includedata-slot— passing it as a fresh object literal fails TypeScript's excess-property check. Fixed by extracting the props into a namedconstfirst (matching the existing pattern already used inflex.tsx), which sidesteps the check without a cast.Testing
data-table/menu/popovertest files, none introduced by this change)@raystack/apsararollup build: passeswwwdocs site build: passes