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 Aspect Ratio component with utility classes and prop on Image #1593

Merged
merged 50 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
a47075d
WIP(aspect-ratio): 🚧 comp to props
RubenSibon Sep 20, 2024
ae71fa9
Merge branch 'develop' of github.com:Amsterdam/design-system into fea…
RubenSibon Sep 20, 2024
859edb9
WIP(aspect-ratio): clarifying TODO comment
RubenSibon Sep 20, 2024
4ef95dd
Merge branches 'feat/DES-834-aspect-ratio-comp-to-prop' and 'develop'…
RubenSibon Oct 2, 2024
b07ffc5
Merge branch 'develop' of github.com:Amsterdam/design-system into fea…
RubenSibon Oct 16, 2024
9999466
fix: apply ratio to Breakout stories
RubenSibon Oct 16, 2024
239a0d6
docs(ratio): 📄 more prop descr
RubenSibon Oct 16, 2024
5c85695
docs(ratio): 📄 design token docs
RubenSibon Oct 16, 2024
55943f9
docs(ratio): 🪞 refer to token docs
RubenSibon Oct 16, 2024
cbe31d2
chore(ratio): 🗑️ remove out-of-scope code
RubenSibon Oct 16, 2024
a612015
test(ratio): 🧪 test for ratio class names
RubenSibon Oct 18, 2024
e35e71f
feat(ratio): 🪩 add class names to Image
RubenSibon Oct 18, 2024
e540c06
Merge branch 'develop' of github.com:Amsterdam/design-system into fea…
RubenSibon Oct 18, 2024
d0874a3
feat(ratio): 🦋 aspect-ratio mixins
RubenSibon Oct 18, 2024
a51445f
docs(ratio) 📄 cleanup and expand
RubenSibon Oct 18, 2024
6232739
fix(ratio): docs
RubenSibon Oct 18, 2024
be946f1
chore: remove unnecessary todo
RubenSibon Oct 18, 2024
0b8e4b5
Merge branch 'develop' of github.com:Amsterdam/design-system into fea…
RubenSibon Oct 30, 2024
441bd2e
style(image): ♻️ prop order and sorting
RubenSibon Oct 30, 2024
69fcdf7
refactor(ratio): 🦋 remove mixin and img class
RubenSibon Oct 30, 2024
6731087
refactor(ratio): ♻️ replace comp with common types
RubenSibon Oct 30, 2024
04aa7d9
test(image): 🖼️ new className
RubenSibon Oct 30, 2024
3900d6b
docs(image): 📄 simplify aspect ratio docs
RubenSibon Oct 30, 2024
2ee01ac
docs(ratio): 📄 CSS class util story
RubenSibon Oct 30, 2024
dd7ca4f
Merge branch 'develop' of github.com:Amsterdam/design-system into fea…
RubenSibon Oct 30, 2024
326dfa2
Merge branch 'develop' of github.com:Amsterdam/design-system into fea…
RubenSibon Nov 1, 2024
3f25d4b
feat(image)!: 🗑️ remove cover prop
RubenSibon Nov 1, 2024
dd73879
refactor(ratio): ♻️ use color tokens in story
RubenSibon Nov 1, 2024
e993691
Merge branch 'develop' into feat/DES-834-aspect-ratio-comp-to-prop
VincentSmedinga Nov 7, 2024
e812d3e
Make token ans class documentation consistent
VincentSmedinga Nov 7, 2024
07cf808
Use a pink box for the generic example
VincentSmedinga Nov 7, 2024
5aa4423
Update border heading and table layout for consistency
VincentSmedinga Nov 7, 2024
59d79cc
Set the default aspect ratio for images
VincentSmedinga Nov 7, 2024
f349157
Remove intermediate type
VincentSmedinga Nov 7, 2024
138bf3b
Consolidate reused types
VincentSmedinga Nov 7, 2024
01b0f35
Test for extra class consistently
VincentSmedinga Nov 7, 2024
a194a11
Write aspect ratios consistently and use token
VincentSmedinga Nov 7, 2024
114d828
Add ‘Tokens’ heading
VincentSmedinga Nov 7, 2024
393b789
Fix type import paths
VincentSmedinga Nov 7, 2024
fa3006a
Remove explicit default aspect ratio from Image
VincentSmedinga Nov 7, 2024
dea0535
Test all aspect ratio classes on Image
VincentSmedinga Nov 7, 2024
1aca351
Remove duplicate type from union
VincentSmedinga Nov 7, 2024
487766f
Remove default aspect ratio from Image instances
VincentSmedinga Nov 7, 2024
8f9ecc1
Remove obsolete prop
VincentSmedinga Nov 7, 2024
d6787be
Ensure the utility class always wins
VincentSmedinga Nov 8, 2024
bfa6dfb
Inline type to prevent confusing rename
VincentSmedinga Nov 8, 2024
e47dc7f
Revert compiler setting
VincentSmedinga Nov 8, 2024
14893b9
Infer removed type
VincentSmedinga Nov 8, 2024
062f47e
Use consistent rationale
VincentSmedinga Nov 8, 2024
e0c4049
Apply suggestions from code review
VincentSmedinga Nov 8, 2024
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: 14 additions & 10 deletions packages/css/src/components/aspect-ratio/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@

