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.

Sign up for GitHub

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

Image component refactor #627

Closed
wants to merge 2 commits into from
Closed

Image component refactor #627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c370dc
fix image component story controls
joshfarrant Jun 25, 2024
0b4584e
squash
joshfarrant Aug 5, 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
16 changes: 16 additions & 0 deletions .changeset/early-balloons-carry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@primer/react-brand': minor
---

Refactored Image component and simplified its API.

- Removed the `as`, `media`, and `sources` props, as well code paths which render a `<picture>` element. To use a `<picture>` element with the updated `Image` component, wrap the `Image` with a `picture` element, for example:

```jsx
<picture>
<source srcSet="..." media="..." />
<Image src="..." alt="..." />
</picture>
```

- Fixed an edge case with the border radius of the `Card.Image` component. It was previously possible for the div that wraps the image and the image itself to have different border radii.
136 changes: 67 additions & 69 deletions apps/docs/content/components/Image/react.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ description: Use the image component to display a graphical representation.
---

import ComponentLayout from '../../../src/layouts/component-layout'
import {PropTableValues} from '../../../src/components'
import {ImageBorderRadiusOptions} from '@primer/react-brand'
export default ComponentLayout

import {Label} from '@primer/react'
export default ComponentLayout

```js
import {Image} from '@primer/react-brand'
Expand All @@ -20,7 +21,7 @@ import {Image} from '@primer/react-brand'

### Default

This component uses the `img` element by default.
This component renders an `img` element.

```jsx live
<Image
Expand All @@ -31,36 +32,36 @@ This component uses the `img` element by default.

### Picture

The `as` prop can be used to set the container of the image to use `picture`.
As the `Image` component always renders an `img` element, you can wrap it with a `picture` element.

```jsx live
<Image
as="picture"
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
/>
<picture>
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
/>
</picture>
```

### Picture with sources

The `sources` prop can be used to set the source elements within the `picture` component. This can only be used when `as` is set to `picture`.
Add `source` elements as siblings to the `Image` component inside the `picture` element.

```jsx live
<Image
as="picture"
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
sources={[
{
srcset: 'https://via.placeholder.com/600x400/d3d9df/d3d9df.png',
media: '(min-width: 600px)',
},
{
srcset: 'https://via.placeholder.com/900x600/d3d9df/d3d9df.png',
media: '(min-width: 900px)',
},
]}
/>
<picture>
<source
srcset="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
media="(min-width: 600px)"
/>
<source
srcset="https://via.placeholder.com/900x600/d3d9df/d3d9df.png"
media="(min-width: 900px)"
/>
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
/>
</picture>
```

### Image with source set
Expand All @@ -77,27 +78,43 @@ The `srcSet` prop can be used to set the srcSet of the image. This can only be u

### Aspect ratio

The `aspectRatio` prop can be used to set the aspect ratio of the image. This is useful when the image is not the same aspect ratio as the container.

```jsx live
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
aspectRatio="16:9"
/>
```

### Height

The `height` prop can be used to set the height of the image. This can be used along side the `aspectRatio` prop to create a responsive image the same size as other images.
The CSS `aspect-ratio` property (or `style.aspectRatio`) can be used to set the aspect ratio of the image. Note that either `width` or `height` must be set for this to take effect.

```jsx live
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
height={200}
aspectRatio="16:9"
/>
<Stack direction="horizontal" alignItems="flex-start">
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
height={100}
style={{
aspectRatio: '1/1',
}}
/>
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
height={100}
style={{
aspectRatio: '4/3',
}}
/>
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
height={100}
style={{
aspectRatio: '16/10',
}}
/>
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
height={100}
style={{
aspectRatio: '16/9',
}}
/>
</Stack>
```

