Skip to content

Commit

Permalink
add heading
Browse files Browse the repository at this point in the history
  • Loading branch information
langermank committed Dec 10, 2024
1 parent 6dfcba7 commit e1e1e85
Show file tree
Hide file tree
Showing 6 changed files with 319 additions and 197 deletions.
35 changes: 19 additions & 16 deletions packages/react/src/ActionList/ActionList.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,21 @@
color: var(--fgColor-muted);
}
}

/* hide dividers */
@media (hover: hover) {
&:hover {
& .ActionListSubContent::before,
& + .ActionListItem .ActionListSubContent::before {
visibility: hidden;
}

& [data-description-variant='inline']::before,
& + .ActionListItem [data-description-variant='inline']::before {
visibility: hidden;
}
}
}
}

/* if item has subitem, move hover styles to ActionListContent */
Expand Down Expand Up @@ -255,21 +270,6 @@
}
}

/* hide dividers */
@media (hover: hover) {
&:hover {
& .ActionListSubContent::before,
& + .ActionListItem .ActionListSubContent::before {
visibility: hidden;
}

& [data-description-variant='inline']::before,
& + .ActionListItem [data-description-variant='inline']::before {
visibility: hidden;
}
}
}

/* Make sure that the first visible item isn't a divider */
&[aria-hidden] + .Divider {
display: none;
Expand Down Expand Up @@ -385,6 +385,7 @@

&:hover {
text-decoration: none;
cursor: pointer;
}

/* collapsible item [aria-expanded] */
Expand Down Expand Up @@ -535,8 +536,10 @@

& .Description {
/* adjust line-height for baseline alignment */

/* line-height: calc(var(--control-medium-lineBoxHeight) - var(--base-size-2)); */
/* stylelint-disable-next-line primer/typography */
line-height: calc(var(--control-medium-lineBoxHeight) - var(--base-size-2));
line-height: 16px;
}
}
}
Expand Down
129 changes: 1 addition & 128 deletions packages/react/src/ActionList/ActionList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import theme from '../theme'
import {ActionList} from '.'
import {BookIcon} from '@primer/octicons-react'
import {behavesAsComponent, checkExports} from '../utils/testing'
import {BaseStyles, ThemeProvider, ActionMenu} from '..'
import {BaseStyles, ThemeProvider} from '..'
import {FeatureFlags} from '../FeatureFlags'

function SimpleActionList(): JSX.Element {
Expand Down Expand Up @@ -237,133 +237,6 @@ describe('ActionList', () => {
expect(onClick).toHaveBeenCalled()
})

it('should throw an error when ActionList.GroupHeading has an `as` prop when it is used within ActionMenu context', async () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
expect(() =>
HTMLRender(
<ThemeProvider theme={theme}>
<BaseStyles>
<ActionMenu open={true}>
<ActionMenu.Button>Trigger</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Group>
<ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</BaseStyles>
</ThemeProvider>,
),
).toThrow(
"Looks like you are trying to set a heading level to a menu role. Group headings for menu type action lists are for representational purposes, and rendered as divs. Therefore they don't need a heading level.",
)
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})

it('should render the ActionList.GroupHeading component as a heading with the given heading level', async () => {
const container = HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>,
)
const heading = container.getByRole('heading', {level: 2})
expect(heading).toBeInTheDocument()
expect(heading).toHaveTextContent('Group Heading')
})
it('should throw an error if ActionList.GroupHeading is used without an `as` prop when no role is specified (for list role)', async () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
expect(() =>
HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
),
).toThrow(
"You are setting a heading for a list, that requires a heading level. Please use 'as' prop to set a proper heading level.",
)
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})
it('should render the ActionList.GroupHeading component as a span (not a heading tag) when role is specified as listbox', async () => {
const container = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>,
)
const label = container.getByText('Group Heading')
expect(label).toBeInTheDocument()
expect(label.tagName).toEqual('SPAN')
})
it('should render the ActionList.GroupHeading component as a span with role="presentation" and aria-hidden="true" when role is specified as listbox', async () => {
const container = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>,
)
const label = container.getByText('Group Heading')
const wrapper = label.parentElement
expect(wrapper).toHaveAttribute('role', 'presentation')
expect(wrapper).toHaveAttribute('aria-hidden', 'true')
})
it('should label the list with the group heading id', async () => {
const {container, getByText} = HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group data-test-id="ActionList.Group">
<ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
)
const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
const heading = getByText('Group Heading')
expect(list).toHaveAttribute('aria-labelledby', heading.id)
})
it('should NOT label the list with the group heading id when role is specified', async () => {
const {container, getByText} = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group data-test-id="ActionList.Group">
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
)
const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
const heading = getByText('Group Heading')
expect(list).not.toHaveAttribute('aria-labelledby', heading.id)
})
it('should label the list using aria-label when role is specified', async () => {
const {container, getByText} = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group data-test-id="ActionList.Group">
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
)
const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
const heading = getByText('Group Heading')
expect(list).toHaveAttribute('aria-label', heading.textContent)
})

