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

Add image support to Pillar component #862

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
45607ac
Add image support to Pillar component
danielguillan Dec 11, 2024
1f3dec8
Add changeset
danielguillan Dec 11, 2024
863c87e
Update snapshots
danielguillan Dec 12, 2024
a19b21c
Standardize image margin across Pillar and Card
danielguillan Dec 12, 2024
12e600a
Update interface guidelines
danielguillan Dec 12, 2024
fd25bf2
Update React docs
danielguillan Dec 12, 2024
76c1378
Update .changeset/proud-students-collect.md
danielguillan Dec 17, 2024
912eb34
Update apps/docs/content/components/Pillar/index.mdx
danielguillan Dec 17, 2024
ac975a6
Update apps/docs/content/components/Pillar/index.mdx
danielguillan Dec 17, 2024
c3219e1
Update Pillar and Card docs to use minimal example for image usage
danielguillan Dec 17, 2024
39441ef
Update snapshots
danielguillan Dec 18, 2024
eb099a9
Update snapshots
danielguillan Dec 17, 2024
09013e8
Update apps/docs/content/components/Pillar/react.mdx
danielguillan Dec 17, 2024
f8d8997
Fix docs
danielguillan Dec 17, 2024
4bf6462
Update packages/react/src/Pillar/Pillar.test.tsx
danielguillan Dec 17, 2024
3008b7a
Update packages/react/src/Pillar/Pillar.test.tsx
danielguillan Dec 17, 2024
b1dee09
Remove snapshots
danielguillan Dec 18, 2024
ce6fc64
Add snapshots from main
danielguillan Dec 18, 2024
41a9432
Update snapshots
danielguillan Dec 18, 2024
d07ee45
Update snapshots
danielguillan Dec 18, 2024
047b7d8
Remove snapshots
danielguillan Dec 18, 2024
004e757
Add snapshots from main
danielguillan Dec 18, 2024
28608f9
Update snapshots
danielguillan Dec 18, 2024
518eb7b
Update snapshots
danielguillan Dec 18, 2024
257e59c
Remove old snapshosts
danielguillan Dec 18, 2024
230f59d
Merge branch 'main' into danielguillan/add-pillar-image-support
danielguillan Dec 19, 2024
1392374
Update image margin
danielguillan Dec 19, 2024
9849354
Update snapshots
danielguillan Dec 19, 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
5 changes: 5 additions & 0 deletions .changeset/proud-students-collect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react-brand': patch
---

Added image support to `Pillar` component
4 changes: 2 additions & 2 deletions apps/docs/content/components/Card/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Cards should always be used in groups of two or more related concepts.

### Stacked

Cards should be stacked to display a list of items that are related to each other. If displaying a visual asset, do so for all cards to achieve a visually balanced layout. The visual asset should be the same type for all cards, either an icon or an image.
Cards should be stacked to display a list of items that are related to each other. When displaying a visual asset, ensure it is consistent across all cards to create a visually balanced layout. The visual asset should be the same type for each card, either an icon or an image.

For a better visual experience, we recommend using similar length size for the heading and description in all stacked cards.

Expand Down Expand Up @@ -123,7 +123,7 @@ The call to action label indicates the action or resource the card links to. It

### Border

Card offers a variation with a border. Border is disabled by default.
Card offers a variation with a border. Border is disabled by default.

## Related components

Expand Down
1 change: 0 additions & 1 deletion apps/docs/content/components/Card/react.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ Use the `Image` component to add an image to the `Card`. The `Image` component i
<Card.Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an gray background color"
aspectRatio="16:9"
/>
<Card.Heading>Code search & code view</Card.Heading>
<Card.Description>
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
45 changes: 33 additions & 12 deletions apps/docs/content/components/Pillar/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ description: Use the pillar component to group related content together.
---

import ComponentLayout from '../../../src/layouts/component-layout'
import doStackImages from './images/do-stack-images.png'
import dontStackImages from './images/dont-stack-images.png'

export default ComponentLayout

## Anatomy
Expand All @@ -17,34 +20,38 @@ export default ComponentLayout

## Usage