Aspect Ratio updates the height of an element with its width.

These are the proportions we use:
## Class names

| keyword | ratio |
| :-------- | ----: |
| `2x-wide` | 16:5 |
| `x-wide` | 16:9 |
| `wide` | 4:3 |
| `square` | 1:1 |
| `tall` | 3:4 |
| `x-tall` | 9:16 |
Each available aspect ratio has an associated class name.
The class can be applied to any component or element.

| Class name | Example |
| :-------------------------- | :-------------------------------------------------------------------------- |
| `ams-aspect-ratio--x-tall` | <div className="ams-docs-token-example--space ams-aspect-ratio--x-tall" /> |
| `ams-aspect-ratio--tall` | <div className="ams-docs-token-example--space ams-aspect-ratio--tall" /> |
| `ams-aspect-ratio--square` | <div className="ams-docs-token-example--space ams-aspect-ratio--square" /> |
| `ams-aspect-ratio--wide` | <div className="ams-docs-token-example--space ams-aspect-ratio--wide" /> |
| `ams-aspect-ratio--x-wide` | <div className="ams-docs-token-example--space ams-aspect-ratio--x-wide" /> |
| `ams-aspect-ratio--2x-wide` | <div className="ams-docs-token-example--space ams-aspect-ratio--2x-wide" /> |

## Guidelines

- Use this component to constrain images, videos and other elements to one of the available aspect ratios.
- Apply one of these classes to an image, video or other media content to constrain its dimensions to one of the available aspect ratios.
- The default aspect ratio is 16:9.
31 changes: 5 additions & 26 deletions packages/css/src/components/aspect-ratio/aspect-ratio.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,9 @@
*/

.ams-aspect-ratio {
overflow: hidden;
position: relative;
}

.ams-aspect-ratio--x-tall {
aspect-ratio: var(--ams-aspect-ratio-x-tall);
}

.ams-aspect-ratio--tall {
aspect-ratio: var(--ams-aspect-ratio-tall);
}

.ams-aspect-ratio--square {
aspect-ratio: var(--ams-aspect-ratio-square);
}

.ams-aspect-ratio--wide {
aspect-ratio: var(--ams-aspect-ratio-wide);
}

.ams-aspect-ratio--x-wide {
aspect-ratio: var(--ams-aspect-ratio-x-wide);
}

.ams-aspect-ratio--2x-wide {
aspect-ratio: var(--ams-aspect-ratio-2x-wide);
@each $name in ("x-tall", "tall", "square", "wide", "x-wide", "2x-wide") {
&--#{$name} {
aspect-ratio: var(--ams-aspect-ratio-#{$name});
}
}
}
7 changes: 6 additions & 1 deletion packages/css/src/components/image/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

Displays an image.

## Design