### Border radius
Expand Down Expand Up @@ -144,33 +161,14 @@ The `borderRadius` prop can be used to apply rounded corners to images using pre
</Stack>
```

### Width

The `width` prop can be used to set the width of the image. This can be used along side the `aspectRatio` prop to create a responsive image the same size as other images.

```jsx live
<Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an off-white background color"
width={200}
aspectRatio="16:9"
/>
```

## Component props

### Image <Label>Required</Label>

| name | type | default | required | description |
| -------------- | -------------------------------------------------------------------------- | ----------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `src` | `string` | | `true` | Specifies the path to the image |
| `alt` | `string` | | `true` | Specifies a text value explaining the nature of the image for users of assistive technology |
| `as` | `img`, `picture` | `img` | `false` | Specification to create a picture component |
| `sources` | `{srcset: string, media: string}[]` | | `false` | When picture is specified in the `as` prop sources allows you to set the source elements. |
| `aspectRatio` | `'1:1'`, `'16:9'`, `'16:10'`, `'4:3'`, `'custom'` | `undefined` | `false` | Sets the image aspect ratio. A custom ratio can be provided in the design tokens. |
| `borderRadius` | <PropTableValues values={[...ImageBorderRadiusOptions]} commaSeparated /> | `undefined` | `false` | Applies a system-level border radius value to the Image. |
| `height` | `number` | | `false` | The height of the image element or its container if it has an aspect ratio |
| `width` | `number` | | `false` | The width of the image element or its container if it has an aspect ratio |
| `loading` | `eager`, `lazy` | `eager` | `false` | The loading attribute specifies whether a browser should load an image immediately or to defer loading of off-screen images until for example the user scrolls near them. |
| `decoding` | `sync`, `async`, `auto` | `sync` | `false` | Sets the image decoding strategy. Representing a hint given to the browser on how it should decode the image. |
| `className` | `string` | | `false` | Sets a custom CSS class |
All standard `img` attributes are supported as defined by `React.ImgHTMLAttributes<HTMLImageElement>` with the following additions.

| name | type | default | required | description |
| -------------- | -------------------------------------------------------------------------- | ----------- | -------- | -------------------------------------------------------------------------------------------- |
| `alt` | `string` | | `true` | Specifies a text value explaining the nature of the image for users of assistive technology. |
| `animate` | `AnimateProps` | `undefined` | `false` | When wrapped in an `<AnimationProvider>`, applies an animation preset. |
| `borderRadius` | <PropTableValues values={[...ImageBorderRadiusOptions]} commaSeparated /> | `undefined` | `false` | Applies a system-level border radius value to the Image. |
5 changes: 4 additions & 1 deletion packages/react/src/Card/Card.features.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ export const ImageAndLabel: StoryFn<typeof Card> = () => {
export const ImageUsingPictureElement: StoryFn<typeof Card> = () => {
return (
<Card href="https://github.com">
<Card.Image as="picture" src={placeholderImage} alt="placeholder, blank area with an gray background color" />
<picture>
<source srcSet={placeholderImage} media="(min-width: 600px)" />
<Card.Image src={placeholderImage} alt="placeholder, blank area with an gray background color" />
</picture>
<Card.Heading>Code search & code view</Card.Heading>
<Card.Description>
Enables you to rapidly search, navigate, and understand code, right from GitHub.com.
Expand Down
5 changes: 4 additions & 1 deletion packages/react/src/Card/Card.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
max-width: unset;
}

.Card--fullWidth .Card__image {
width: 100%;
}

.Card--variant-minimal {
padding: 0;
}
Expand Down Expand Up @@ -96,7 +100,6 @@

.Card__image {
margin-bottom: var(--base-size-28);
border-radius: var(--brand-borderRadius-medium);
overflow: hidden;
grid-area: image;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/Card/Card.module.css.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
declare const styles: {
readonly "Card": string;
readonly "Card--fullWidth": string;
readonly "Card__image": string;
readonly "Card--variant-minimal": string;
readonly "Card--colorMode-light": string;
readonly "Card__link": string;
Expand All @@ -12,7 +13,6 @@ declare const styles: {
readonly "Card__icon": string;
readonly "Card--variant-default": string;
readonly "Card--colorMode-dark": string;
readonly "Card__image": string;
readonly "Card__icon--badge": string;
readonly "Card__heading": string;
readonly "Card__description": string;
Expand Down
8 changes: 2 additions & 6 deletions packages/react/src/Card/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,8 @@ function DefaultCardWrapperComponent({children}) {

type CardImageProps = ImageProps

function CardImage({className, ...rest}: CardImageProps) {
return (
<div className={styles.Card__image}>
<Image className={className} {...rest} />
</div>
)
function CardImage({borderRadius = 'medium', className, ...props}: CardImageProps) {
return <Image borderRadius={borderRadius} className={clsx(styles.Card__image, className)} {...props} />
}

type CardIconProps = BaseProps<HTMLSpanElement> & {
Expand Down
75 changes: 15 additions & 60 deletions packages/react/src/Image/Image.features.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,77 +2,32 @@ import React from 'react'
import {Meta, StoryFn} from '@storybook/react'

import placeholderImage from '../fixtures/images/placeholder-600x400.png'
import style from './Image.stories.module.css'

import {Image, ImageBorderRadiusOptions} from './Image'
import {Image, type ImageAspectRatio, ImageBorderRadiusOptions} from './Image'
import {Stack} from '../Stack'

export default {
title: 'Components/Image/Features',
component: Image,
} as Meta<typeof Image>

export const CustomPictureAspectRatio: StoryFn<typeof Image> = () => (
<Image
src={placeholderImage}
alt="placeholder, blank area with an off-white background color"
aspectRatio="custom"
as="picture"
/>
)

export const CustomImageAspectRatio: StoryFn<typeof Image> = () => (
<Image src={placeholderImage} alt="placeholder, blank area with an off-white background color" aspectRatio="custom" />
)

export const CustomImageHeight: StoryFn<typeof Image> = () => (
<Image src={placeholderImage} alt="placeholder, blank area with an off-white background color" height={200} />
)

export const CustomImageWidth: StoryFn<typeof Image> = () => (
<Image src={placeholderImage} alt="placeholder, blank area with an off-white background color" width={200} />
)

export const CustomImageWidthAndHeight: StoryFn<typeof Image> = () => (
<Image
src={placeholderImage}
alt="placeholder, blank area with an off-white background color"
height={200}
width={200}
/>
)

export const CustomClass: StoryFn<typeof Image> = () => (
<Image
src={placeholderImage}
className={style['custom-image']}
alt="placeholder, blank area with an off-white background color"
height={200}
width={200}
/>
)

export const CustomClassOnPicture: StoryFn<typeof Image> = () => (
<Image
src={placeholderImage}
className={style['custom-image']}
alt="placeholder, blank area with an off-white background color"
height={200}
width={200}
as="picture"
/>
)
const demoAspectRatios: ImageAspectRatio[] = ['16:9', '16:10', '4:3', '1:1']

export const CustomClassWithAspectRatio: StoryFn<typeof Image> = () => (
<Image
src={placeholderImage}
className={style['custom-image']}
alt="placeholder, blank area with an off-white background color"
aspectRatio="custom"
/>
export const AspectRatio: StoryFn<typeof Image> = () => (
<Stack direction="horizontal" alignItems="flex-start">
{demoAspectRatios.map(aspectRatio => (
<Image
key={aspectRatio}
src={placeholderImage}
alt="placeholder, blank area with an off-white background color"
width={200}
aspectRatio={aspectRatio}
/>
))}
</Stack>
)

export const BorderRadiusOptions: StoryFn<typeof Image> = () => (
export const BorderRadius: StoryFn<typeof Image> = () => (
<Stack direction="horizontal">
{ImageBorderRadiusOptions.map(borderRadius => (
<Image
Expand Down
15 changes: 6 additions & 9 deletions packages/react/src/Image/Image.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
.Image__container {
display: inline-block;
}

.Image {
object-fit: cover;
display: inline-block;
aspect-ratio: var(--brand-image-aspectRatio);
}

.Image--aspect-ratio-custom {
.Image--aspectRatio-custom {
aspect-ratio: var(--brand-image-aspectRatio);
}

.Image--aspect-ratio-1-1 {
.Image--aspectRatio-1-1 {
aspect-ratio: 1 / 1;
}

.Image--aspect-ratio-16-9 {
.Image--aspectRatio-16-9 {
aspect-ratio: 16 / 9;
}

.Image--aspect-ratio-16-10 {
.Image--aspectRatio-16-10 {
aspect-ratio: 16 / 10;
}

.Image--aspect-ratio-4-3 {
.Image--aspectRatio-4-3 {
aspect-ratio: 4 / 3;
}

Expand Down
Loading
Loading