Use pillars to group similar features or benefits together to easily scan and read. Pillars display content in a familiar and recognizable style.
Use pillars to display a collection of features or benefits, making them easier to scan and read. Pillars present content in a familiar and recognizable style.

### Stacked

Pillars should be stacked to display a list of items that are related to each other. Use consistent pillar sizes within a row or group. A group or a row of content pillars is more visually appealing when the widths and heights of all elements match, so keep the content lenght of each pillar consistent.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈 thanks for tidying up those typo's.. wondering if we should enable a CI spell checker

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be neat!

Stack multiple pillars to create a collection of items. Ensure that all the items in a group have consistent content lengths so that their width and height match, making the content more visually appealing. When displaying a visual, create a balanced layout by using the same type for each pillar, either an icon or an image.

<DoDontContainer>
<Do>
<img src="https://github.com/primer/brand/assets/912236/cd9679ab-cfa0-4c6d-8a55-21d16668f1ca" />
<Caption>
Use a similar content size to achieve a visually balanced layout.
Use similar character counts to achieve a visually balanced layout.
</Caption>
</Do>
<Dont>
<img src="https://github.com/primer/brand/assets/912236/cd6e471f-98cf-4c70-801f-c0aeb4d2bdf5" />
<Caption>
Don't use different length sizes for the title or the description.
Avoid varying content lengths for the title and description.
</Caption>
</Dont>
</DoDontContainer>

If displaying an icon, do so for all cards to achieve a visually balanced layout.
<Note variant="warning">
Use pillars with images only for groups of a maximum of three to six items.
Avoid stacking multiple sections that each contain groups of six pillars.
</Note>

<DoDontContainer>
<Do>
<img src="https://github.com/primer/brand/assets/912236/cdceda67-a911-40a0-839a-4620c83fe510" />
<Caption>
Use icons either for all pillars or none for a visually balanced layout.
When using icons, add them to all pillars to create a visually balanced
layout.
</Caption>
</Do>
<Dont>
Expand All @@ -53,9 +60,23 @@ If displaying an icon, do so for all cards to achieve a visually balanced layout
</Dont>
</DoDontContainer>

When presenting more than four pillars, we recommend displaying them in multiple rows. You can use the [Grid](/components/Grid) or the [Stack](/components/Stack) component to achieve this. We recommend using a grid with 3 or 4 columns and balancing the number of pillars on each row.
<DoDontContainer>
<Do>
<img alt="" src={doStackImages} />
<Caption>
When using images, add them to all pillars to create a visually balanced
layout.
</Caption>
</Do>
<Dont>
<img alt="" src={dontStackImages} />
<Caption>Don't use images with different aspect ratios.</Caption>
</Dont>
</DoDontContainer>

When presenting more than four pillars, we recommend displaying them in multiple rows. You can use the [Grid](/components/Grid) or the [Stack](/components/Stack) component to achieve this. We recommend using a grid with three or four columns and balancing the number of pillars on each row.

