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

PageHeader: Address general sign-off review feedback in storybook (missing href values, non-working menus) #3817

Merged
merged 10 commits into from
Oct 17, 2023
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
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
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
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
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
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
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
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
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
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
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
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
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
207 changes: 161 additions & 46 deletions src/PageHeader/PageHeader.examples.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ import {
ChecklistIcon,
FileDiffIcon,
ArrowRightIcon,
TriangleDownIcon,
CheckIcon,
} from '@primer/octicons-react'

import {PageHeader} from './PageHeader'
import {Hidden} from '../Hidden'
import {UnderlineNav} from '../UnderlineNav2'
import {ActionMenu} from '../ActionMenu'
import {ActionList} from '../ActionList'
import VisuallyHidden from '../_VisuallyHidden'

const meta: Meta = {
title: 'Drafts/Components/PageHeader/Examples',
Expand Down Expand Up @@ -66,17 +71,26 @@ export const PullRequestPage = () => (
<PageHeader.ParentLink href="http://github.com">Pull requests</PageHeader.ParentLink>
</PageHeader.ContextArea>
<PageHeader.TitleArea>
<PageHeader.Title as="h2">
<PageHeader.Title as="h1">
PageHeader component initial layout explorations extra long pull request title
</PageHeader.Title>
<PageHeader.Actions>
<Hidden when={['regular', 'wide']}>
<IconButton aria-label="More" icon={KebabHorizontalIcon} />
{/* Pop up actions */}
<ActionMenu>
<ActionMenu.Anchor>
<IconButton aria-label="More pull request actions" icon={KebabHorizontalIcon} />
</ActionMenu.Anchor>
<ActionMenu.Overlay width="small">
<ActionList>
<ActionList.Item onSelect={() => alert('Edit button action')}>Edit</ActionList.Item>
<ActionList.Item onSelect={() => alert('Code button action')}>Code</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</Hidden>

<Hidden when={['narrow']}>
<Box sx={{display: 'flex', gap: 2}}>
<Box sx={{display: 'flex'}}>
<Button>Edit</Button>
<Button leadingIcon={CodeIcon}>Code</Button>
</Box>
Expand All @@ -87,18 +101,18 @@ export const PullRequestPage = () => (
<StateLabel status="pullOpened">Open</StateLabel>
<Hidden when={['narrow']}>
<Text sx={{fontSize: 1, color: 'fg.muted'}}>
<Link href="#" muted sx={{fontWeight: 'bold'}}>
<Link href="https://github.com/broccolinisoup" sx={{fontWeight: 'bold'}}>
broccolinisoup
</Link>{' '}
wants to merge 3 commits into <BranchName href="#">main</BranchName> from{' '}
<BranchName href="#">broccolinisoup/switch-to-new-underlineNav</BranchName>
wants to merge 3 commits into <BranchName href="https://github.com/primer/react">main</BranchName> from{' '}
<BranchName href="https://github.com/primer/react">bs/pageheader-title</BranchName>
</Text>
</Hidden>
<Hidden when={['regular', 'wide']}>
<Text sx={{fontSize: 1, color: 'fg.muted'}}>
<BranchName href="#">main</BranchName>
<BranchName href="https://github.com/primer/react">main</BranchName>
<ArrowRightIcon />
<BranchName href="#">page-header-initial</BranchName>
<BranchName href="https://github.com/primer/react">page-header-initial</BranchName>
</Text>
</Hidden>
</PageHeader.Description>
Expand Down Expand Up @@ -134,21 +148,120 @@ export const FilesPage = () => (
<PageHeader.ContextArea>
<PageHeader.ParentLink>Files</PageHeader.ParentLink>
<PageHeader.ContextAreaActions>
<Button size="small" leadingIcon={GitBranchIcon}>
Main
</Button>
<IconButton size="small" aria-label="More" icon={KebabHorizontalIcon} />
<ActionMenu>
<ActionMenu.Anchor>
<Button size="small" leadingIcon={GitBranchIcon} trailingIcon={TriangleDownIcon}>
Main
</Button>
</ActionMenu.Anchor>
<ActionMenu.Overlay width="medium">
<ActionList>
<ActionList.Item onSelect={() => alert('Main')}>
<ActionList.LeadingVisual>
<CheckIcon />
</ActionList.LeadingVisual>
main <ActionList.TrailingVisual>default</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Branch 1')}>branch-1</ActionList.Item>
<ActionList.Item onSelect={() => alert('Branch 2')}>branch-2</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>

<ActionMenu>
<ActionMenu.Anchor>
<IconButton size="small" aria-label="More file actions" icon={KebabHorizontalIcon} />
</ActionMenu.Anchor>
<ActionMenu.Overlay width="medium">
<ActionList>
<ActionList.Group title="Raw file content">
<ActionList.Item onSelect={() => alert('Download')}>Download</ActionList.Item>
</ActionList.Group>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Jump to line')}>
Jump to line
<ActionList.TrailingVisual>L</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Copy path')}>
Copy path
<ActionList.TrailingVisual>⌘⇧.</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Copy permalink')}>
Copy permalink
<ActionList.TrailingVisual>⌘⇧,</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Group title="View Options">
<ActionList.Item onSelect={() => alert('Show code folding buttons')}>
Show code folding buttons
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Wrap lines')}>Wrap lines</ActionList.Item>
<ActionList.Item onSelect={() => alert('Center content')}>Center content</ActionList.Item>
</ActionList.Group>
<ActionList.Divider />
<ActionList.Item variant="danger" onSelect={() => alert('Delete file clicked')}>
Delete file
<ActionList.TrailingVisual>⌘D</ActionList.TrailingVisual>
</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</PageHeader.ContextAreaActions>
</PageHeader.ContextArea>
<PageHeader.TitleArea>
<Breadcrumbs>
<Breadcrumbs.Item href="#">...</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">primer</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">react</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">src</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">PageHeader</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">PageHeader.tsx</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main">react</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main/src">src</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main/src/PageHeader">
PageHeader
</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/blob/main/src/PageHeader/PageHeader.tsx">
PageHeader.tsx
</Breadcrumbs.Item>
</Breadcrumbs>
<VisuallyHidden as="h2">PageHeader.tsx</VisuallyHidden>
<PageHeader.Actions hidden={{narrow: true}}>
<ActionMenu>
<ActionMenu.Anchor>
<IconButton size="small" aria-label="More file actions" icon={KebabHorizontalIcon} />
</ActionMenu.Anchor>
<ActionMenu.Overlay width="medium">
<ActionList>
<ActionList.Group title="Raw file content">
<ActionList.Item onSelect={() => alert('Download')}>Download</ActionList.Item>
</ActionList.Group>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Jump to line')}>
Jump to line
<ActionList.TrailingVisual>L</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Copy path')}>
Copy path
<ActionList.TrailingVisual>⌘⇧.</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Copy permalink')}>
Copy permalink
<ActionList.TrailingVisual>⌘⇧,</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Group title="View Options">
<ActionList.Item onSelect={() => alert('Show code folding buttons')}>
Show code folding buttons
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Wrap lines')}>Wrap lines</ActionList.Item>
<ActionList.Item onSelect={() => alert('Center content')}>Center content</ActionList.Item>
</ActionList.Group>
<ActionList.Divider />
<ActionList.Item variant="danger" onSelect={() => alert('Delete file clicked')}>
Delete file
<ActionList.TrailingVisual>⌘D</ActionList.TrailingVisual>
</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</PageHeader.Actions>
</PageHeader.TitleArea>
</PageHeader>
</Box>
Expand All @@ -167,14 +280,24 @@ export const WithPageLayout = () => {
<PageHeader.ParentLink href="http://github.com">Pull requests</PageHeader.ParentLink>
</PageHeader.ContextArea>
<PageHeader.TitleArea>
<PageHeader.Title as="h2">
<PageHeader.Title as="h1">
PageHeader component initial layout explorations extra long pull request title &nbsp;
<Text sx={{color: 'fg.muted', fontWeight: 'light'}}>#1831</Text>
</PageHeader.Title>
<PageHeader.Actions>
<Hidden when={['regular', 'wide']}>
<IconButton aria-label="More" icon={KebabHorizontalIcon} />
{/* Pop up actions */}
<ActionMenu>
<ActionMenu.Anchor>
<IconButton aria-label="More pull request actions" icon={KebabHorizontalIcon} />
</ActionMenu.Anchor>
<ActionMenu.Overlay width="small">
<ActionList>
<ActionList.Item onSelect={() => alert('Edit button action')}>Edit</ActionList.Item>
<ActionList.Item onSelect={() => alert('Code button action')}>Code</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</Hidden>

<Hidden when={['narrow']}>
Expand All @@ -189,18 +312,20 @@ export const WithPageLayout = () => {
<StateLabel status="pullOpened">Open</StateLabel>
<Hidden when={['narrow']}>
<Text sx={{fontSize: 1, color: 'fg.muted'}}>
<Link href="#" muted sx={{fontWeight: 'bold'}}>
<Link href="https://github.com/broccolinisoup" sx={{fontWeight: 'bold'}}>
broccolinisoup
</Link>{' '}
wants to merge 3 commits into <BranchName href="#">main</BranchName> from{' '}
<BranchName href="#">broccolinisoup/switch-to-new-underlineNav</BranchName>
wants to merge 3 commits into <BranchName href="https://github.com/primer/react">main</BranchName> from{' '}
<BranchName href="https://github.com/primer/react">
broccolinisoup/switch-to-new-underlineNav
</BranchName>
</Text>
</Hidden>
<Hidden when={['regular', 'wide']}>
<Text sx={{fontSize: 1, color: 'fg.muted'}}>
<BranchName href="#">main</BranchName>
<BranchName href="https://github.com/primer/react">main</BranchName>
<ArrowRightIcon />
<BranchName href="#">page-header-initial</BranchName>
<BranchName href="https://github.com/primer/react">page-header-initial</BranchName>
</Text>
</Hidden>
</PageHeader.Description>
Expand All @@ -224,32 +349,22 @@ export const WithPageLayout = () => {
</PageLayout.Header>
<PageLayout.Content>
<Box sx={{border: '1px solid', borderRadius: 2, borderColor: 'border.default', height: 200}}></Box>
<Box
sx={{
maxWidth: '100%',
overflowX: 'auto',
border: '1px solid',
whiteSpace: 'nowrap',
borderColor: 'border.default',
mt: 3,
p: 3,
borderRadius: 2,
}}
tabIndex={0}
>
This box has really long content. If it is too long, it will cause x overflow and should show a scrollbar.
When this overflows, it should not break to overall page layout!
</Box>
</PageLayout.Content>
<PageLayout.Pane>
<Box sx={{display: 'flex', flexDirection: 'column', gap: 3}}>
<Box>
<Text sx={{fontSize: 0, fontWeight: 'bold', display: 'block', color: 'fg.muted'}}>Assignees</Text>
<Text sx={{fontSize: 0, color: 'fg.muted', lineHeight: 'condensed'}}>
No one –{' '}
<Link href="#" muted>
<Text sx={{fontSize: 0, color: 'fg.muted', lineHeight: 'condensed', display: 'flex', alignItems: 'center'}}>
No one —
<Button
variant="invisible"
onClick={() => {
alert('Assign yourself')
}}
sx={{color: 'fg.muted'}}
>
assign yourself
</Link>
</Button>
</Text>
</Box>
<Box role="separator" sx={{width: '100%', height: 1, backgroundColor: 'border.default'}}></Box>
Expand Down
41 changes: 22 additions & 19 deletions src/PageHeader/PageHeader.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ export const WithComponentAsATitle = () => (
<PageHeader>
<PageHeader.TitleArea>
<Breadcrumbs>
<Breadcrumbs.Item href="#">...</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">primer</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">react</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">src</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">PageHeader</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">PageHeader.tsx</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main">react</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main/src">src</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main/src/PageHeader">
PageHeader
</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/blob/main/src/PageHeader/PageHeader.tsx">
PageHeader.tsx
</Breadcrumbs.Item>
</Breadcrumbs>
<VisuallyHidden as="h2">Visually Hidden Title</VisuallyHidden>
<VisuallyHidden as="h2">PageHeader.tsx</VisuallyHidden>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be h1 as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah my bad. I just read your replies to @jscholes. I see this is intentional

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh good catch! @pksjce

Just to clarify, the h2 intentional ones I mentioned in my response are the ones under the settings page (reference)

Looking at the current files page implementation, seems like it has changed since I built this component and stories and it should be h1. However, we will address the cases where breadcrumb components are used as a title in the following PageHeader PR and the structure will change a bit, so I think we can leave this as is to reduce the number of merge conflicts if that is okay?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes! That seems fine with me. This one is good to merge 👍

</PageHeader.TitleArea>
</PageHeader>
</Box>
Expand Down Expand Up @@ -131,7 +133,7 @@ export const WithDescriptionSlot = () => (
</PageHeader.TitleArea>
<PageHeader.Description>
<Text sx={{fontSize: 1, color: 'fg.muted'}}>
<Link href="#" muted sx={{fontWeight: 'bold'}}>
<Link href="https://github.com/broccolinisoup" sx={{fontWeight: 'bold'}}>
broccolinisoup
</Link>{' '}
created this branch 5 days ago · 14 commits · updated today
Expand Down Expand Up @@ -175,12 +177,12 @@ export const WithCustomNavigation = () => (
<PageHeader.Navigation as="nav" aria-label="Item list">
<Box as="ul" sx={{display: 'flex', gap: '8px', listStyle: 'none', paddingY: 0, paddingX: 3}} role="list">
<li>
<Link href="#" aria-current="page">
<Link href="https://github.com/primer/react" aria-current="page">
Item 1
</Link>
</li>
<li>
<Link href="#">Item 2</Link>
<Link href="https://github.com/primer/react/pulls">Item 2</Link>
</li>
</Box>
</PageHeader.Navigation>
Expand Down Expand Up @@ -211,8 +213,8 @@ export const WithParentLinkAndActionsOfContextArea = () => (
<PageHeader.ParentLink href="http://github.com">Parent Link</PageHeader.ParentLink>

<PageHeader.ContextAreaActions>
<Button size="small" leadingIcon={GitBranchIcon}>
Main
<Button size="small" trailingAction={TriangleDownIcon}>
Add File
</Button>
<IconButton size="small" aria-label="More Options" icon={KebabHorizontalIcon} />
</PageHeader.ContextAreaActions>
Expand All @@ -236,15 +238,16 @@ export const WithContextBarAndActionsOfContextArea = () => (
<PageHeader.ContextArea>
<PageHeader.ContextBar>
<Breadcrumbs>
<Breadcrumbs.Item href="#">...</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">primer</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">react</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">src</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">PageHeader</Breadcrumbs.Item>
<Breadcrumbs.Item href="#">PageHeader.tsx</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main">react</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main/src">src</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/tree/main/src/PageHeader">
PageHeader
</Breadcrumbs.Item>
<Breadcrumbs.Item href="https://github.com/primer/react/blob/main/src/PageHeader/PageHeader.tsx">
PageHeader.tsx
</Breadcrumbs.Item>
</Breadcrumbs>
</PageHeader.ContextBar>

<PageHeader.ContextAreaActions>
<Button size="small" leadingIcon={GitBranchIcon}>
Main
Expand Down
Loading
Loading