diff --git a/.changeset/curly-hornets-bathe.md b/.changeset/curly-hornets-bathe.md new file mode 100644 index 00000000000..a729d86b6a2 --- /dev/null +++ b/.changeset/curly-hornets-bathe.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +UnderlineNav2: Introduce disclosure widget pattern diff --git a/.changeset/moody-garlics-know.md b/.changeset/moody-garlics-know.md new file mode 100644 index 00000000000..31af99f60e7 --- /dev/null +++ b/.changeset/moody-garlics-know.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +UnderlineNav2: Always show at least two items in the overflow menu diff --git a/.changeset/twelve-spoons-walk.md b/.changeset/twelve-spoons-walk.md new file mode 100644 index 00000000000..883a16eb6a3 --- /dev/null +++ b/.changeset/twelve-spoons-walk.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +UnderlineNav2: Render a visually hidden heading for screen readers when aria-label is present diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index 9231b4b9b90..ff303fdb070 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -112,4 +112,10 @@ describe('UnderlineNav', () => { expect(loadingCounter?.className).toContain('LoadingCounter') expect(loadingCounter?.textContent).toBe('') }) + it('renders a visually hidden h2 heading for screen readers when aria-label is present', () => { + const {container} = render() + const heading = container.getElementsByTagName('h2')[0] + expect(heading.className).toContain('VisuallyHidden') + expect(heading.textContent).toBe('Repository navigation') + }) }) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index cf256f9409a..b58470ecdc3 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -2,16 +2,22 @@ import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefO import Box from '../Box' import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' -import {ActionMenu} from '../ActionMenu' -import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' import {ChildWidthArray, ResponsiveProps} from './types' - -import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, menuItemStyles, GAP} from './styles' +import VisuallyHidden from '../_VisuallyHidden' +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, menuStyles, menuItemStyles, GAP} from './styles' import styled from 'styled-components' import {LoadingCounter} from './LoadingCounter' +import {Button} from '../Button' +import {useFocusZone} from '../hooks/useFocusZone' +import {FocusKeys} from '@primer/behaviors' +import {TriangleDownIcon} from '@primer/octicons-react' +import {useOnEscapePress} from '../hooks/useOnEscapePress' +import {useOnOutsideClick} from '../hooks/useOnOutsideClick' +import {ActionList} from '../ActionList' +import {useSSRSafeId} from '@react-aria/ssr' export type UnderlineNavProps = { 'aria-label'?: React.AriaAttributes['aria-label'] @@ -63,23 +69,34 @@ const overflowEffect = ( const items: Array = [] const actions: Array = [] - // For fine pointer devices, first we check if we can fit all the items with icons + // First, we check if we can fit all the items with their icons if (childArray.length <= numberOfItemsPossible) { items.push(...childArray) } else if (childArray.length <= numberOfItemsWithoutIconPossible) { - // if we can't fit all the items with icons, we check if we can fit all the items without icons + // if we can't fit all the items with their icons, we check if we can fit all the items without their icons iconsVisible = false items.push(...childArray) } else { - // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + // if we can't fit all the items without their icons, we keep the icons hidden and show the ones that doesn't fit into the list in the overflow menu iconsVisible = false + + /* Below is an accessibiility requirement. Never show only one item in the overflow menu. + * If there is only one item left to display in the overflow menu according to the calculation, + * we need to pull another item from the list into the overflow menu. + */ + const numberOfItemsInMenu = childArray.length - numberOfItemsPossibleWithMoreMenu + const numberOfListItems = + numberOfItemsInMenu === 1 ? numberOfItemsPossibleWithMoreMenu - 1 : numberOfItemsPossibleWithMoreMenu + for (const [index, child] of childArray.entries()) { - if (index < numberOfItemsPossibleWithMoreMenu) { + if (index < numberOfListItems) { items.push(child) - // keeping selected item always visible. + // We need to make sure to keep the selected item always visible. } else if (child.props.selected) { - // If selected item's index couldn't make the list, we swap it with the last item in the list. - const propsectiveAction = items.splice(numberOfItemsPossibleWithMoreMenu - 1, 1, child)[0] + // If selected item can't make it to the list, we swap it with the last item in the list. + const indexToReplaceAt = numberOfListItems - 1 // because we are replacing the last item in the list + // splice method modifies the array by removing 1 item here at the given index and replace it with the "child" element then returns the removed item. + const propsectiveAction = items.splice(indexToReplaceAt, 1, child)[0] actions.push(propsectiveAction) } else { actions.push(child) @@ -129,6 +146,9 @@ export const UnderlineNav = forwardRef( const navRef = (forwardedRef ?? backupRef) as MutableRefObject const listRef = useRef(null) const moreMenuRef = useRef(null) + const moreMenuBtnRef = useRef(null) + const containerRef = React.useRef(null) + const disclosureWidgetId = useSSRSafeId() const {theme} = useTheme() @@ -187,13 +207,12 @@ export const UnderlineNav = forwardRef( React.MouseEvent | React.KeyboardEvent | null >(null) - const [asNavItem, setAsNavItem] = useState('a') - const [iconsVisible, setIconsVisible] = useState(true) const afterSelectHandler = (event: React.MouseEvent | React.KeyboardEvent) => { if (!event.defaultPrevented) { if (typeof afterSelect === 'function') afterSelect(event) + closeOverlay() } } @@ -235,6 +254,39 @@ export const UnderlineNav = forwardRef( // eslint-disable-next-line no-console console.warn('Use the `aria-label` prop to provide an accessible label for assistive technology') } + const [isWidgetOpen, setIsWidgetOpen] = useState(false) + + const closeOverlay = React.useCallback(() => { + setIsWidgetOpen(false) + }, [setIsWidgetOpen]) + + const focusOnMoreMenuBtn = React.useCallback(() => { + moreMenuBtnRef.current?.focus() + }, []) + + useFocusZone({ + containerRef: backupRef, + bindKeys: FocusKeys.ArrowVertical | FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd | FocusKeys.Tab + }) + + useOnEscapePress( + (event: KeyboardEvent) => { + if (isWidgetOpen) { + event.preventDefault() + closeOverlay() + focusOnMoreMenuBtn() + } + }, + [isWidgetOpen] + ) + + useOnOutsideClick({onClickOutside: closeOverlay, containerRef, ignoreClickRefs: [moreMenuBtnRef]}) + const onAnchorClick = useCallback((event: React.MouseEvent) => { + if (event.defaultPrevented || event.button !== 0) { + return + } + setIsWidgetOpen(isWidgetOpen => !isWidgetOpen) + }, []) return ( + {ariaLabel && {`${ariaLabel} navigation`}} (getNavStyles(theme, {align}), sxProp)} @@ -265,46 +317,54 @@ export const UnderlineNav = forwardRef( {actions.length > 0 && ( - - More - - - {actions.map((action, index) => { - const {children: actionElementChildren, ...actionElementProps} = action.props - return ( - - | React.KeyboardEvent - ) => { - swapMenuItemWithListItem(action, index, event, updateListAndMenu) - setSelectEvent(event) - }} - > - - {actionElementChildren} - - {loadingCounters ? ( - - ) : ( - actionElementProps.counter !== undefined && ( - {actionElementProps.counter} - ) - )} - - + + + {actions.map((action, index) => { + const {children: actionElementChildren, ...actionElementProps} = action.props + return ( + + | React.KeyboardEvent) => { + swapMenuItemWithListItem(action, index, event, updateListAndMenu) + setSelectEvent(event) + closeOverlay() + focusOnMoreMenuBtn() + }} + > + + {actionElementChildren} + + {loadingCounters ? ( + + ) : ( + actionElementProps.counter !== undefined && ( + {actionElementProps.counter} + ) + )} - ) - })} - - - + + + ) + })} + )} diff --git a/src/UnderlineNav2/UnderlineNavContext.tsx b/src/UnderlineNav2/UnderlineNavContext.tsx index 08521017f7a..0e2b79668b2 100644 --- a/src/UnderlineNav2/UnderlineNavContext.tsx +++ b/src/UnderlineNav2/UnderlineNavContext.tsx @@ -10,7 +10,6 @@ export const UnderlineNavContext = createContext<{ selectedLinkText: string setSelectedLinkText: React.Dispatch> selectEvent: React.MouseEvent | React.KeyboardEvent | null - setAsNavItem: React.Dispatch> afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void variant: 'default' | 'small' loadingCounters: boolean @@ -24,7 +23,6 @@ export const UnderlineNavContext = createContext<{ selectedLinkText: '', setSelectedLinkText: () => null, selectEvent: null, - setAsNavItem: () => null, variant: 'default', loadingCounters: false, iconsVisible: true diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 288a088e04f..be89953c72a 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -72,7 +72,6 @@ export const UnderlineNavItem = forwardRef( selectedLinkText, setSelectedLinkText, selectEvent, - setAsNavItem, afterSelect, variant, loadingCounters, @@ -107,7 +106,6 @@ export const UnderlineNavItem = forwardRef( if (typeof onSelect === 'function' && selectEvent !== null) onSelect(selectEvent) setSelectedLinkText('') } - setAsNavItem(Component) }, [ ref, preSelected, @@ -118,9 +116,7 @@ export const UnderlineNavItem = forwardRef( setChildrenWidth, setNoIconChildrenWidth, onSelect, - selectEvent, - setAsNavItem, - Component + selectEvent ]) const keyPressHandler = React.useCallback( diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 8f224053ff7..de02290a690 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -72,16 +72,16 @@ export const withCounterLabels = () => { ) } -const items: {navigation: string; icon: React.FC; counter?: number | string}[] = [ - {navigation: 'Code', icon: CodeIcon}, - {navigation: 'Issues', icon: IssueOpenedIcon, counter: '12K'}, - {navigation: 'Pull Requests', icon: GitPullRequestIcon, counter: 13}, - {navigation: 'Discussions', icon: CommentDiscussionIcon, counter: 5}, - {navigation: 'Actions', icon: PlayIcon, counter: 4}, - {navigation: 'Projects', icon: ProjectIcon, counter: 9}, - {navigation: 'Insights', icon: GraphIcon, counter: '0'}, - {navigation: 'Settings', icon: GearIcon, counter: 10}, - {navigation: 'Security', icon: ShieldLockIcon} +const items: {navigation: string; icon: React.FC; counter?: number | string; href?: string}[] = [ + {navigation: 'Code', icon: CodeIcon, href: '#code'}, + {navigation: 'Issues', icon: IssueOpenedIcon, counter: '12K', href: '#issues'}, + {navigation: 'Pull Requests', icon: GitPullRequestIcon, counter: 13, href: '#pull-requests'}, + {navigation: 'Discussions', icon: CommentDiscussionIcon, counter: 5, href: '#discussions'}, + {navigation: 'Actions', icon: PlayIcon, counter: 4, href: '#actions'}, + {navigation: 'Projects', icon: ProjectIcon, counter: 9, href: '#projects'}, + {navigation: 'Insights', icon: GraphIcon, counter: '0', href: '#insights'}, + {navigation: 'Settings', icon: GearIcon, counter: 10, href: '#settings'}, + {navigation: 'Security', icon: ShieldLockIcon, href: '#security'} ] export const InternalResponsiveNav = () => { @@ -96,6 +96,7 @@ export const InternalResponsiveNav = () => { selected={index === selectedIndex} onSelect={() => setSelectedIndex(index)} counter={item.counter} + href={item.href} > {item.navigation} diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index 5b82b8190ce..e0eda108de1 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -39,8 +39,7 @@ export const getNavStyles = (theme?: Theme, props?: Partial ({ @@ -71,7 +71,10 @@ export const moreBtnStyles = { fontWeight: 'normal', boxShadow: 'none', paddingY: 1, - paddingX: 2 + paddingX: 2, + '& > span[data-component="trailingIcon"]': { + marginLeft: 0 + } } export const getLinkStyles = ( @@ -142,3 +145,16 @@ export const menuItemStyles = { // To reset the style when the menu items are rendered as react router links textDecoration: 'none' } + +export const menuStyles = { + position: 'absolute', + top: '90%', + right: '0', + boxShadow: '0 1px 3px rgba(0, 0, 0, 0.12), 0 1px 2px rgba(0, 0, 0, 0.24)', + borderRadius: '12px', + backgroundColor: 'canvas.overlay', + listStyle: 'none', + // Values are from ActionMenu + minWidth: '192px', + maxWidth: '640px' +} diff --git a/src/hooks/useOnOutsideClick.tsx b/src/hooks/useOnOutsideClick.tsx index 9cc3d38c404..58af086f1d8 100644 --- a/src/hooks/useOnOutsideClick.tsx +++ b/src/hooks/useOnOutsideClick.tsx @@ -4,7 +4,7 @@ export type TouchOrMouseEvent = MouseEvent | TouchEvent type TouchOrMouseEventCallback = (event: TouchOrMouseEvent) => boolean | undefined export type UseOnOutsideClickSettings = { - containerRef: React.RefObject + containerRef: React.RefObject | React.RefObject ignoreClickRefs?: React.RefObject[] onClickOutside: (e: TouchOrMouseEvent) => void }