![An image showing a group of numerous Pillars stacked in a 3 column grid.](https://github.com/primer/brand/assets/912236/350c5eab-bd21-450c-a7a3-60e29c828a3f)
![An image showing a group of numerous Pillars stacked in a three column grid.](https://github.com/primer/brand/assets/912236/350c5eab-bd21-450c-a7a3-60e29c828a3f)

### Pillar and Card

Expand All @@ -71,18 +92,18 @@ Cards should be used for noteworthy information to immediately draw user attenti

#### Group size

Cards should be used in small groups (3, 4 or 5 items maximum), whereas Pillars can be used in larger (more than 5 items) or smaller groups. As users encounter an increasing number of items within a group, the distinctiveness of each individual element diminishes.
Cards should be used in small groups of three to five items, while Pillars can accommodate larger groups of up to nine items, or six if using images. The more items in a group, the less distinctive each individual element becomes.

<DoDontContainer>
<Do>
<img src="https://github.com/primer/brand/assets/912236/bc33c846-71c2-4af6-a3ac-f886bad71995" />
<Caption>Pillars are more suitable for groups of 5 or more items.</Caption>
<Caption>Pillars are suitable for groups of up to six items.</Caption>
</Do>
<Dont>
<img src="https://github.com/primer/brand/assets/912236/302dbc4d-6199-4c0c-96d5-76ac2e35fdea" />
<Caption>
Don't use Cards for groups of 5 or more items. Use them sparingly in
groups of 3 or 4.
Don't use Cards for groups of five or more items. Use them sparingly in
groups of three or four.
</Caption>
</Dont>
</DoDontContainer>
Expand Down
22 changes: 22 additions & 0 deletions apps/docs/content/components/Pillar/react.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,24 @@ You can add an `icon` to enhance the visual context. We recommend using an [Octi
</Pillar>
```

### Image

You can add an image to enhance the visual context.

```jsx live
<Pillar>
<Pillar.Image
src="https://via.placeholder.com/600x400/d3d9df/d3d9df.png"
alt="placeholder, blank area with an gray background color"
/>
<Pillar.Heading>Code search & code view</Pillar.Heading>
<Pillar.Description>
Enables you to rapidly search, navigate, and understand code, right from
GitHub.com.
</Pillar.Description>
</Pillar>
```

### Link

You can add an external link to the Pillar using the `Link` component.
Expand Down Expand Up @@ -130,6 +148,10 @@ Use the `Stack` component to stack multiple Pillars horizontally or vertically.
| `icon` | `React.Node` | | `true` | Octicon |
| `color` | <PropTableValues values={PillarIconColors} addLineBreaks /> | `default` | `false` | The color of the Pillar icon |

### Pillar.Image

Forwards all the props from the [Image component](/components/Image), including `src`, `alt`, and `aspectRatio`.

### Pillar.Heading

| name | type | default | required | description |
Expand Down
21 changes: 19 additions & 2 deletions packages/react/src/Pillar/Pillar.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import startShape from '../fixtures/images/testimonial-bg-1.png'
import endShape from '../fixtures/images/testimonial-bg-2.png'

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

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

export const Icon: StoryFn<typeof Pillar> = () => {
export const WithIcon: StoryFn<typeof Pillar> = () => {
return (
<Pillar>
<Pillar.Icon icon={<RocketIcon />} />
Expand All @@ -26,7 +27,23 @@ export const Icon: StoryFn<typeof Pillar> = () => {
)
}

export const IconColors: StoryFn<typeof Pillar> = () => {
export const WithImage: StoryFn<typeof Pillar> = () => {
return (
<Pillar>
<Pillar.Image
aspectRatio="16:10"
danielguillan marked this conversation as resolved.
Show resolved Hide resolved
src={placeholderImage}
alt="placeholder, blank area with a gray background color"
/>
<Pillar.Heading>Code search & code view</Pillar.Heading>
<Pillar.Description>
Enables you to rapidly search, navigate, and understand code, right from GitHub.com.
</Pillar.Description>
</Pillar>
)
}

export const WithIconColors: StoryFn<typeof Pillar> = () => {
return (
<Grid>
{PillarIconColors.map((color, id) => {
Expand Down
11 changes: 11 additions & 0 deletions packages/react/src/Pillar/Pillar.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@
text-align: left;
}

.Pillar__image {
margin-bottom: var(--base-size-28);
border-radius: var(--brand-borderRadius-medium);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All border-radius for cards, bento's etc are supposedly increasing to xlarge. Can we apply that now? Maybe also sneak in the same change into Card too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer that we handle this in a dedicated PR to ensure all components that need updating are addressed.

Copy link
Collaborator

@rezrah rezrah Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, mentioning as I can see you've made a padding update to the Card in this PR already, so felt okay to do it in this PR too? Agree other components like Bento can wait. Fine if you want to defer it also.

overflow: hidden;
}

.Pillar__image img,
.Pillar__image span {
display: block;
}

.Pillar__icon {
display: flex;
margin-bottom: var(--base-size-24);
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/Pillar/Pillar.module.css.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ declare const styles: {
readonly "Pillar": string;
readonly "Pillar--align-center": string;
readonly "Pillar--align-start": string;
readonly "Pillar__image": string;
readonly "Pillar__icon": string;
readonly "Pillar__heading": string;
readonly "Pillar__description": string;
Expand Down
18 changes: 18 additions & 0 deletions packages/react/src/Pillar/Pillar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,24 @@ describe('Pillar', () => {
expect(pillarEl).toHaveClass(classToCheck)
})

it('renders the image correctly into the document', () => {
const mockTestId = 'test'
danielguillan marked this conversation as resolved.
Show resolved Hide resolved
const mockImage = 'mock.png'
const classToCheck = 'Image'
const testAltText = 'alternative text'

const {getByAltText} = render(
<Pillar data-testid={mockTestId}>
<Pillar.Image src={mockImage} alt={testAltText} />
<Pillar.Heading>{mockHeading}</Pillar.Heading>
<Pillar.Description>{mockDescription}</Pillar.Description>
</Pillar>,
)

const pillarEl = getByAltText(testAltText)
expect(pillarEl).toHaveClass(classToCheck)
})

it('renders the link correctly into the document', () => {
const mockTestId = 'test'
const classToCheckLink = 'Pillar__link'
Expand Down
14 changes: 13 additions & 1 deletion packages/react/src/Pillar/Pillar.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, {forwardRef, PropsWithChildren, HTMLAttributes, type Ref} from 'react'
import clsx from 'clsx'
import {Heading, HeadingProps, Text, Link, LinkProps} from '..'
import {Heading, HeadingProps, Text, Image, type ImageProps, Link, LinkProps} from '..'
import type {BaseProps} from '../component-helpers'
import {Colors} from '../constants'
import {useAnimation} from '../animation'
Expand Down Expand Up @@ -40,6 +40,7 @@ const PillarRoot = forwardRef(
const filteredChildren = React.Children.toArray(children).filter(child => {
if (React.isValidElement(child) && typeof child.type !== 'string') {
if (
child.type === PillarImage ||
child.type === PillarIcon ||
child.type === PillarHeading ||
child.type === PillarDescription ||
Expand Down Expand Up @@ -67,6 +68,16 @@ const PillarRoot = forwardRef(
},
)

type PillarImageProps = ImageProps

function PillarImage({className, ...rest}: PillarImageProps) {
return (
<div className={styles.Pillar__image}>
<Image className={className} {...rest} />
</div>
)
}

type PillarIconProps = BaseProps<HTMLSpanElement> & {
icon: React.ReactNode | IconProps
color?: (typeof PillarIconColors)[number]
Expand Down Expand Up @@ -148,6 +159,7 @@ const PillarLink = forwardRef(({className, children, href, ...props}: PillarLink
*/
export const Pillar = Object.assign(PillarRoot, {
Icon: PillarIcon,
Image: PillarImage,
Heading: PillarHeading,
Description: PillarDescription,
Link: PillarLink,
Expand Down
17 changes: 13 additions & 4 deletions packages/react/src/Pillar/Pillar.visual.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,24 @@ test.describe('Visual Comparison: Pillar', () => {
expect(await page.screenshot({fullPage: true})).toMatchSnapshot()
})

test('Pillar / Icon', async ({page}) => {
await page.goto('http://localhost:6006/iframe.html?args=&id=components-pillar-features--icon&viewMode=story')
test('Pillar / With Icon', async ({page}) => {
await page.goto('http://localhost:6006/iframe.html?args=&id=components-pillar-features--with-icon&viewMode=story')

await page.waitForTimeout(500)
expect(await page.screenshot({fullPage: true})).toMatchSnapshot()
})

test('Pillar / Icon Colors', async ({page}) => {
await page.goto('http://localhost:6006/iframe.html?args=&id=components-pillar-features--icon-colors&viewMode=story')
test('Pillar / With Image', async ({page}) => {
await page.goto('http://localhost:6006/iframe.html?args=&id=components-pillar-features--with-image&viewMode=story')

await page.waitForTimeout(500)
expect(await page.screenshot({fullPage: true})).toMatchSnapshot()
})

test('Pillar / With Icon Colors', async ({page}) => {
await page.goto(
'http://localhost:6006/iframe.html?args=&id=components-pillar-features--with-icon-colors&viewMode=story',
)

await page.waitForTimeout(500)
expect(await page.screenshot({fullPage: true})).toMatchSnapshot()
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading