Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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/early-ghosts-enter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
Comment thread
adierkens marked this conversation as resolved.
Outdated
---

Add support to ActionList for 'tablist' and 'tab' roles
44 changes: 44 additions & 0 deletions packages/react/src/ActionList/ActionList.dev.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import {Divider} from './Divider'
import {Description} from './Description'
import Avatar from '../Avatar'
import {FileDirectoryIcon, HeartFillIcon} from '@primer/octicons-react'
import {TabContainerElement} from '@github/tab-container-element'
import {Stack} from '../Stack'
import {AnchoredOverlay} from '../AnchoredOverlay'
import createComponent from '../utils/create-component'

export default {
title: 'Components/ActionList/Dev',
Expand Down Expand Up @@ -375,3 +377,45 @@ export const OverlayWrapping = () => {
</div>
)
}

const TabContainerComponent = createComponent(TabContainerElement, 'tab-container')

export const TabListRoles = () => (
<TabContainerComponent>
<ActionList role="tablist" aria-label="Tabs example">
<ActionList.Item role="tab">Tab 1</ActionList.Item>
<ActionList.Item role="tab">Tab 2</ActionList.Item>
<ActionList.Item role="tab" aria-selected>
Tab 3
</ActionList.Item>
</ActionList>
<div role="tabpanel" data-tab-container-no-tabstop>
Panel 1
</div>
<div role="tabpanel" data-tab-container-no-tabstop hidden>
Panel 2
</div>
<div role="tabpanel" data-tab-container-no-tabstop hidden>
Panel 3
</div>
</TabContainerComponent>
)

export const TabListInfersItemRole = () => (
<TabContainerComponent>
<ActionList role="tablist" aria-label="Tabs example">
<ActionList.Item>Tab 1</ActionList.Item>
<ActionList.Item>Tab 2</ActionList.Item>
<ActionList.Item>Tab 3</ActionList.Item>
</ActionList>
<div role="tabpanel" data-tab-container-no-tabstop>
Panel 1
</div>
<div role="tabpanel" data-tab-container-no-tabstop hidden>
Panel 2
</div>
<div role="tabpanel" data-tab-container-no-tabstop hidden>
Panel 3
</div>
</TabContainerComponent>
)
3 changes: 2 additions & 1 deletion packages/react/src/ActionList/ActionList.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@
}

/* active state [aria-current] */
&:where([data-active]) {
&:where([data-active]),
&:where([aria-selected='true']):not([data-selectable]) {
background: var(--control-transparent-bgColor-selected);

/* provides a visual indication of the current item for Windows high-contrast mode */
Expand Down
31 changes: 31 additions & 0 deletions packages/react/src/ActionList/Item.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,35 @@ describe('ActionList.Item', () => {
expect(item).toHaveTextContent('Item, Description')
expect(item).toHaveAccessibleDescription('Description')
})

it('should add role="tab" when ActionList has role="tablist"', () => {
const {getAllByRole} = HTMLRender(
<ActionList role="tablist">
<ActionList.Item>Tab 1</ActionList.Item>
<ActionList.Item>Tab 2</ActionList.Item>
<ActionList.Item>Tab 3</ActionList.Item>
</ActionList>,
)
const tabs = getAllByRole('tab')
expect(tabs[0]).toBeInTheDocument()
expect(tabs).toHaveLength(3)
})

it('should update stylings when aria-selected is added outside of React', () => {
const {getByRole} = HTMLRender(
<ActionList role="tablist">
<ActionList.Item>Tab 1</ActionList.Item>
</ActionList>,
)

const tab = getByRole('tab')
expect(tab).toBeInTheDocument()

const backgroundColorBefore = window.getComputedStyle(tab).backgroundColor
// Simulate outside of React setting aria-selected
tab.setAttribute('aria-selected', 'true')
const backgroundColorAfter = window.getComputedStyle(tab).backgroundColor
expect(backgroundColorBefore).not.toBe(backgroundColorAfter)
expect(tab).toHaveAttribute('aria-selected', 'true')
})
})
9 changes: 8 additions & 1 deletion packages/react/src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
else inferredItemRole = 'menuitem'
} else if (listRole === 'listbox') {
if (selectionVariant !== undefined && !role) inferredItemRole = 'option'
} else if (listRole === 'tablist') {
inferredItemRole = 'tab'
}

const itemRole = role || inferredItemRole
Expand All @@ -142,7 +144,11 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
const itemSelectionAttribute = selectionAttribute || inferredSelectionAttribute
// Ensures ActionList.Item retains list item semantics if a valid ARIA role is applied, or if item is inactive
const listItemSemantics =
role === 'option' || role === 'menuitem' || role === 'menuitemradio' || role === 'menuitemcheckbox'
itemRole === 'option' ||

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This just seemed like a bug. We weren't using the inferredItemRole when checking if the item is a list item

itemRole === 'menuitem' ||
itemRole === 'menuitemradio' ||
itemRole === 'menuitemcheckbox' ||
itemRole === 'tab'

const listRoleTypes = ['listbox', 'menu', 'list']
const listSemantics = (listRole && listRoleTypes.includes(listRole)) || inactive || listItemSemantics
Expand Down Expand Up @@ -249,6 +255,7 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
ref={listSemantics ? forwardedRef : null}
data-variant={variant === 'danger' ? variant : undefined}
data-active={active ? true : undefined}
data-selectable={includeSelectionAttribute ? true : undefined}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Using this to limit the new CSS selector. Could also gate this behavior behind a FF or prop to limit exposure if needed.

data-inactive={inactiveText ? true : undefined}
data-has-subitem={slots.subItem ? true : undefined}
data-has-description={slots.description ? true : false}
Expand Down
Loading