it('should render ActionList.Item as button when feature flag is enabled', async () => {
const featureFlag = {
primer_react_action_list_item_as_button: true,
Expand Down
40 changes: 40 additions & 0 deletions packages/react/src/ActionList/Group.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
.Group:not(:first-child) {
margin-block-start: var(--base-size-8);
}

.GroupHeadingWrap {
display: flex;
font-size: var(--text-body-size-small);
font-weight: var(--base-text-weight-semibold);

/* line-height: var(--text-body-lineHeight-small); use when FF rolls out */
/* stylelint-disable-next-line primer/typography */
line-height: 18px;
color: var(--fgColor-muted);
flex-direction: column;
padding-inline: var(--base-size-8);
padding-block: var(--base-size-8);

&:where([data-variant='filled']) {
/* stylelint-disable-next-line primer/spacing */
margin-block-start: calc(var(--base-size-8) - var(--borderWidth-thin));
margin-block-end: var(--base-size-8);
margin-inline: calc(-1 * var(--base-size-8));
background: var(--bgColor-muted);
border-top: solid var(--borderWidth-thin) var(--borderColor-muted);
border-bottom: solid var(--borderWidth-thin) var(--borderColor-muted);
padding-inline: var(--base-size-16);

&:first-child {
margin-block-start: 0;
}
}
}

.GroupHeading {
margin: 0;
font-size: var(--text-body-size-small);
font-weight: var(--base-text-weight-semibold);
color: var(--fgColor-muted);
align-self: flex-start;
}
150 changes: 150 additions & 0 deletions packages/react/src/ActionList/Group.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import {render as HTMLRender} from '@testing-library/react'
import React from 'react'
import theme from '../theme'
import {ActionList} from '.'
import {BaseStyles, ThemeProvider, ActionMenu} from '..'
import {FeatureFlags} from '../FeatureFlags'

describe('ActionList.Group', () => {
it('should throw an error when ActionList.GroupHeading has an `as` prop when it is used within ActionMenu context', async () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
expect(() =>
HTMLRender(
<ThemeProvider theme={theme}>
<BaseStyles>
<ActionMenu open={true}>
<ActionMenu.Button>Trigger</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Group>
<ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</BaseStyles>
</ThemeProvider>,
),
).toThrow(
"Looks like you are trying to set a heading level to a menu role. Group headings for menu type action lists are for representational purposes, and rendered as divs. Therefore they don't need a heading level.",
)
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})

it('should render the ActionList.GroupHeading component as a heading with the given heading level', async () => {
const container = HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>,
)
const heading = container.getByRole('heading', {level: 2})
expect(heading).toBeInTheDocument()
expect(heading).toHaveTextContent('Group Heading')
})
it('should throw an error if ActionList.GroupHeading is used without an `as` prop when no role is specified (for list role)', async () => {
const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
expect(() =>
HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
),
).toThrow(
"You are setting a heading for a list, that requires a heading level. Please use 'as' prop to set a proper heading level.",
)
expect(spy).toHaveBeenCalled()
spy.mockRestore()
})
it('should render the ActionList.GroupHeading component as a span (not a heading tag) when role is specified as listbox', async () => {
const container = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>,
)
const label = container.getByText('Group Heading')
expect(label).toBeInTheDocument()
expect(label.tagName).toEqual('SPAN')
})
it('should render the ActionList.GroupHeading component as a span with role="presentation" and aria-hidden="true" when role is specified as listbox', async () => {
const container = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>,
)
const label = container.getByText('Group Heading')
const wrapper = label.parentElement
expect(wrapper).toHaveAttribute('role', 'presentation')
expect(wrapper).toHaveAttribute('aria-hidden', 'true')
})
it('should label the list with the group heading id', async () => {
const {container, getByText} = HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group data-test-id="ActionList.Group">
<ActionList.GroupHeading as="h2">Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
)
const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
const heading = getByText('Group Heading')
expect(list).toHaveAttribute('aria-labelledby', heading.id)
})
it('should NOT label the list with the group heading id when role is specified', async () => {
const {container, getByText} = HTMLRender(
<ActionList role="listbox">
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group data-test-id="ActionList.Group">
<ActionList.GroupHeading>Group Heading</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
)
const list = container.querySelector(`li[data-test-id='ActionList.Group'] > ul`)
const heading = getByText('Group Heading')
expect(list).not.toHaveAttribute('aria-labelledby', heading.id)
})

it('should support a custom `className` on the outermost element', () => {
const Element = () => {
return (
<ActionList>
<ActionList.Group>
<ActionList.GroupHeading as="h2" className="test-class-name">
Test
</ActionList.GroupHeading>
</ActionList.Group>
</ActionList>
)
}
const FeatureFlagElement = () => {
return (
<FeatureFlags
flags={{
primer_react_css_modules_team: true,
primer_react_css_modules_staff: true,
primer_react_css_modules_ga: true,
}}
>
<Element />
</FeatureFlags>
)
}
expect(HTMLRender(<FeatureFlagElement />).container.querySelector('h2')).toHaveClass('test-class-name')
expect(HTMLRender(<FeatureFlagElement />).container.querySelector('h2')).toHaveClass('test-class-name')
})
})
Loading

0 comments on commit e1e1e85

Please sign in to comment.