Every Image must display in one of the [available aspect ratios](https://designsystem.amsterdam/?path=/docs/brand-design-tokens-aspect-ratio--docs).
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved
The default is 16:9.
If the intrinsic dimensions of the source do not match an aspect ratio, the image will get cropped to cover the intended area without distorting its appearance.
VincentSmedinga marked this conversation as resolved.
Show resolved Hide resolved

## Guidelines

- Do not forget to include a description of the image in the `alt` attribute.
Expand All @@ -15,7 +21,6 @@ Displays an image.
For example, provide small, medium, and large variants for various screen sizes.
This prevents unnecessary downloading of large files.
Do this especially for the main image of a page, where the difference between sizes on a narrow and wide screen is most significant.
- Ensure that the aspect ratio of the image is supported by the [Aspect Ratio](/docs/components-layout-aspect-ratio--docs) component.

## Relevant WCAG requirements

Expand Down
8 changes: 4 additions & 4 deletions packages/css/src/components/image/image.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@
*/

.ams-image {
aspect-ratio: var(--ams-image-aspect-ratio);
block-size: auto; /* [1] */
font-style: italic; /* [3] */
inline-size: fit-content;
max-inline-size: 100%; /* [1] */
object-fit: cover; /* [4] */
vertical-align: middle; /* [2] */

&--cover {
object-fit: cover;
}
}

// [1] Allow for fluid image sizing while maintaining aspect ratio governed by inline/block size attributes
// [2] Remove ‘phantom’ white space
// [3] Italicise alt text to visually offset it from surrounding copy
// Source: https://x.com/csswizardry/status/1717841334462005661
// [4] Crop the image to maintain a supported aspect ratio
62 changes: 0 additions & 62 deletions packages/react/src/AspectRatio/AspectRatio.test.tsx

This file was deleted.

25 changes: 0 additions & 25 deletions packages/react/src/AspectRatio/AspectRatio.tsx

This file was deleted.

5 changes: 0 additions & 5 deletions packages/react/src/AspectRatio/README.md

This file was deleted.

2 changes: 0 additions & 2 deletions packages/react/src/AspectRatio/index.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react/src/Column/Column.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, screen } from '@testing-library/react'
import { createRef } from 'react'
import { Column, columnGapSizes } from './Column'
import { crossAlignOptionsForColumn, mainAlignOptions } from '../common/layout'
import { crossAlignOptionsForColumn, mainAlignOptions } from '../common/types'
import '@testing-library/jest-dom'

