fix(ui): collapse Icon line-box to fix baseline misalignment#1789
Merged
Conversation
…n alignment The Icon component wrapped its SVG in containers that inherited the surrounding line-height, leaving descender space below the SVG and shifting icons above the visual centerline in flex layouts. This fix collapses every container in the icon containment chain to its content (via `inline-flex` + `leading-none`) so `align-items: center` centers the actual icon, not a line-height-padded box. Follow-up fixes in components that visibly relied on the prior behavior: - PopupMenu: collapse outer wrapper, default toggle, and base toggle styles so default and custom toggles both align correctly. - ComboBox: switch toggle button and validation/loading icon containers to explicit flex centering and `top-1/2 + translate-y-[-50%]` so positioning is independent of font-size, line-height, and icon size. - Select: same treatment for chevron span and centered (error/loading) icon container. Also fixed an unrelated incomplete utility class `jn:h-` in `PopupMenuSectionSeparator` (now `jn:h-px`, matching Divider). Added a PageHeader story `WithThemeToggleAndUserMenu` as a visual regression target for the alignment fix. Refs #1788 Signed-off-by: Esther Schmitz <esther.schmitz@sap.com>
🦋 Changeset detectedLatest commit: c5edd23 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 addresses baseline/line-box-driven icon misalignment in flex layouts by collapsing icon wrappers (and a few consuming components) to the SVG/content box using Tailwind utilities like inline-flex and leading-none, plus follow-up centering fixes where prior alignment depended on inherited line-height.
Changes:
- Collapse
Iconwrapper line-boxes (plain/anchor/button variants) so SVGs don’t carry descender space. - Update
PopupMenu,ComboBox, andSelectto use flex-based centering and remove line-height-dependent positioning. - Add a
PageHeaderStorybook story as a visual regression target; add a changeset for the patch release.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-components/src/components/Icon/Icon.component.tsx | Collapses icon wrappers with inline-flex + leading-none to eliminate descender-space baseline issues. |
| packages/ui-components/src/components/PopupMenu/PopupMenu.component.tsx | Applies shared toggle alignment styles and updates wrapper display; fixes separator height utility. |
| packages/ui-components/src/components/ComboBox/ComboBox.component.tsx | Centers toggle/validation icons via flex + top-1/2/translate rather than magic offsets. |
| packages/ui-components/src/components/Select/Select.component.tsx | Aligns chevron/validation icons and collapses centered icon wrapper line-box. |
| packages/ui-components/src/components/PageHeader/PageHeader.stories.tsx | Adds a story to visually verify icon alignment in a centered flex row. |
| .changeset/icon-baseline-alignment.md | Publishes a patch changeset for the UI components package. |
- PopupMenuToggle: replace `${disabled && ...}` with ternary to avoid
emitting a literal "false" class on enabled toggles
- Icon plain-icon path: drop ref from span wrapper so forwardRef type
(HTMLAnchorElement | HTMLButtonElement) remains accurate; the plain
icon path never forwarded a ref before this PR
- Reformat plain-icon return with prettier
Signed-off-by: Esther Schmitz <esther.schmitz@sap.com>
taymoor89
approved these changes
Jun 23, 2026
vlad-schur-external-sap
approved these changes
Jun 23, 2026
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.
Summary
Fixes a baseline-alignment bug where
Iconrendered with descender space below the SVG, shifting icons above the visual centerline of flex layouts. Collapses the icon's containment chain to its content viainline-flex+leading-none, and applies the same treatment toPopupMenu,ComboBox, andSelect— components that visibly relied on the prior (coincidental) alignment.Changes Made
Icon.component.tsx): plain-icon wrapper<span>,buttonIconStyles, andanchorIconStylesnow useinline-flex+leading-noneso the wrapper hugs the SVG.PopupMenu.component.tsx):HeadlessMenuwrapper switched toinline-flex.inline-flex items-center leading-none.<PopupMenu.Toggle>get the same alignment via sharedbaseToggleStylesapplied inPopupMenuToggle.jn:h-→jn:h-pxonPopupMenuSectionSeparator(matchesDivider/SelectDividerconvention; pre-existing typo).ComboBox.component.tsx):inline-flex items-center justify-end leading-noneso the chevron is centered vertically inside theh-textinputtoggle.top-[.4rem]withtop-1/2 + translate-y-[-50%]so positioning is independent of icon height / line-height.centeredIconStyles(error/loading icon): addedinline-flex leading-noneso the translate-50% centering operates on the SVG box.Select.component.tsx):centeredIconStyles: same treatment as ComboBox.items-center; bare wrapper span around the chevron now usesinline-flex leading-none.PageHeader.stories.tsx): newWithThemeToggleAndUserMenustory as a visual-regression target for icon alignment in a flex row.Related Issues
Screenshots (if applicable)
Before:
After:
Testing Instructions
pnpm ipnpm --filter @cloudoperators/juno-ui-components test— confirm Icon, PopupMenu, ComboBox, and Select tests pass.cd packages/ui-components && pnpm storybook— then visually inspect:Layout/PageHeader → With Theme Toggle And User Menu(new story; both icons should be centered in the flex row).Components/PopupMenustories: default toggle, custom toggles, and items with icons.Forms/ComboBoxstories: default, valid, invalid, loading, error states.Forms/Selectstories: default, valid, invalid, loading, error states.Components/Iconbutton and anchor variants.Checklist
PR Manifesto
Review the PR Manifesto for best practises.