diff --git a/.changeset/lazy-lions-repair.md b/.changeset/lazy-lions-repair.md new file mode 100644 index 00000000000..a051340c94b --- /dev/null +++ b/.changeset/lazy-lions-repair.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +UnderlineNav2: Introduce "keeping the selected item always visible" functionality diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index e66ede78c2a..0c13a2431c3 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -34,7 +34,7 @@ import {UnderlineNav} from '@primer/react/drafts' Issues - Pull requests + Pull Requests Discussions @@ -61,7 +61,7 @@ When overflow occurs, the component first hides icons if present to optimize for Issues - Pull requests + Pull Requests Discussions @@ -91,7 +91,7 @@ If there is still overflow, the component will behave depending on the pointer. Issues - Pull requests + Pull Requests Discussions diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index 1c4f88a0cbe..b85c47bf633 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -1,7 +1,16 @@ import React from 'react' import '@testing-library/jest-dom/extend-expect' import {fireEvent, render} from '@testing-library/react' -import {CodeIcon, EyeIcon} from '@primer/octicons-react' +import { + IconProps, + CodeIcon, + IssueOpenedIcon, + GitPullRequestIcon, + CommentDiscussionIcon, + ProjectIcon, + ShieldLockIcon, + GraphIcon +} from '@primer/octicons-react' import {UnderlineNav} from '.' @@ -25,46 +34,61 @@ Object.defineProperty(window.Element.prototype, 'scrollTo', { value: jest.fn(), writable: true }) + +const ResponsiveUnderlineNav = ({ + selectedItemText = 'Code', + loadingCounters = false +}: { + selectedItemText?: string + loadingCounters?: boolean +}) => { + const items: {navigation: string; icon?: React.FC; counter?: number}[] = [ + {navigation: 'Code', icon: CodeIcon}, + {navigation: 'Issues', icon: IssueOpenedIcon, counter: 120}, + {navigation: 'Pull Requests', icon: GitPullRequestIcon, counter: 13}, + {navigation: 'Discussions', icon: CommentDiscussionIcon, counter: 5}, + {navigation: 'Actions', counter: 4}, + {navigation: 'Projects', icon: ProjectIcon, counter: 9}, + {navigation: 'Insights', icon: GraphIcon}, + {navigation: 'Settings', counter: 10}, + {navigation: 'Security', icon: ShieldLockIcon} + ] + return ( + + {items.map(item => ( + + {item.navigation} + + ))} + + ) +} + describe('UnderlineNav', () => { - test('selected nav', () => { - const {getByText} = render( - - Item 1 - Item 2 - Item 3 - - ) - const selectedNavLink = getByText('Item 1').closest('a') + it('renders aria-current attribute to be pages when an item is selected', () => { + const {getByText} = render() + const selectedNavLink = getByText('Code').closest('a') expect(selectedNavLink?.getAttribute('aria-current')).toBe('page') }) - test('basic nav functionality', () => { - const {container} = render( - - Item 1 - Item 2 - Item 3 - - ) + it('renders aria-label attribute correctly', () => { + const {container} = render() expect(container.getElementsByTagName('nav').length).toEqual(1) const nav = container.getElementsByTagName('nav')[0] - expect(nav.getAttribute('aria-label')).toBe('Test nav') + expect(nav.getAttribute('aria-label')).toBe('Repository') }) - test('with icons', () => { - const {container} = render( - - Code - - Issues - - Pull Request - - ) + it('renders icons correctly', () => { + const {container} = render() const nav = container.getElementsByTagName('nav')[0] - expect(nav.getElementsByTagName('svg').length).toEqual(2) + expect(nav.getElementsByTagName('svg').length).toEqual(7) }) - test('should fire onSelect on click and keypress', async () => { + it('fires onSelect on click and keypress', async () => { const onSelect = jest.fn() const {getByText} = render( @@ -79,32 +103,16 @@ describe('UnderlineNav', () => { fireEvent.keyPress(item, {key: 'Enter', code: 13, charCode: 13}) expect(onSelect).toHaveBeenCalledTimes(2) }) - test('respect counter prop', () => { - const {getByText} = render( - - - Item 1 - - Item 2 - Item 3 - - ) - const item = getByText('Item 1').closest('a') - const counter = item?.getElementsByTagName('span')[2] + it('respects counter prop', () => { + const {getByText} = render() + const item = getByText('Issues').closest('a') + const counter = item?.getElementsByTagName('span')[3] expect(counter?.className).toContain('CounterLabel') - expect(counter?.textContent).toBe('8') + expect(counter?.textContent).toBe('120') }) - test('respect loadingCounters prop', () => { - const {getByText} = render( - - - Item 1 - - Item 2 - Item 3 - - ) - const item = getByText('Item 1').closest('a') + it('respects loadingCounters prop', () => { + const {getByText} = render() + const item = getByText('Actions').closest('a') const loadingCounter = item?.getElementsByTagName('span')[2] expect(loadingCounter?.className).toContain('LoadingCounter') expect(loadingCounter?.textContent).toBe('') diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 23e3aee5091..e0c67bbee06 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -12,7 +12,15 @@ import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' -import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, scrollStyles, moreMenuStyles} from './styles' +import { + moreBtnStyles, + getDividerStyle, + getNavStyles, + ulStyles, + scrollStyles, + moreMenuStyles, + menuItemStyles +} from './styles' import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' import styled from 'styled-components' import {LoadingCounter} from './LoadingCounter' @@ -47,6 +55,10 @@ const NavigationList = styled.ul` ${sx}; ` +const MoreMenuListItem = styled.li` + display: flex; +` + const handleArrowBtnsVisibility = ( scrollableList: RefObject, callback: (scroll: {scrollLeft: number; scrollRight: number}) => void @@ -63,18 +75,17 @@ const overflowEffect = ( childWidthArray: ChildWidthArray, noIconChildWidthArray: ChildWidthArray, isCoarsePointer: boolean, - callback: (props: ResponsiveProps, iconsVisible: boolean) => void + updateListAndMenu: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true let overflowStyles: BetterSystemStyleObject | null = {} - if (childWidthArray.length === 0) { - callback({items: childArray, actions: [], overflowStyles}, iconsVisible) + updateListAndMenu({items: childArray, actions: [], overflowStyles}, iconsVisible) } const numberOfItemsPossible = calculatePossibleItems(childWidthArray, navWidth) const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, navWidth) - // We need take more menu width into account when calculating the number of items possible + // We need to take more menu width into account when calculating the number of items possible const numberOfItemsPossibleWithMoreMenu = calculatePossibleItems( noIconChildWidthArray, navWidth, @@ -106,6 +117,11 @@ const overflowEffect = ( for (const [index, child] of childArray.entries()) { if (index < numberOfItemsPossibleWithMoreMenu) { items.push(child) + // keeping 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] + actions.push(propsectiveAction) } else { actions.push(child) } @@ -113,7 +129,7 @@ const overflowEffect = ( } } - callback({items, actions, overflowStyles}, iconsVisible) + updateListAndMenu({items, actions, overflowStyles}, iconsVisible) } const getValidChildren = (children: React.ReactNode) => { @@ -151,16 +167,72 @@ export const UnderlineNav = forwardRef( forwardedRef ) => { const backupRef = useRef(null) - const newRef = (forwardedRef ?? backupRef) as MutableRefObject + const navRef = (forwardedRef ?? backupRef) as MutableRefObject const listRef = useRef(null) - const moreMenuRef = useRef(null) + const moreMenuRef = useRef(null) const {theme} = useTheme() + function getItemsWidth(itemText: string): number { + return noIconChildWidthArray.find(item => item.text === itemText)?.width || 0 + } + + const swapMenuItemWithListItem = ( + prospectiveListItem: React.ReactElement, + indexOfProspectiveListItem: number, + event: React.MouseEvent | React.KeyboardEvent, + callback: (props: ResponsiveProps, displayIcons: boolean) => void + ) => { + // get the selected menu item's width + const widthToFitIntoList = getItemsWidth(prospectiveListItem.props.children) + // Check if there is any empty space on the right side of the list + const availableSpace = + navRef.current.getBoundingClientRect().width - (listRef.current?.getBoundingClientRect().width || 0) + + // Calculate how many items need to be pulled in to the menu to make room for the selected menu item + // I.e. if we need to pull 2 items in (index 0 and index 1), breakpoint (index) will return 1. + const index = getBreakpointForItemSwapping(widthToFitIntoList, availableSpace) + const indexToSliceAt = responsiveProps.items.length - 1 - index + // Form the new list of items + const itemsLeftInList = [...responsiveProps.items].slice(0, indexToSliceAt) + const updatedItemList = [...itemsLeftInList, prospectiveListItem] + // Form the new menu items + const itemsToAddToMenu = [...responsiveProps.items].slice(indexToSliceAt) + const updatedMenuItems = [...actions] + // Add itemsToAddToMenu array's items to the menu at the index of the prospectiveListItem and remove 1 count of items (prospectiveListItem) + updatedMenuItems.splice(indexOfProspectiveListItem, 1, ...itemsToAddToMenu) + setSelectedLinkText(prospectiveListItem.props.children) + callback( + {items: updatedItemList, actions: updatedMenuItems, overflowStyles: responsiveProps.overflowStyles}, + false + ) + } + // How many items do we need to pull in to the menu to make room for the selected menu item. + function getBreakpointForItemSwapping(widthToFitIntoList: number, availableSpace: number) { + let widthToSwap = 0 + let breakpoint = 0 + for (const [index, item] of [...responsiveProps.items].reverse().entries()) { + widthToSwap += getItemsWidth(item.props.children) + if (widthToFitIntoList < widthToSwap + availableSpace) { + breakpoint = index + break + } + } + return breakpoint + } + const isCoarsePointer = useMedia('(pointer: coarse)') const [selectedLink, setSelectedLink] = useState | undefined>(undefined) + // selectedLinkText is needed to be able set the selected menu item as selectedLink. + // This is needed because setSelectedLink only accepts ref but at the time of setting selected menu item as selectedLink, its ref as a list item is not available + const [selectedLinkText, setSelectedLinkText] = useState('') + // Capture the mouse/keyboard event when a menu item is selected so that we can use it to fire the onSelect callback after the menu item is swapped with the list item + const [selectEvent, setSelectEvent] = useState< + React.MouseEvent | React.KeyboardEvent | null + >(null) + const [iconsVisible, setIconsVisible] = useState(true) const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ @@ -180,7 +252,7 @@ export const UnderlineNav = forwardRef( overflowStyles: {} }) - const callback = useCallback((props: ResponsiveProps, displayIcons: boolean) => { + const updateListAndMenu = useCallback((props: ResponsiveProps, displayIcons: boolean) => { setResponsiveProps(props) setIconsVisible(displayIcons) }, []) @@ -216,28 +288,21 @@ export const UnderlineNav = forwardRef( }) }, []) - // resizeObserver calls this function infinitely without a useCallback - const resizeObserverCallback = useCallback( - (resizeObserverEntries: ResizeObserverEntry[]) => { - const childArray = getValidChildren(children) - const navWidth = resizeObserverEntries[0].contentRect.width - const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 - overflowEffect( - navWidth, - moreMenuWidth, - childArray, - childWidthArray, - noIconChildWidthArray, - isCoarsePointer, - callback - ) - - handleArrowBtnsVisibility(listRef, updateOffsetValues) - }, - [callback, updateOffsetValues, childWidthArray, noIconChildWidthArray, children, isCoarsePointer, moreMenuRef] - ) - - useResizeObserver(resizeObserverCallback, newRef as RefObject) + useResizeObserver((resizeObserverEntries: ResizeObserverEntry[]) => { + const childArray = getValidChildren(children) + const navWidth = resizeObserverEntries[0].contentRect.width + const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 + overflowEffect( + navWidth, + moreMenuWidth, + childArray, + childWidthArray, + noIconChildWidthArray, + isCoarsePointer, + updateListAndMenu + ) + handleArrowBtnsVisibility(listRef, updateOffsetValues) + }, navRef as RefObject) useEffect(() => { const listEl = listRef.current @@ -247,19 +312,24 @@ export const UnderlineNav = forwardRef( }, [scrollOnList]) useEffect(() => { - // scroll the selected link into the view - if (selectedLink && selectedLink.current && listRef.current) { + // scroll the selected link into the view (coarse pointer behaviour) + if (isCoarsePointer && selectedLink?.current && listRef.current) { scrollIntoView(selectedLink.current, listRef.current, underlineNavScrollMargins) + return } - }, [selectedLink]) + }, [selectedLink, isCoarsePointer]) return ( (getNavStyles(theme, {align}), sxProp)} aria-label={label} - ref={newRef} + ref={navRef} > {isCoarsePointer && ( 0} onScrollWithButton={onScrollWithButton} /> @@ -278,40 +348,50 @@ export const UnderlineNav = forwardRef( (responsiveProps.overflowStyles, ulStyles)} ref={listRef}> {responsiveProps.items} + {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} + )} + + + ) + })} + + + + + )} {isCoarsePointer && ( 0} onScrollWithButton={onScrollWithButton} /> )} - - {actions.length > 0 && ( - - - - More - - - {actions.map((action, index) => { - const {children: actionElementChildren, ...actionElementProps} = action.props - return ( - - - {actionElementChildren} - - {loadingCounters ? ( - - ) : ( - {actionElementProps.counter} - )} - - - ) - })} - - - - - )} ) diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx index b36192aefbe..569545aa656 100644 --- a/src/UnderlineNav2/UnderlineNavArrowButton.tsx +++ b/src/UnderlineNav2/UnderlineNavArrowButton.tsx @@ -1,9 +1,9 @@ -import React from 'react' +import React, {useContext} from 'react' import {IconButton} from '../Button/IconButton' import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' import {getLeftArrowHiddenBtn, getRightArrowHiddenBtn, getLeftArrowVisibleBtn, getRightArrowVisibleBtn} from './styles' import {OnScrollWithButtonEventType} from './types' -import {useTheme} from '../ThemeProvider' +import {UnderlineNavContext} from './UnderlineNavContext' const LeftArrowButton = ({ show, @@ -12,7 +12,7 @@ const LeftArrowButton = ({ show: boolean onScrollWithButton: OnScrollWithButtonEventType }) => { - const {theme} = useTheme() + const {theme} = useContext(UnderlineNavContext) return ( { - const {theme} = useTheme() + const {theme} = useContext(UnderlineNavContext) return ( - setNoIconChildrenWidth: React.Dispatch<{width: number}> + theme: Theme | undefined + setChildrenWidth: React.Dispatch<{text: string; width: number}> + setNoIconChildrenWidth: React.Dispatch<{text: string; width: number}> selectedLink: RefObject | undefined setSelectedLink: (ref: RefObject) => void + selectedLinkText: string + setSelectedLinkText: React.Dispatch> + selectEvent: React.MouseEvent | React.KeyboardEvent | null afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void variant: 'default' | 'small' loadingCounters: boolean iconsVisible: boolean }>({ + theme: {}, setChildrenWidth: () => null, setNoIconChildrenWidth: () => null, selectedLink: undefined, setSelectedLink: () => null, + selectedLinkText: '', + setSelectedLinkText: () => null, + selectEvent: null, variant: 'default', loadingCounters: false, iconsVisible: true diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 3e50524f315..45cfbe6132f 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -5,7 +5,6 @@ import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' -import {useTheme} from '../ThemeProvider' import {getLinkStyles, wrapperStyles, iconWrapStyles, counterStyles} from './styles' import {LoadingCounter} from './LoadingCounter' @@ -65,29 +64,60 @@ export const UnderlineNavItem = forwardRef( const backupRef = useRef(null) const ref = (forwardedRef ?? backupRef) as RefObject const { + theme, setChildrenWidth, setNoIconChildrenWidth, selectedLink, setSelectedLink, + selectedLinkText, + setSelectedLinkText, + selectEvent, afterSelect, variant, loadingCounters, iconsVisible } = useContext(UnderlineNavContext) - const {theme} = useTheme() + useLayoutEffect(() => { const domRect = (ref as MutableRefObject).current.getBoundingClientRect() - // might want to select this better - const icon = (ref as MutableRefObject).current.children[0].children[0] - const iconWidthWithMargin = - icon.getBoundingClientRect().width + - Number(getComputedStyle(icon).marginRight.slice(0, -2)) + - Number(getComputedStyle(icon).marginLeft.slice(0, -2)) - setChildrenWidth({width: domRect.width}) - setNoIconChildrenWidth({width: domRect.width - iconWidthWithMargin}) + const icon = Array.from((ref as MutableRefObject).current.children[0].children).find( + child => child.getAttribute('data-component') === 'icon' + ) + + const content = Array.from((ref as MutableRefObject).current.children[0].children).find( + child => child.getAttribute('data-component') === 'text' + ) as HTMLElement + const text = content.textContent as string + + const iconWidthWithMargin = icon + ? icon.getBoundingClientRect().width + + Number(getComputedStyle(icon).marginRight.slice(0, -2)) + + Number(getComputedStyle(icon).marginLeft.slice(0, -2)) + : 0 + + setChildrenWidth({text, width: domRect.width}) + setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin}) preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject) - }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth, setNoIconChildrenWidth]) + + // Only runs when a menu item is selected (swapping the menu item with the list item to keep it visible) + if (selectedLinkText === text) { + setSelectedLink(ref as RefObject) + if (typeof onSelect === 'function' && selectEvent !== null) onSelect(selectEvent) + setSelectedLinkText('') + } + }, [ + ref, + preSelected, + selectedLink, + selectedLinkText, + setSelectedLinkText, + setSelectedLink, + setChildrenWidth, + setNoIconChildrenWidth, + onSelect, + selectEvent + ]) const keyPressHandler = React.useCallback( event => { diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index dd05e995b25..71f712bd6de 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -206,3 +206,10 @@ export const scrollStyles: BetterSystemStyleObject = { } export const moreMenuStyles: BetterSystemStyleObject = {whiteSpace: 'nowrap'} + +export const menuItemStyles = { + // This is needed to hide the selected check icon on the menu item. https://github.com/primer/react/blob/main/src/ActionList/Selection.tsx#L32 + '& > span': { + display: 'none' + } +} diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index 635213764e3..7f49d92eb58 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -1,5 +1,5 @@ import {BetterSystemStyleObject} from '../sx' -export type ChildWidthArray = Array<{width: number}> +export type ChildWidthArray = Array<{text: string; width: number}> export type ResponsiveProps = { items: Array actions: Array diff --git a/src/hooks/useResizeObserver.ts b/src/hooks/useResizeObserver.ts index 4ed92e1de44..6d217e82135 100644 --- a/src/hooks/useResizeObserver.ts +++ b/src/hooks/useResizeObserver.ts @@ -1,4 +1,4 @@ -import {RefObject} from 'react' +import {RefObject, useRef} from 'react' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' // https://gist.github.com/strothj/708afcf4f01dd04de8f49c92e88093c3 @@ -9,13 +9,26 @@ export interface ResizeObserverEntry { } export function useResizeObserver(callback: ResizeObserverCallback, target?: RefObject) { + const savedCallback = useRef(callback) + + useLayoutEffect(() => { + savedCallback.current = callback + }) + useLayoutEffect(() => { const targetEl = target && 'current' in target ? target.current : document.documentElement - if (!targetEl) return () => {} - const observer = new window.ResizeObserver(entries => callback(entries)) + if (!targetEl) { + return + } + + const observer = new ResizeObserver(entries => { + savedCallback.current(entries) + }) + observer.observe(targetEl) + return () => { observer.disconnect() } - }, [callback, target]) + }, [target]) }