describe('Column', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Column/Column.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import clsx from 'clsx'
import { forwardRef } from 'react'
import type { HTMLAttributes, PropsWithChildren } from 'react'
import type { CrossAlignForColumn, MainAlign } from '../common/layout'
import type { CrossAlignForColumn, MainAlign } from '../common/types'

export const columnGapSizes = ['none', 'extra-small', 'small', 'large', 'extra-large'] as const

Expand Down
14 changes: 8 additions & 6 deletions packages/react/src/Image/Image.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { render } from '@testing-library/react'
import { createRef } from 'react'
import { Image } from './Image'
import { aspectRatioOptions } from '../common/types'
import '@testing-library/jest-dom'

describe('Image', () => {
Expand All @@ -26,16 +27,17 @@ describe('Image', () => {

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

expect(component).toHaveClass('extra')
expect(component).toHaveClass('ams-image')
expect(component).toHaveClass('ams-image extra')
})

it('renders a class name to display the image as large as its container', () => {
const { container } = render(<Image cover />)
aspectRatioOptions.forEach((aspectRatio) => {
it(`renders class names to display the image in the ${aspectRatio} aspect ratio`, () => {
const { container } = render(<Image aspectRatio={aspectRatio} />)

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

expect(component).toHaveClass('ams-image--cover')
expect(component).toHaveClass(`ams-aspect-ratio--${aspectRatio}`)
})
})

it('supports ForwardRef in React', () => {
Expand Down
14 changes: 8 additions & 6 deletions packages/react/src/Image/Image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@
import clsx from 'clsx'
import { forwardRef } from 'react'
import type { ForwardedRef, ImgHTMLAttributes } from 'react'
import { AspectRatioProps } from '../common/types'

export type ImageProps = {
/** Whether to display the image exactly as large as its container. This will clip the image if necessary. */
cover?: boolean
} & ImgHTMLAttributes<HTMLImageElement>
export type ImageProps = AspectRatioProps & ImgHTMLAttributes<HTMLImageElement>

export const Image = forwardRef(
({ className, cover = false, ...restProps }: ImageProps, ref: ForwardedRef<HTMLImageElement>) => (
<img {...restProps} ref={ref} className={clsx('ams-image', cover && 'ams-image--cover', className)} />
({ aspectRatio, className, ...restProps }: ImageProps, ref: ForwardedRef<HTMLImageElement>) => (
<img
{...restProps}
ref={ref}
className={clsx('ams-image', aspectRatio && `ams-aspect-ratio--${aspectRatio}`, className)}
/>
),
)

Expand Down
6 changes: 1 addition & 5 deletions packages/react/src/ImageSlider/ImageSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,9 @@ import { ImageSliderControls } from './ImageSliderControls'
import { ImageSliderItem } from './ImageSliderItem'
import { ImageSliderScroller } from './ImageSliderScroller'
import { ImageSliderThumbnails } from './ImageSliderThumbnails'
import { Ratio } from '../AspectRatio'
import { Image, ImageProps } from '../Image/Image'

export type ImageSliderImageProps = ImageProps & {
/** Specify the aspect ratio to use for the images. */
aspectRatio: Ratio
}
export type ImageSliderImageProps = ImageProps

export type ImageSliderProps = {
/** Display buttons to navigate to the previous or next image. */
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Row/Row.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, screen } from '@testing-library/react'
import { createRef } from 'react'
import { Row, rowGapSizes } from './Row'
import { crossAlignOptions, mainAlignOptions } from '../common/layout'
import { crossAlignOptions, mainAlignOptions } from '../common/types'
import '@testing-library/jest-dom'

describe('Row', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Row/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import clsx from 'clsx'
import { forwardRef } from 'react'
import type { HTMLAttributes, PropsWithChildren } from 'react'
import type { CrossAlign, MainAlign } from '../common/layout'
import type { CrossAlign, MainAlign } from '../common/types'

export const rowGapSizes = ['none', 'extra-small', 'small', 'large', 'extra-large'] as const

Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Switch/Switch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('Switch', () => {

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

expect(component).toHaveClass('ams-switch', 'extra')
expect(component).toHaveClass('ams-switch extra')
})

it('is able to pass a React ref', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
export const aspectRatioOptions = ['x-tall', 'tall', 'square', 'wide', 'x-wide', '2x-wide'] as const
export type AspectRatioProps = {
/** The aspect ratio to display media content in. */
aspectRatio?: (typeof aspectRatioOptions)[number]
}

export const crossAlignOptions = ['start', 'center', 'baseline', 'end'] as const
export type CrossAlign = (typeof crossAlignOptions)[number]

Expand Down
1 change: 0 additions & 1 deletion packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export * from './Switch'
export * from './Spotlight'
export * from './Card'
export * from './Alert'
export * from './AspectRatio'
export * from './Footer'
export * from './PageMenu'
export * from './TopTaskLink'
Expand Down
2 changes: 1 addition & 1 deletion proprietary/tokens/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ Examples:
:root {
--ams-color-primary-red: #ec0000;
--ams-space-md: 1rem;
--ams-proportion-wide: 4/3;
--ams-aspect-ratio-wide: 4/3;
--ams-border-width-lg: 0.1875rem;
}
```
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ams": {
"proportion": {
"aspect-ratio": {
"x-tall": { "value": "9 / 16" },
"tall": { "value": "3 / 4" },
"square": { "value": "1 / 1" },
Expand Down
12 changes: 0 additions & 12 deletions proprietary/tokens/src/components/ams/aspect-ratio.tokens.json

This file was deleted.

2 changes: 1 addition & 1 deletion proprietary/tokens/src/components/ams/avatar.tokens.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"ams": {
"avatar": {
"aspect-ratio": { "value": "{ams.proportion.square}" },
"aspect-ratio": { "value": "{ams.aspect-ratio.square}" },
"font-family": { "value": "{ams.text.font-family}" },
"font-size": { "value": "{ams.text.level.6.font-size}" },
"font-weight": { "value": "{ams.text.font-weight.normal}" },
Expand Down
7 changes: 7 additions & 0 deletions proprietary/tokens/src/components/ams/image.tokens.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"ams": {
"image": {
"aspect-ratio": { "value": "{ams.aspect-ratio.x-wide}" }
}
}
}
Loading