Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/tiny-mirrors-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Add `headingWrapElement` prop to `ActionList.GroupHeading` to support `div` or `li` elements for proper list markup
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.
112 changes: 112 additions & 0 deletions e2e/components/NavList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,116 @@ test.describe('NavList', () => {
})
}
})

test.describe('With Title and Heading', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist-devonly--with-group-title-and-heading',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`NavList.With Title and Heading.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist-devonly--with-group-title-and-heading',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('Simple', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist--simple',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`NavList.Simple.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist--simple',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('With Group', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-group',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`NavList.With Group.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-group',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})

test.describe('With Sub Items', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-sub-items',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`NavList.With Sub Items.${theme}.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-navlist--with-sub-items',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations()
})
})
}
})
})
7 changes: 5 additions & 2 deletions packages/react/src/ActionList/Group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export type ActionListGroupHeadingProps = Pick<ActionListGroupProps, 'variant' |
SxProp &
React.HTMLAttributes<HTMLElement> & {
as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
headingWrapElement?: 'div' | 'li'
_internalBackwardCompatibleTitle?: string
}

Expand All @@ -132,6 +133,7 @@ export const GroupHeading: React.FC<React.PropsWithChildren<ActionListGroupHeadi
children,
className,
sx = defaultSxProp,
headingWrapElement = 'div',
...props
}) => {
const {variant: listVariant, role: listRole} = React.useContext(ListContext)
Expand All @@ -158,6 +160,7 @@ export const GroupHeading: React.FC<React.PropsWithChildren<ActionListGroupHeadi
fontSize: 0,
fontWeight: 'bold',
color: 'fg.muted',
listStyle: 'none',
...(variant === 'filled' && {
backgroundColor: 'canvas.subtle',
marginX: 0,
Expand Down Expand Up @@ -186,13 +189,13 @@ export const GroupHeading: React.FC<React.PropsWithChildren<ActionListGroupHeadi
<>
{/* for listbox (SelectPanel) and menu (ActionMenu) roles, group titles are presentational. */}
{listRole && listRole !== 'list' ? (
<Box sx={styles} role="presentation" aria-hidden="true" {...props}>
<Box sx={styles} role="presentation" aria-hidden="true" {...props} as={headingWrapElement}>
<span id={groupHeadingId}>{_internalBackwardCompatibleTitle ?? children}</span>
{auxiliaryText && <div className="ActionListGroupHeadingDescription">{auxiliaryText}</div>}
</Box>
) : (
// for explicit (role="list" is passed as prop) and implicit list roles (ActionList ins rendered as list by default), group titles are proper heading tags.
<Box sx={styles}>
<Box sx={styles} as={headingWrapElement}>
<Heading
className={clsx(className, 'ActionListGroupHeading')}
as={as || 'h3'}
Expand Down
23 changes: 23 additions & 0 deletions packages/react/src/NavList/NavList.dev.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,26 @@ export const WithBadExampleOfSubNavAndTrailingAction = () => {
WithBadExampleOfSubNavAndTrailingAction.storyName = 'With SubNav and Trailing Action (Bad example, do not copy)'

export default meta

export const WithGroupTitleAndHeading = () => (
<PageLayout>
<PageLayout.Pane position="start">
<NavList>
<NavList.Group title="Group 1">
<NavList.GroupHeading>Hello</NavList.GroupHeading>
<NavList.Item aria-current="true" href="#">
Item 1A
</NavList.Item>
<NavList.Item href="#">Item 1B</NavList.Item>
<NavList.Item href="#">Item 1C</NavList.Item>
</NavList.Group>
<NavList.Group title="Group 2">
<NavList.Item href="#">Item 2A</NavList.Item>
<NavList.Item href="#">Item 2B</NavList.Item>
<NavList.Item href="#">Item 2C</NavList.Item>
</NavList.Group>
</NavList>
</PageLayout.Pane>
<PageLayout.Content></PageLayout.Content>
</PageLayout>
)
1 change: 1 addition & 0 deletions packages/react/src/NavList/NavList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ const GroupHeading: React.FC<NavListGroupHeadingProps> = ({as = 'h3', sx: sxProp
sxProp,
)}
data-component="NavList.GroupHeading"
headingWrapElement="li"
{...rest}
/>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ exports[`NavList renders with groups 1`] = `
font-size: 12px;
font-weight: 600;
color: var(--fgColor-muted,var(--color-fg-muted,#656d76));
list-style: none;
}

.c3 .ActionListGroupHeading {
Expand Down
Loading