Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Replace ‘on background’ props with ‘inverseColor’ and ‘contrastColor’ for Link, Link List Link, and Icon Button #1448

Merged
merged 15 commits into from
Jul 26, 2024
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
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