diff --git a/.changeset/navlist-linked-subnav.md b/.changeset/navlist-linked-subnav.md new file mode 100644 index 00000000000..c2ae5e0c8bc --- /dev/null +++ b/.changeset/navlist-linked-subnav.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +NavList: Support linked sub-nav parent items with a separate toggle diff --git a/e2e/components/NavList.test.ts b/e2e/components/NavList.test.ts index e74bf0e8c06..a63af1b2945 100644 --- a/e2e/components/NavList.test.ts +++ b/e2e/components/NavList.test.ts @@ -42,6 +42,48 @@ const stories = [ ] test.describe('NavList', () => { + for (const colorScheme of ['light', 'dark', 'light_high_contrast']) { + test(`linked sub-nav parent keyboard interaction @avt ${colorScheme}`, async ({page}) => { + await visit(page, { + id: 'components-navlist-features--with-linked-sub-nav-parent', + globals: {colorScheme}, + }) + + const link = page.getByRole('link', {name: 'Overview', exact: true}) + const toggle = page.getByRole('button', {name: 'Toggle overview pages', exact: true}) + const subNav = page.getByRole('list', {name: 'Overview'}) + const initialUrl = page.url() + + await expect(link).toHaveAttribute('aria-current', 'page') + await expect(toggle).toHaveAttribute('aria-expanded', 'false') + await expect(subNav).toBeHidden() + await expect(page).toHaveNoViolations() + + await link.focus() + await page.keyboard.press('Tab') + await expect(toggle).toBeFocused() + await page.keyboard.press('Enter') + await expect(toggle).toHaveAttribute('aria-expanded', 'true') + await expect(subNav).toBeVisible() + await expect(page).toHaveURL(initialUrl) + await expect(page).toHaveNoViolations() + + await page.keyboard.press('Tab') + await expect(page.getByRole('link', {name: 'Review requested'})).toBeFocused() + await page.keyboard.press('Shift+Tab') + await page.keyboard.press('Space') + await expect(toggle).toBeFocused() + await expect(toggle).toHaveAttribute('aria-expanded', 'false') + await expect(subNav).toBeHidden() + + await page.keyboard.press('Shift+Tab') + await expect(link).toBeFocused() + await page.keyboard.press('Enter') + await expect(page).toHaveURL(`${initialUrl}#`) + await expect(toggle).toHaveAttribute('aria-expanded', 'false') + }) + } + for (const story of stories) { test.describe(story.title, () => { for (const theme of themes) { diff --git a/packages/react/src/ActionList/ActionList.module.css b/packages/react/src/ActionList/ActionList.module.css index 7a4a9065c0a..b1076712ddf 100644 --- a/packages/react/src/ActionList/ActionList.module.css +++ b/packages/react/src/ActionList/ActionList.module.css @@ -115,7 +115,7 @@ border-radius: var(--borderRadius-medium); /* apply flex if trailing action exists as an immediate child */ - &[data-has-trailing-action] { + &[data-has-trailing-action]:not([data-has-subitem='true']) { display: flex; flex-wrap: nowrap; } @@ -497,6 +497,43 @@ /* button or a tag */ +.ActionListItemRow { + position: relative; + display: flex; + border-radius: var(--borderRadius-medium); + + :where(.ActionListItem[data-active='true']) > & { + background: var(--control-transparent-bgColor-selected); + outline: 2px solid transparent; + + & .ItemLabel { + font-weight: var(--base-text-weight-semibold); + color: var(--control-fgColor-rest); + } + + & .ActionListSubContent::before { + visibility: hidden; + } + + &::after { + @mixin activeIndicatorLine; + } + } + + @media (hover: hover) { + &:hover { + background-color: var(--control-transparent-bgColor-hover); + outline: solid var(--borderWidth-thin) transparent; + outline-offset: calc(-1 * var(--borderWidth-thin)); + box-shadow: var(--boxShadow-thin) var(--control-transparent-borderColor-active); + } + } + + &:active { + background-color: var(--control-transparent-bgColor-active); + } +} + /* [ [spacer] [leadingAction] [leadingVisual] [content] ] */ .ActionListContent { --subitem-depth: 0px; diff --git a/packages/react/src/ActionList/Item.tsx b/packages/react/src/ActionList/Item.tsx index f811925983f..53aba1fa5be 100644 --- a/packages/react/src/ActionList/Item.tsx +++ b/packages/react/src/ActionList/Item.tsx @@ -334,67 +334,69 @@ const UnwrappedItem = ( data-trailing-action-loading={trailingActionRendered && slots.trailingAction?.props.loading ? true : undefined} className={clsx(classes.ActionListItem, className)} > - - - {/* Reset TooltipContext so that child components don't detect - the ConditionalTooltip and suppress their own internal tooltips. */} - - - - - {slots.leadingVisual} - - {/* TODO: next-major: change to data-component="ActionList.Item.DividerContainer" next major version */} - - - - {childrenWithoutSlots} - {/* Loading message needs to be in here so it is read with the label */} - {/* If the item is inactive, we do not simultaneously announce that it is loading */} - {loading === true && !inactive && Loading} - - {slots.description} - + + + + {/* Reset TooltipContext so that child components don't detect + the ConditionalTooltip and suppress their own internal tooltips. */} + + + - {trailingVisual} + {slots.leadingVisual} - - { - // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel), - // render the inactive warning message directly in the item. - !showInactiveIndicator && inactiveText ? ( - - {inactiveText} + {/* TODO: next-major: change to data-component="ActionList.Item.DividerContainer" next major version */} + + + + {childrenWithoutSlots} + {/* Loading message needs to be in here so it is read with the label */} + {/* If the item is inactive, we do not simultaneously announce that it is loading */} + {loading === true && !inactive && Loading} - ) : null - } - - - - - {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction} + {slots.description} + + + {trailingVisual} + + + { + // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel), + // render the inactive warning message directly in the item. + !showInactiveIndicator && inactiveText ? ( + + {inactiveText} + + ) : null + } + + + + + {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction} + {slots.subItem} diff --git a/packages/react/src/NavList/NavList.docs.json b/packages/react/src/NavList/NavList.docs.json index 91826f957ff..c5cff2c32ef 100644 --- a/packages/react/src/NavList/NavList.docs.json +++ b/packages/react/src/NavList/NavList.docs.json @@ -15,6 +15,7 @@ {"id": "components-navlist-features--with-group-expand"}, {"id": "components-navlist-features--with-group-heading-links"}, {"id": "components-navlist-features--with-inactive-items"}, + {"id": "components-navlist-features--with-linked-sub-nav-parent"}, {"id": "components-navlist-features--with-nested-sub-items"}, {"id": "components-navlist-features--with-next-js-link"}, {"id": "components-navlist-features--with-react-router-link"}, @@ -101,7 +102,7 @@ "name": "href", "type": "string", "defaultValue": "", - "description": "The URL that the item navigates to. `href` is passed to the underlying `` element. If `as` is specified, the component may need different props. If the item contains a sub-nav, the item is rendered as a `