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 5 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
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 background. */
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
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
4 changes: 2 additions & 2 deletions proprietary/tokens/src/components/ams/icon-button.tokens.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"disabled": {
"color": { "value": "{ams.color.neutral-grey2}" }
},
"on-background-light": {
"contrast-color": {
"color": { "value": "{ams.color.primary-black}" },
"hover": {
"background-color": { "value": "rgba(0, 0, 0, 0.125)" },
Expand All @@ -20,7 +20,7 @@
"color": { "value": "{ams.color.neutral-grey2}" }
}
},
"on-background-dark": {
"inverse-color": {
"background-color": { "value": "{ams.color.primary-blue}" },
"color": { "value": "{ams.color.primary-white}" },
"hover": {
Expand Down
Loading