Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/icon-baseline-alignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@cloudoperators/juno-ui-components": patch
---

fix(ui): collapse Icon line-box so icons align reliably inside flex containers, with follow-up fixes in PopupMenu, ComboBox, and Select
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ const buttonStyles = `
jn:pr-2
jn:h-textinput
jn:w-8
jn:inline-flex
jn:items-center
jn:justify-end
jn:leading-none
jn:border-l-0
jn:border-y
jn:border-r
Expand Down Expand Up @@ -144,14 +148,19 @@ const menuStyles = `

const iconContainerStyles = `
jn:absolute
jn:top-[.4rem]
jn:top-1/2
jn:right-8
jn:inline-flex
jn:leading-none
jn:translate-y-[-50%]
`

const centeredIconStyles = `
jn:absolute
jn:top-1/2
jn:left-1/2
jn:inline-flex
jn:leading-none
jn:translate-y-[-50%]
jn:translate-x-[-0.75rem]
`
Expand Down
10 changes: 7 additions & 3 deletions packages/ui-components/src/components/Icon/Icon.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,10 @@ Generic Icon component.
// hover style needs to be revisited. only works if no icon color was passed
const anchorIconStyles = `
jn:text-current
jn:inline-flex
jn:leading-none
jn:hover:text-theme-high
jn:focus:outline-hidden
jn:focus:outline-hidden
jn:focus-visible:ring-2
jn:focus-visible:ring-theme-focus
jn:focus-visible:ring-offset-1
Expand All @@ -83,8 +85,10 @@ const anchorIconStyles = `

// hover style needs to be revisited. only works if no icon color was passed
const buttonIconStyles = `
jn:inline-flex
jn:leading-none
jn:hover:text-theme-high
jn:focus:outline-hidden
jn:focus:outline-hidden
jn:focus-visible:ring-2
jn:focus-visible:ring-theme-focus
jn:focus-visible:ring-offset-1
Expand Down Expand Up @@ -998,7 +1002,7 @@ export const Icon = forwardRef<HTMLAnchorElement | HTMLButtonElement, IconProps>

/* render an <a> if href was passed, otherwise render button if onClick was passes, otherwise render plain icon: */
/* if plain icon, add ref to the icon. In the other cases the ref goes on the anchor or button */
return href ? anchor : onClick ? button : <span ref={ref}>{icn}</span>
return href ? anchor : onClick ? button : <span className="jn:inline-flex jn:leading-none">{icn}</span>
})

export interface IconProps extends Omit<HTMLProps<HTMLAnchorElement> | HTMLProps<HTMLButtonElement>, "size"> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import CustomLogoPortraitPNG from "./custom-logo-placeholders/custom-logo-portra
import CustomLogoSquarePNG from "./custom-logo-placeholders/custom-logo-square.png"
import { PageHeader } from "./index"
import { Button } from "../Button/"
import { ThemeToggle } from "../ThemeToggle/"
import { PopupMenu, PopupMenuOptions, PopupMenuItem } from "../PopupMenu/"

type Story = StoryObj<typeof meta>

Expand Down Expand Up @@ -187,3 +189,29 @@ export const WithWrappedLogo: Story = {
),
},
}

export const WithThemeToggleAndUserMenu: Story = {
parameters: {
docs: {
description: {
story:
"Visual test for icon alignment: a `ThemeToggle` next to a `PopupMenu` (default icon toggle using `accountCircle`) inside a flex container with `align-items: center`. Both icons should appear vertically centered with no extra space below the user-menu icon.",
},
},
},
args: {
applicationName: "My Awesome App",
children: (
<div className="jn:flex jn:items-center jn:gap-2">
<ThemeToggle />
<PopupMenu icon="accountCircle">
<PopupMenuOptions>
<PopupMenuItem label="Profile" icon="manageAccounts" />
<PopupMenuItem label="Settings" icon="edit" />
<PopupMenuItem label="Log Out" icon="exitToApp" />
</PopupMenuOptions>
</PopupMenu>
</div>
),
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ import { PortalProvider } from "../PortalProvider/"

// ----- Styles -----

// Base styles applied to every toggle (default and custom).
// inline-flex + leading-none collapse the button's line box to its content,
// so an Icon child sits without descender space below it.
const baseToggleStyles = `
jn:inline-flex
jn:items-center
jn:leading-none
`

const defaultToggleStyles = `
jn:hover:text-theme-accent
jn:active:text-theme-accent
Expand Down Expand Up @@ -107,7 +116,7 @@ const sectionTitleStyles = `
`

const sectionSeparatorStyles = `
jn:h-
jn:h-px
jn:bg-theme-background-lvl-3
`

Expand All @@ -116,6 +125,13 @@ const floatingReferenceWrapperStyles = `
jn:inline-flex
`

// The outer wrapper must also be a flex container (rather than a default block element),
// otherwise its inline-flex child sits in an anonymous line box and inherits line-height,
// reintroducing descender space below the toggle.
const popupMenuWrapperStyles = `
jn:inline-flex
`

// ----- Interfaces -----

export interface PopupMenuContextType {
Expand Down Expand Up @@ -256,7 +272,7 @@ const PopupMenu = ({
const menu = childrenArray.find((child) => isValidElement(child) && child.type === PopupMenuOptions)

return (
<HeadlessMenu as="div" className={`juno-popupmenu ${className}`} {...props}>
<HeadlessMenu as="div" className={`juno-popupmenu ${popupMenuWrapperStyles} ${className}`} {...props}>
{/* Update our open state when the open render prop from headless ui menu changes, so we can run the handlers when our tracking state changes: */}
{({ open, close }) => {
// Set our tracking open state outside of rendering cycle. Trying to set the state during render will cause an error:
Expand Down Expand Up @@ -322,7 +338,7 @@ export const PopupMenuToggle = ({
}: PopupMenuToggleProps): ReactNode => (
<MenuButton
as={as}
className={`juno-popupmenu-toggle ${disabled && disabledToggleStyles} ${className}`}
className={`juno-popupmenu-toggle ${baseToggleStyles} ${disabled ? disabledToggleStyles : ""} ${className}`}
disabled={disabled}
Comment thread
edda marked this conversation as resolved.
{...props}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ const centeredIconStyles = `
jn:absolute
jn:top-1/2
jn:left-1/2
jn:inline-flex
jn:leading-none
jn:translate-y-[-50%]
jn:translate-x-[-0.75rem]
`
Expand Down Expand Up @@ -395,10 +397,10 @@ export const Select = ({
>
{getDisplayValue(value)}
</span>
<span className="jn:flex">
<span className="jn:flex jn:items-center">
{isValid ? <Icon icon="checkCircle" color="jn:text-theme-success" /> : ""}
{isInvalid ? <Icon icon="dangerous" color="jn:text-theme-error" /> : ""}
<span>
<span className="jn:inline-flex jn:leading-none">
<Icon icon={open ? "expandLess" : "expandMore"} />
</span>
</span>
Expand Down
Loading