Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 ref={ref} className="jn:inline-flex jn:leading-none">{icn}</span>
Comment thread
edda marked this conversation as resolved.
Outdated
})

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