feat(react-icons): expose the bundled icon variant as a data attribute - #1228
Open
Ray Knight (ArrayKnight) wants to merge 3 commits into
Open
feat(react-icons): expose the bundled icon variant as a data attribute#1228Ray Knight (ArrayKnight) wants to merge 3 commits into
Ray Knight (ArrayKnight) wants to merge 3 commits into
Conversation
`bundleIcon` renders both glyphs of a pair and hides one with `data-fui-icon-hidden`. A consumer stylesheet that wants to style the two variants differently — or select the visible one — currently has to key off the `fui-Icon-filled` / `fui-Icon-regular` class names, which are a rendering detail rather than a contract. This stamps `data-fui-icon-variant="filled" | "regular"` on the glyphs `bundleIcon` renders, exported as `DATA_FUI_ICON_VARIANT` alongside the existing `DATA_FUI_ICON`, `DATA_FUI_ICON_RTL`, `DATA_FUI_ICON_HIDDEN` and `DATA_FUI_ICON_FONT`. It is additive: no existing attribute, class name or export changes, and nothing renders differently. The headless entrypoint's test for bundled-variant data attributes is extended to cover it, and both copies of the attribute documentation (`docs/headless.md` and the docsite mirror) are updated.
… layered `[data-fui-icon-hidden]` is the one rule in this file consumers routinely have to override — a hover or selection glyph swap reveals the inactive variant. Every other non-obvious rule here already carries a rationale block; this one did not, so the constraint it places on consumers (import this file BELOW your component layers, then a normal declaration wins) was only discoverable by experiment. Comment only; the declaration is unchanged.
…annot tear a file `finalizeCjs` collected rewrite targets into an array. A stale pre-renamed file and its fresh source can resolve to the same target path, so the same file was queued twice and `forEachConcurrent` rewrote it from two workers at once, tearing it. Collecting into a `Set` makes the target list unique. Unrelated to the variant attribute in the preceding commits; happy to split this into its own PR if preferred.
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
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.
bundleIconrenders both glyphs of a pair and hides one withdata-fui-icon-hidden. A consumerstylesheet that wants to style the two variants differently — or select the visible one — currently
has to key off the
fui-Icon-filled/fui-Icon-regularclass names, which are a rendering detailrather than a contract.
This adds
data-fui-icon-variant="filled" | "regular"to the glyphsbundleIconrenders, exportedas
DATA_FUI_ICON_VARIANTalongside the existingDATA_FUI_ICON,DATA_FUI_ICON_RTL,DATA_FUI_ICON_HIDDENandDATA_FUI_ICON_FONT. It is additive: no existing attribute, class nameor export changes, and nothing renders differently.
The headless entrypoint's bundled-variant test coverage is extended to assert the attribute on both
glyphs in both
filledstates, and both copies of the attribute documentation(
docs/headless.mdand the docsite mirror) are updated.Context. This comes out of building a complete Tailwind v4 + CSS Modules styling layer over
@fluentui/react-headless-components-previewinmicrosoft/fluentui— see microsoft/fluentui#36656. Thatlayer styles icons entirely from CSS, and the class-name route was the one place it had to depend on
a string rather than a contract. It is useful independently of that work: any consumer styling
headless icons hits the same wall.
fix(react-icons): dedupe CJS rewrite targets so concurrent rewrites cannot tear a file(separate commit, unrelated to the above — happy to split into its own PR if preferred)
finalizeCjscollected rewrite targets into an array. A stale pre-renamed file and its fresh sourcecan resolve to the same target path, so the same file was rewritten twice concurrently and torn.
Collecting into a
Setmakes the target list unique.docs(react-icons): explain how the hidden-variant rule is meant to be layered(comment-only commit; drop it if you would rather not carry it)
[data-fui-icon-hidden]is the one rule inheadless/styles.cssconsumers routinely override — anyhover or selection glyph swap reveals the inactive variant. Every other non-obvious rule in that
file carries a rationale block; this one did not, so the constraint it places on a consumer (import
the stylesheet below your component layers, then a normal declaration wins) was only discoverable by
experiment. The declaration itself is untouched — it matches
upstream/mainand published npm(
2.0.339) byte for byte.Why a
data-*attribute rather than the class namesSelecting on
fui-Icon-filledcouples a consumer's stylesheet to a string the package is free tochange. The
DATA_FUI_ICON_*family is the surface the headless entrypoint already treats as itspublic selector contract —
data-fui-icon,data-fui-icon-rtl,data-fui-icon-hidden,data-fui-icon-font. The variant is the one gap in that family, and it is the fact a CSS-firstconsumer reaches for most, because a
bundleIconpair is precisely the case where "which glyph isthis?" has to be answerable from CSS alone.
On the name. The attribute is namespaced to match every sibling constant:
data-fui-icon-variant/DATA_FUI_ICON_VARIANT. A baredata-varianton every bundled glyphwould collide with consumer and design-system attributes, so the short spelling was ruled out before
this PR rather than argued for in it.
What changed, file by file
featcommit —packages/react-icons/src/headless/{shared.ts,index.ts}andbundleIcon. Addsexport const DATA_FUI_ICON_VARIANT = 'data-fui-icon-variant', re-exports it, and spreads{ [DATA_FUI_ICON_VARIANT]: 'filled' }/'regular'onto the two glyphsbundleIconrenders.Test.
headless.test.tsxgainsbundleIcon stamps data-fui-icon-variant on both glyphs regardless of filled, asserting both glyphsin both
filledstates. The pre-existing "bundleIcon renders both variants with data attributes"test (which asserts only
DATA_FUI_ICON_HIDDEN) is left as-is. Suite: 24 passed, was 23.Docs. Both copies updated —
packages/react-icons/docs/headless.mdand its mirrorpackages/docsite/stories/Icons/Headless/IconsHeadless.md. Each gains the constant in the exportlist and a
bundleIcon variantrow in the "How it works" table naming thefui-Icon-filled/fui-Icon-regularclass names as the Griffel-side equivalent. Prettier reflowed the table's secondcolumn as a result; that column-width churn is the only incidental diff in either file.
One caveat for the reviewer, pre-existing and left alone. The docsite mirror tells readers to
import these constants from
@fluentui/react-icons/headless/utils, butsrc/headless/utils.tsre-exports only the class-name constants, never the
DATA_FUI_ICON_*ones — they come from the./headlessbarrel. That inaccuracy predates this change; worth fixing separately.fixcommit —packages/react-icons/scripts/module-format.js, infinalizeCjs:toRewritechanges from a
string[]with.push()to aSet<string>with.add(), spread at theforEachConcurrentcall site. Genuinely independent of everything else here — it has nothing to dowith Tailwind, styling, or the attribute above.
Relationship to the companion
microsoft/fluentuiPRThe dependency is real and narrow. The styling layer imports
@fluentui/react-icons/headlessin106 files, but that entrypoint is not what it needs this PR for — the whole
src/headless/tree and its
./headless*export conditions already exist here.What it needs is the
data-fui-icon-variantstamp specifically. It publishes two@custom-variantdefinitions against the attribute in a documented public export of its own package, and every
component that swaps a glyph —
Button,ToggleButton,MenuButton,MenuItem,BreadcrumbButton,Tab— reaches the attribute only through those two variants. Without thischange, those rules match nothing, silently.
So the linkage is: this PR opens first; the companion PR keeps a pinned resolution until this one
merges and publishes, then moves to the published version and drops it. Cross-links in both
directions.
Changelog. No change file: this repo uses neither beachball nor changesets. It runs
@commitlint/config-conventionalwithnx releasedriven by conventional commits, andpackages/react-icons/CHANGELOG.mdis machine-generated with PR backlinks. All three commitsubjects are valid conventional commits.