Skip to content

Commit

Permalink
feat!: Replace ‘on background’ props with ‘inverseColor’ and ‘contras…
Browse files Browse the repository at this point in the history
…tColor’ for Link, Link List Link, and Icon Button (#1448)

Co-authored-by: Aram <[email protected]>
Co-authored-by: alimpens <[email protected]>
  • Loading branch information
3 people authored Jul 26, 2024
1 parent f74a107 commit f5176b1
Show file tree
Hide file tree
Showing 37 changed files with 297 additions and 260 deletions.
12 changes: 11 additions & 1 deletion documentation/storybook.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,20 @@ A guideline on how to use the prop may be added, as well as a description of its
Prevent mentioning the component’s name in a prop description – that should be obvious.
Prop types that aren’t exported don’t require their properties to be described.

### Args

Add [`args`](https://storybook.js.org/docs/react/writing-stories/args) to the Story to set initial values for props.
Follow these guidelines:

1. For Boolean props, set their default value to `false`,
unless this has side effects e.g. rendering a class name.
In that case, don’t specify a value.
Storybook will then display a button ‘Set boolean’ that show a switch.

### Arg Types

Add [`argTypes`](https://storybook.js.org/docs/api/arg-types) to the Story to document native HTML attributes or override the generated controls.
Be sure to follow these guidelines when you do:
Follow these guidelines:

1. Add a `description` field instead of a JSDoc comment for native HTML attributes.
Use terse sentences that end with a full stop.
Expand Down
24 changes: 12 additions & 12 deletions packages/css/src/components/icon-button/icon-button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,31 @@
@include reset;
}

.ams-icon-button--on-background-light {
color: var(--ams-icon-button-on-background-light-color);
.ams-icon-button--contrast-color {
color: var(--ams-icon-button-contrast-color-color);

&:hover {
background-color: var(--ams-icon-button-on-background-light-hover-background-color);
color: var(--ams-icon-button-on-background-light-hover-color);
background-color: var(--ams-icon-button-contrast-color-hover-background-color);
color: var(--ams-icon-button-contrast-color-hover-color);
}

&:disabled {
background-color: transparent;
color: var(--ams-icon-button-on-background-light-disabled-color);
color: var(--ams-icon-button-contrast-color-disabled-color);
}
}

.ams-icon-button--on-background-dark {
background-color: var(--ams-icon-button-on-background-dark-background-color);
color: var(--ams-icon-button-on-background-dark-color);
.ams-icon-button--inverse-color {
background-color: var(--ams-icon-button-inverse-color-background-color);
color: var(--ams-icon-button-inverse-color-color);

&:hover {
background-color: var(--ams-icon-button-on-background-dark-hover-background-color);
color: var(--ams-icon-button-on-background-dark-hover-color);
background-color: var(--ams-icon-button-inverse-color-hover-background-color);
color: var(--ams-icon-button-inverse-color-hover-color);
}

&:disabled {
background-color: var(--ams-icon-button-on-background-dark-disabled-background-color);
color: var(--ams-icon-button-on-background-dark-disabled-color);
background-color: var(--ams-icon-button-inverse-color-disabled-background-color);
color: var(--ams-icon-button-inverse-color-disabled-color);
}
}
12 changes: 6 additions & 6 deletions packages/css/src/components/link-list/link-list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,18 @@
@include hyphenation;
}

.ams-link-list__link--on-background-dark {
color: var(--ams-link-list-link-on-background-dark-color);
.ams-link-list__link--contrast-color {
color: var(--ams-link-list-link-contrast-color-color);

&:hover {
color: var(--ams-link-list-link-on-background-dark-hover-color);
color: var(--ams-link-list-link-contrast-color-hover-color);
}
}

.ams-link-list__link--on-background-light {
color: var(--ams-link-list-link-on-background-light-color);
.ams-link-list__link--inverse-color {
color: var(--ams-link-list-link-inverse-color-color);

&:hover {
color: var(--ams-link-list-link-on-background-light-hover-color);
color: var(--ams-link-list-link-inverse-color-hover-color);
}
}
16 changes: 8 additions & 8 deletions packages/css/src/components/link/link.scss
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,26 @@
}
}

.ams-link--on-background-dark {
color: var(--ams-link-on-background-dark-color);
.ams-link--contrast-color {
color: var(--ams-link-contrast-color-color);

&:hover {
color: var(--ams-link-on-background-dark-hover-color);
color: var(--ams-link-contrast-color-hover-color);
}

&:visited {
color: var(--ams-link-on-background-dark-visited-color);
color: var(--ams-link-contrast-color-visited-color);
}
}

.ams-link--on-background-light {
color: var(--ams-link-on-background-light-color);
.ams-link--inverse-color {
color: var(--ams-link-inverse-color-color);

&:hover {
color: var(--ams-link-on-background-light-hover-color);
color: var(--ams-link-inverse-color-hover-color);
}

&:visited {
color: var(--ams-link-on-background-light-visited-color);
color: var(--ams-link-inverse-color-visited-color);
}
}
12 changes: 6 additions & 6 deletions packages/react/src/IconButton/IconButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,20 @@ describe('Icon button', () => {
expect(component).toBeInTheDocument()
})

it('renders the right on background light class', () => {
render(<IconButton label="Test" onBackground="light" />)
it('renders the class name for contrast color', () => {
render(<IconButton label="Test" contrastColor />)

const component = screen.getByRole('button')

expect(component).toHaveClass('ams-icon-button--on-background-light')
expect(component).toHaveClass('ams-icon-button--contrast-color')
})

it('renders the right on background dark class', () => {
render(<IconButton label="Test" onBackground="dark" />)
it('renders the class name for inverse color', () => {
render(<IconButton label="Test" inverseColor />)

const component = screen.getByRole('button')

expect(component).toHaveClass('ams-icon-button--on-background-dark')
expect(component).toHaveClass('ams-icon-button--inverse-color')
})

it('supports ForwardRef in React', () => {
Expand Down
12 changes: 7 additions & 5 deletions packages/react/src/IconButton/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ import { Icon } from '../Icon'
export type IconButtonProps = {
/** The accessible text for the button. Will be announced by screen readers. Should describe the button’s action. */
label: string
/** Describes the underlying background colour. Allows the icon to provide visual contrast. */
onBackground?: 'light' | 'dark'
/** Changes the text colour for readability on a light (but not white) background. */
contrastColor?: boolean
/** Changes the text colour for readability on a dark background. */
inverseColor?: boolean
/** The size of the icon. Corresponds with the text levels. */
size?: 'level-3' | 'level-4' | 'level-5' | 'level-6'
/** The component rendering the icon’s markup. */
Expand All @@ -22,16 +24,16 @@ export type IconButtonProps = {

export const IconButton = forwardRef(
(
{ className, label, onBackground, size = 'level-5', svg = CloseIcon, ...restProps }: IconButtonProps,
{ className, label, contrastColor, inverseColor, size = 'level-5', svg = CloseIcon, ...restProps }: IconButtonProps,
ref: ForwardedRef<HTMLButtonElement>,
) => (
<button
{...restProps}
ref={ref}
className={clsx(
'ams-icon-button',
onBackground === 'light' && 'ams-icon-button--on-background-light',
onBackground === 'dark' && 'ams-icon-button--on-background-dark',
contrastColor && 'ams-icon-button--contrast-color',
inverseColor && 'ams-icon-button--inverse-color',
className,
)}
>
Expand Down
31 changes: 16 additions & 15 deletions packages/react/src/Link/Link.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import '@testing-library/jest-dom'

describe('Link', () => {
const linktext = 'Linktext'

it('renders with href attribute', () => {
const { container } = render(<Link href="#">{linktext}</Link>)

Expand Down Expand Up @@ -34,37 +35,37 @@ describe('Link', () => {
expect(link).toHaveClass('ams-link ams-link--inline')
})

it('renders light background color', () => {
it('renders an additional class name', () => {
const { container } = render(<Link className="large" />)

const link = container.querySelector(':only-child')

expect(link).toHaveClass('large')
expect(link).toHaveClass('ams-link')
})

it('renders the class name for contrast color', () => {
const { container } = render(
<Link onBackground="light" href="#">
<Link contrastColor href="#">
{linktext}
</Link>,
)

const link = container.querySelector('a:only-child')

expect(link).toHaveClass('ams-link ams-link--on-background-light')
expect(link).toHaveClass('ams-link ams-link--contrast-color')
})

it('renders dark background color', () => {
it('renders the class name for inverse color', () => {
const { container } = render(
<Link onBackground="dark" href="#">
<Link inverseColor href="#">
{linktext}
</Link>,
)

const link = container.querySelector('a:only-child')

expect(link).toHaveClass('ams-link ams-link--on-background-dark')
})

it('renders an additional class name', () => {
const { container } = render(<Link className="large" />)

const link = container.querySelector(':only-child')

expect(link).toHaveClass('large')
expect(link).toHaveClass('ams-link')
expect(link).toHaveClass('ams-link ams-link--inverse-color')
})

it('supports ForwardRef in React', () => {
Expand Down
19 changes: 9 additions & 10 deletions packages/react/src/Link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,31 @@ import clsx from 'clsx'
import { forwardRef } from 'react'
import type { AnchorHTMLAttributes, ForwardedRef } from 'react'

type LinkOnBackground = 'default' | 'light' | 'dark'
type LinkVariant = 'standalone' | 'inline'

export type LinkProps = {
/** Describes the underlying background colour. Allows the text to provide visual contrast. */
onBackground?: LinkOnBackground
/** Changes the text colour for readability on a light background. */
contrastColor?: boolean
/** Changes the text colour for readability on a dark background. */
inverseColor?: boolean
/** Whether the link is inline or stands alone. */
variant?: LinkVariant
} & Omit<AnchorHTMLAttributes<HTMLAnchorElement>, 'placeholder'>

export const Link = forwardRef(
(
{ children, variant = 'standalone', onBackground, className, ...restProps }: LinkProps,
{ children, className, contrastColor, inverseColor, variant = 'standalone', ...restProps }: LinkProps,
ref: ForwardedRef<HTMLAnchorElement>,
) => (
<a
{...restProps}
ref={ref}
className={clsx(
'ams-link',
{
'ams-link--standalone': variant === 'standalone',
'ams-link--inline': variant === 'inline',
'ams-link--on-background-light': onBackground === 'light',
'ams-link--on-background-dark': onBackground === 'dark',
},
contrastColor && 'ams-link--contrast-color',
inverseColor && 'ams-link--inverse-color',
variant === 'inline' && 'ams-link--inline',
variant === 'standalone' && 'ams-link--standalone',
className,
)}
>
Expand Down
14 changes: 11 additions & 3 deletions packages/react/src/LinkList/LinkListLink.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,20 @@ describe('Link list link', () => {
expect(component).toHaveClass('ams-link-list__link--small')
})

it('renders a class name for the background color', () => {
render(<LinkList.Link href="#" onBackground="dark" />)
it('renders the class name for contrast color', () => {
render(<LinkList.Link href="#" contrastColor />)

const component = screen.getByRole('link')

expect(component).toHaveClass('ams-link-list__link--on-background-dark')
expect(component).toHaveClass('ams-link-list__link--contrast-color')
})

it('renders the class name for inverse color', () => {
render(<LinkList.Link href="#" inverseColor />)

const component = screen.getByRole('link')

expect(component).toHaveClass('ams-link-list__link--inverse-color')
})

it('supports ForwardRef in React', () => {
Expand Down
13 changes: 7 additions & 6 deletions packages/react/src/LinkList/LinkListLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import { forwardRef } from 'react'
import type { AnchorHTMLAttributes, ForwardedRef, PropsWithChildren } from 'react'
import { Icon } from '../Icon'

type BackgroundName = 'default' | 'light' | 'dark'

export type LinkListLinkProps = {
/** An icon to display instead of the default chevron. Don’t mix custom icons with chevrons in one list. */
icon?: Function
/** Describes the underlying background colour. Allows the text to provide visual contrast. */
onBackground?: BackgroundName
/** Changes the text colour for readability on a light background. */
contrastColor?: boolean
/** Changes the text colour for readability on a dark background. */
inverseColor?: boolean
/** The size of the text. Use the same size for all items in the list. */
size?: 'small' | 'large'
} & PropsWithChildren<AnchorHTMLAttributes<HTMLAnchorElement>>
Expand All @@ -29,15 +29,16 @@ const iconSizeMap = {
/** One link with a Link List. */
export const LinkListLink = forwardRef(
(
{ children, className, icon, onBackground, size, ...restProps }: LinkListLinkProps,
{ children, className, icon, contrastColor, inverseColor, size, ...restProps }: LinkListLinkProps,
ref: ForwardedRef<HTMLAnchorElement>,
) => {
return (
<li>
<a
className={clsx(
'ams-link-list__link',
onBackground && `ams-link-list__link--on-background-${onBackground}`,
contrastColor && 'ams-link-list__link--contrast-color',
inverseColor && 'ams-link-list__link--inverse-color',
size && `ams-link-list__link--${size}`,
className,
)}
Expand Down
24 changes: 18 additions & 6 deletions proprietary/tokens/src/common/ams/link-appearance.tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
"hover": {
"color": { "value": "{ams.color.dark-blue}" }
},
"on-background-dark": {
"color": { "value": "{ams.color.primary-white}" }
},
"on-background-light": {
"color": { "value": "{ams.color.primary-black}" }
},
"regular": {
"text-decoration-line": { "value": "underline" },
"hover": {
Expand All @@ -25,6 +19,24 @@
"hover": {
"text-decoration-line": { "value": "underline" }
}
},
"contrast": {
"color": { "value": "{ams.color.primary-black}" },
"hover": {
"color": { "value": "{ams.color.primary-black}" }
},
"visited": {
"color": { "value": "{ams.color.primary-black}" }
}
},
"inverse": {
"color": { "value": "{ams.color.primary-white}" },
"hover": {
"color": { "value": "{ams.color.primary-white}" }
},
"visited": {
"color": { "value": "{ams.color.primary-white}" }
}
}
}
}
Expand Down
Loading

0 comments on commit f5176b1

Please sign in to comment.