CL-6470: Fix BoldIconProvider dropping the size:1em default - #191
Merged
Merged
Conversation
IconContext.Provider replaces the whole context value rather than
merging with it, so {weight:"bold"} alone silently un-sized every
glyph with no ancestor CSS rule and no explicit size= prop — the
browser's fallback for an unsized replaced-element <svg> is 300x150,
which is why the right-click context menu and the search bar rendered
wildly oversized.
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
BoldIconProvider(packages/icons/src/index.tsx) was passingIconContext.Provider value={{ weight: "bold" }}— sinceIconContext.Providerreplaces the whole context value rather than merging with it, this silently dropped Phosphor's ownsize: "1em"default for every icon mounted under it.Buttonhas a[&_svg]:size-4rule; react-ui'sMenuItemdoes not) and no explicitsize=prop rendered as a bare<svg>with no width/height — the browser's fallback for an unsized replaced element is 300x150. That is exactly why the right-click context menu and the search bar rendered wildly oversized.boldIconContextValuenow restatessize: "1em"alongsideweight: "bold", and is exported so the fix is directly testable.Context
Part of CL-6470 (design tokens from the mock). The named icon-size scale itself (nav/top-bar/button/checkbox) is a separate PR against
corbitsdev/react-ui: corbitsdev/react-ui#36 — that PR is the token source of truth; this one is the upstream bug that made every unsized icon balloon in the first place.Sweeping the ~14 remaining hand-declared
size=call sites onto the new react-ui tokens is a follow-up unit, not done here (scope kept to the root-cause fix + the token PR, per current timebox policy).Test plan
bun test packages/icons/src/index.test.tsx— new regression test, red before the fix, green afterbun run buildinapps/web— not run in this pass; not covered by this scoped checkNot merged — pushed for peer review per current policy.