Skip to content

Commit

Permalink
feat(Breadcrumb): implement new defined styles and swap out Button wi…
Browse files Browse the repository at this point in the history
…th Anchor for the DNB UI

This change is based on this [UX RFC](https://dnb-it.slack.com/archives/C014NMBQ5SQ/p1704372436321949)

- align the gap between items
- swap out Button with Anchor for DNB UI
- ensure bounding area has still the same height (Anchor vs Button)
- refactor dynamic type and fix missing themeName in visual test
  • Loading branch information
tujoworker committed Mar 6, 2024
1 parent d543c58 commit 8956edc
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 40 deletions.
64 changes: 43 additions & 21 deletions packages/dnb-eufemia/src/components/breadcrumb/BreadcrumbItem.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import React from 'react'

// Components
import Button, { ButtonProps } from '../button/Button'
import Button, { ButtonProps } from '../Button'
import Anchor, { AnchorAllProps } from '../Anchor'
import IconPrimary from '../icon-primary/IconPrimary'
import type { DataAttributeTypes } from '../../shared/types'
import type { IconIcon } from '../icon/Icon'

// Elements
Expand All @@ -12,7 +14,7 @@ import P from '../../elements/P'
import homeIcon from '../../icons/home'

// Shared
import { useTheme, useMediaQuery } from '../../shared'
import { useMediaQuery } from '../../shared'
import Context from '../../shared/Context'
import type { SkeletonShow } from '../skeleton/Skeleton'
import {
Expand All @@ -33,7 +35,7 @@ export type BreadcrumbItemProps = {

/**
* Icon displaying on the left side
* Default: HomeIcon / chevron_left
* Default: HomeIcon / chevron_right
*/
icon?: IconIcon

Expand All @@ -47,7 +49,7 @@ export type BreadcrumbItemProps = {
* Set a custom click event. In this case, you should not define the prop href.
* Default: null
*/
onClick?: React.MouseEventHandler<HTMLButtonElement>
onClick?: React.MouseEventHandler<HTMLAnchorElement | HTMLButtonElement>

/**
* The component variant. Variant 'current' should correspond to the current page and 'home' to the root page.
Expand All @@ -63,7 +65,8 @@ export type BreadcrumbItemProps = {

/** Internal */
itemNr?: number
} & Omit<ButtonProps, 'variant'>
} & (AnchorAllProps & Omit<ButtonProps, 'variant'>) &
DataAttributeTypes

const defaultProps = {
text: null,
Expand All @@ -74,7 +77,7 @@ const defaultProps = {
skeleton: null,
}

const determineSbankenIcon: IconIcon = (
const determineIcon: IconIcon = (
variant: string,
isSmallScreen: boolean
) => {
Expand All @@ -93,6 +96,7 @@ const BreadcrumbItem = (localProps: BreadcrumbItemProps) => {
// Every component should have a context
const context = React.useContext(Context)
const {
theme,
translation: {
Breadcrumb: { homeText },
},
Expand All @@ -114,7 +118,6 @@ const BreadcrumbItem = (localProps: BreadcrumbItemProps) => {
context?.BreadcrumbItem
)

const theme = useTheme()
const isSmallScreen = useMediaQuery({
matchOnSSR: true,
when: { max: 'medium' },
Expand All @@ -124,15 +127,14 @@ const BreadcrumbItem = (localProps: BreadcrumbItemProps) => {
React.useState<IconIcon>('chevron_left')

useLayoutEffect(() => {
if (!icon && theme?.name === 'sbanken') {
const icon = determineSbankenIcon(variant, isSmallScreen)
setCurrentIcon(icon)
if (!icon) {
setCurrentIcon(determineIcon(variant, isSmallScreen))
} else {
if (variant !== 'home') {
setCurrentIcon(icon ?? 'chevron_left')
}
}
}, [icon, isSmallScreen, theme?.name, variant])
}, [icon, isSmallScreen, variant])

const currentText = text || (variant === 'home' && homeText) || ''
const isInteractive =
Expand All @@ -151,16 +153,36 @@ const BreadcrumbItem = (localProps: BreadcrumbItemProps) => {
style={style}
>
{isInteractive ? (
<Button
variant="tertiary"
href={href}
icon={iconToUse}
icon_position="left"
on_click={onClick}
text={currentText}
skeleton={skeleton}
{...props}
/>
theme?.name === 'sbanken' ? (
<Button
variant="tertiary"
href={href}
icon={iconToUse}
icon_position="left"
on_click={onClick}
text={currentText}
skeleton={skeleton}
{...props}
/>
) : (
<>
{variant !== 'home' && (
<IconPrimary
icon={iconToUse}
className="dnb-breadcrumb__item__span__icon"
/>
)}
<Anchor
href={href}
onClick={onClick}
icon={variant === 'home' ? iconToUse : null}
skeleton={skeleton}
{...props}
>
{currentText}
</Anchor>
</>
)
) : (
<span
className="dnb-breadcrumb__item__span"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ describe.each(['ui', 'sbanken'])('Breadcrumb for %s', (themeName) => {
describe('on small screen', () => {
setupPageScreenshot({
url: '/uilib/components/breadcrumb/demos',
themeName,
pageViewport: {
width: 700,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Breadcrumb', () => {
)

expect(screen.queryByTestId(dataTestId)).toBeInTheDocument()
expect(screen.queryByTestId(dataTestId)).toHaveClass('dnb-button')
expect(screen.queryByTestId(dataTestId)).toHaveClass('dnb-anchor')
})

// TODO – can be removed in v11 when we deprecate passing down props to dnb-breadcrumb__item__span
Expand All @@ -77,7 +77,7 @@ describe('Breadcrumb', () => {
data={[
{
text: 'Page 2',
role: 'button',
'aria-label': 'Label',
'data-testid': dataTestId,
},
]}
Expand All @@ -90,7 +90,7 @@ describe('Breadcrumb', () => {
)
expect(
document.querySelector('.dnb-breadcrumb__item__span')
).not.toHaveAttribute('role')
).toHaveAttribute('aria-label', 'Label')
})

it('renders a breadcrumb with multiple items by children', () => {
Expand Down Expand Up @@ -185,7 +185,7 @@ describe('Breadcrumb', () => {
document.querySelector('.dnb-breadcrumb__multiple')
).not.toBeInTheDocument()

fireEvent.click(screen.getByRole('button'))
fireEvent.click(document.querySelector('button'))

expect(
document.querySelector('.dnb-breadcrumb__multiple')
Expand Down Expand Up @@ -238,7 +238,7 @@ describe('Breadcrumb', () => {
/>
)

fireEvent.click(screen.getByRole('button'))
fireEvent.click(document.querySelector('a'))

expect(
document.querySelector('.dnb-breadcrumb__multiple')
Expand Down Expand Up @@ -383,7 +383,7 @@ describe('Breadcrumb', () => {
const onClick = jest.fn()
render(<BreadcrumbItem onClick={onClick} text="Page" />)

fireEvent.click(screen.getByRole('button'))
fireEvent.click(document.querySelector('a'))
expect(onClick).toHaveBeenCalledTimes(1)
})

Expand All @@ -409,7 +409,7 @@ describe('Breadcrumb', () => {
<BreadcrumbItem skeleton onClick={jest.fn()} text="skeleton" />
)

expect(screen.getByRole('button')).toHaveClass('dnb-skeleton')
expect(document.querySelector('a')).toHaveClass('dnb-skeleton')
})

it('inherits skeleton prop from provider', () => {
Expand All @@ -419,18 +419,18 @@ describe('Breadcrumb', () => {
</Provider>
)

expect(screen.getByRole('button')).toHaveClass('dnb-skeleton')
expect(document.querySelector('a')).toHaveClass('dnb-skeleton')
})

it('forwards rest props like data-testid, etc, to the breadcrumb item button when interactive', () => {
it('forwards rest props like data-testid, etc, to the breadcrumb item anchor when interactive', () => {
const dataTestId = 'my-test-id'
render(
<BreadcrumbItem href="/" text="Home" data-testid={dataTestId} />
)

expect(screen.queryByTestId(dataTestId)).toBeInTheDocument()

expect(screen.queryByTestId(dataTestId)).toHaveClass('dnb-button')
expect(screen.queryByTestId(dataTestId)).toHaveClass('dnb-anchor')
})

// TODO – can be removed in v11 when we deprecate passing down props to dnb-breadcrumb__item__span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('BreadcrumbItem', () => {
expect(document.querySelector('img')).toBeDefined()
expect(document.querySelector('.dnb-icon')).toHaveAttribute(
'data-testid',
'chevron left icon'
'chevron right icon'
)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -736,17 +736,39 @@ button.dnb-button::-moz-focus-inner {
list-style: none;
flex-flow: row wrap;
align-items: center;
gap: 1.5rem;
row-gap: 1rem;
column-gap: 0.5rem;
}
.dnb-breadcrumb__item__span {
.dnb-breadcrumb__item {
display: flex;
align-items: center;
line-height: normal;
padding: 0.5rem 0;
}
.dnb-breadcrumb__item__span {
display: flex;
align-items: center;
line-height: var(--line-height-basis);
}
.dnb-breadcrumb__item__span__icon {
margin-right: 0.5rem;
}
.dnb-breadcrumb__item .dnb-anchor {
line-height: 1rem;
position: relative;
}
.dnb-breadcrumb__item .dnb-anchor::after {
content: "";
position: absolute;
inset: 0;
transform: scale(1.25, 2);
border: 0.5rem solid transparent;
}
.dnb-breadcrumb__item:first-child .dnb-anchor {
border-left: none;
}
.dnb-breadcrumb__item:first-child .dnb-anchor .dnb-icon {
margin-right: 0.5rem;
}
.dnb-breadcrumb__multiple {
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -779,8 +801,9 @@ html[data-visual-test] .dnb-breadcrumb__multiple .dnb-breadcrumb__item {
.dnb-breadcrumb__collapse .dnb-breadcrumb__list.dnb-section {
flex-direction: column;
align-items: flex-start;
row-gap: 0;
margin: 0.5rem 0;
margin-left: 1.5rem;
gap: 0;
padding: 0;
}"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,47 @@

flex-flow: row wrap;
align-items: center;
gap: 1.5rem;
row-gap: 1rem; // For when breaking into new lines (Anchor variant)
column-gap: 0.5rem;
}

&__item {
display: flex;
align-items: center;
padding: 0.5rem 0;

&__span {
display: flex;
align-items: center;
line-height: normal;
padding: 0.5rem 0;
line-height: var(--line-height-basis);

// To match the other tertiary buttons
&__icon {
margin-right: 0.5rem;
}
}

.dnb-anchor {
line-height: 1rem;

// To make the click area bigger
position: relative;
&::after {
content: '';
position: absolute;
inset: 0;
transform: scale(1.25, 2);
border: 0.5rem solid transparent;
}
}

// Home icon spacing
&:first-child .dnb-anchor {
border-left: none;
.dnb-icon {
margin-right: 0.5rem;
}
}
}

&__multiple {
Expand Down Expand Up @@ -87,9 +113,10 @@
&__collapse &__list.dnb-section {
flex-direction: column;
align-items: flex-start;
row-gap: 0;

margin: 0.5rem 0;
margin-left: 1.5rem;
gap: 0;
padding: 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
}
}
&__item {
padding: 0;
&__span {
padding: 0.5rem 0;
&__icon {
color: var(--sb-color-violet);
}
Expand All @@ -30,4 +32,9 @@
}
}
}

// For when breaking into new lines
&__list.dnb-section {
row-gap: 0;
}
}

0 comments on commit 8956edc

Please sign in to comment.