From 2c5e29eedd149ca9dcb3dde08317bf2e97c784aa Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 1 Sep 2022 09:25:07 +1000 Subject: [PATCH 01/29] initial impl for overflow behaviour --- src/UnderlineNav2/UnderlineNav.tsx | 63 ++++++++++++++++++----- src/UnderlineNav2/UnderlineNavContext.tsx | 6 ++- src/UnderlineNav2/UnderlineNavItem.tsx | 21 ++++++-- 3 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 11f64403a30..9cfacc43afb 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -18,25 +18,41 @@ const overflowEffect = ( width: number, childArray: Array, childWidthArray: ChildWidthArray, - callback: (props: ResponsiveProps) => void + noIconChildWidthArray: ChildWidthArray, + callback: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { + let iconsVisible = true if (childWidthArray.length === 0) { - callback({items: childArray, actions: []}) + callback({items: childArray, actions: []}, iconsVisible) } - // do this only for overflow const numberOfItemsPossible = calculatePossibleItems(childWidthArray, width) + const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, width) const items: Array = [] const actions: Array = [] - for (const [index, child] of childArray.entries()) { - if (index < numberOfItemsPossible) { - items.push(child) - } else { - actions.push(child) + // First we check if we can fit all the items with 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 + iconsVisible = false + items.push(...childArray) + } else { + iconsVisible = false + // This is only for the overflow behaviour (for fine pointers) + // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + for (const [index, child] of childArray.entries()) { + if (index < numberOfItemsWithoutIconPossible) { + items.push(child) + } else { + actions.push(child) + } } + // TODO: Scroll behaviour to implement (for coarse pointers) } - callback({items, actions}) + + callback({items, actions}, iconsVisible) } export type {ResponsiveProps} @@ -49,7 +65,7 @@ function calculatePossibleItems(childWidthArray: ChildWidthArray, width: number) let breakpoint = childWidthArray.length - 1 let sumsOfChildWidth = 0 for (const [index, childWidth] of childWidthArray.entries()) { - if (sumsOfChildWidth > 0.5 * width) { + if (sumsOfChildWidth > 0.8 * width) { breakpoint = index break } else { @@ -116,6 +132,8 @@ export const UnderlineNav = forwardRef( const [selectedLink, setSelectedLink] = useState | undefined>(undefined) + const [iconsVisible, setIconsVisible] = useState(true) + const afterSelectHandler = (event: React.MouseEvent | React.KeyboardEvent) => { if (!event.defaultPrevented) { if (typeof afterSelect === 'function') afterSelect(event) @@ -127,8 +145,9 @@ export const UnderlineNav = forwardRef( actions: [] }) - const callback = useCallback((props: ResponsiveProps) => { + const callback = useCallback((props: ResponsiveProps, displayIcons: boolean) => { setResponsiveProps(props) + setIconsVisible(displayIcons) }, []) const actions = responsiveProps.actions @@ -140,21 +159,37 @@ export const UnderlineNav = forwardRef( }) }, []) + const [noIconChildWidthArray, setNoIconChildWidthArray] = useState([]) + const setNoIconChildrenWidth = useCallback(size => { + setNoIconChildWidthArray(arr => { + const newArr = [...arr, size] + return newArr + }) + }, []) + // resizeObserver calls this function infinitely without a useCallback const resizeObserverCallback = useCallback( (resizeObserverEntries: ResizeObserverEntry[]) => { if (overflow === 'auto' || overflow === 'menu') { const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width - overflowEffect(navWidth, childArray, childWidthArray, callback) + overflowEffect(navWidth, childArray, childWidthArray, noIconChildWidthArray, callback) } }, - [callback, childWidthArray, children, overflow] + [callback, childWidthArray, noIconChildWidthArray, children, overflow] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) return ( (overflowStyles, ulStyles)}> diff --git a/src/UnderlineNav2/UnderlineNavContext.tsx b/src/UnderlineNav2/UnderlineNavContext.tsx index 1d365773583..6f614aa0864 100644 --- a/src/UnderlineNav2/UnderlineNavContext.tsx +++ b/src/UnderlineNav2/UnderlineNavContext.tsx @@ -2,13 +2,17 @@ import React, {createContext, RefObject} from 'react' export const UnderlineNavContext = createContext<{ setChildrenWidth: React.Dispatch<{width: number}> + setNoIconChildrenWidth: React.Dispatch<{width: number}> selectedLink: RefObject | undefined setSelectedLink: (ref: RefObject) => void afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void variant: 'default' | 'small' + iconsVisible: boolean }>({ setChildrenWidth: () => null, + setNoIconChildrenWidth: () => null, selectedLink: undefined, setSelectedLink: () => null, - variant: 'default' + variant: 'default', + iconsVisible: true }) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 302ae12b411..88d3a350914 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -62,13 +62,28 @@ export const UnderlineNavItem = forwardRef( ) => { const backupRef = useRef(null) const ref = forwardedRef ?? backupRef - const {setChildrenWidth, selectedLink, setSelectedLink, afterSelect, variant} = useContext(UnderlineNavContext) + const { + setChildrenWidth, + setNoIconChildrenWidth, + selectedLink, + setSelectedLink, + afterSelect, + variant, + iconsVisible + } = useContext(UnderlineNavContext) const {theme} = useTheme() useLayoutEffect(() => { const domRect = (ref as MutableRefObject).current.getBoundingClientRect() + 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}) preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject) - }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth]) + }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth, setNoIconChildrenWidth]) const iconWrapStyles = { alignItems: 'center', @@ -182,7 +197,7 @@ export const UnderlineNavItem = forwardRef( ref={ref} > - {LeadingIcon && ( + {iconsVisible && LeadingIcon && ( From fd409d4cbcbe2910c35623ab16dc0e8003876970 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 1 Sep 2022 21:03:28 +1000 Subject: [PATCH 02/29] scroll behaviour initial commit - wip --- src/UnderlineNav2/UnderlineNav.tsx | 53 +++++++++++++------------- src/UnderlineNav2/examples.stories.tsx | 12 ++++-- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 9cfacc43afb..98271fe08db 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -8,7 +8,6 @@ import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' -type Overflow = 'auto' | 'menu' | 'scroll' type ChildWidthArray = Array<{width: number}> type ResponsiveProps = { items: Array @@ -40,16 +39,23 @@ const overflowEffect = ( items.push(...childArray) } else { iconsVisible = false - // This is only for the overflow behaviour (for fine pointers) - // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu - for (const [index, child] of childArray.entries()) { - if (index < numberOfItemsWithoutIconPossible) { - items.push(child) - } else { - actions.push(child) + // determine the media query pointer. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const isCoarsePointer = window.matchMedia && window.matchMedia('(pointer:coarse)').matches + // TODO: refactor this to avoid nested if else + if (isCoarsePointer) { + // TODO: handle scroll overflow here for media course pointer + } else { + // This is only for the overflow behaviour (for fine pointers) + // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + for (const [index, child] of childArray.entries()) { + if (index < numberOfItemsWithoutIconPossible) { + items.push(child) + } else { + actions.push(child) + } } } - // TODO: Scroll behaviour to implement (for coarse pointers) } callback({items, actions}, iconsVisible) @@ -78,7 +84,6 @@ function calculatePossibleItems(childWidthArray: ChildWidthArray, width: number) export type UnderlineNavProps = { label: string as?: React.ElementType - overflow?: Overflow align?: 'right' sx?: SxProp variant?: 'default' | 'small' @@ -88,16 +93,7 @@ export type UnderlineNavProps = { export const UnderlineNav = forwardRef( ( - { - as = 'nav', - overflow = 'auto', - align, - label, - sx: sxProp = {}, - afterSelect, - variant = 'default', - children - }: UnderlineNavProps, + {as = 'nav', align, label, sx: sxProp = {}, afterSelect, variant = 'default', children}: UnderlineNavProps, forwardedRef ) => { const backupRef = useRef(null) @@ -120,7 +116,12 @@ export const UnderlineNav = forwardRef( align: 'row', alignItems: 'center' } - const overflowStyles = overflow === 'scroll' ? {overflowX: 'auto', whiteSpace: 'nowrap'} : {} + + const overflowStyles = { + overflowX: 'auto', + whiteSpace: 'nowrap' + } + const ulStyles = { display: 'flex', listStyle: 'none', @@ -170,13 +171,11 @@ export const UnderlineNav = forwardRef( // resizeObserver calls this function infinitely without a useCallback const resizeObserverCallback = useCallback( (resizeObserverEntries: ResizeObserverEntry[]) => { - if (overflow === 'auto' || overflow === 'menu') { - const childArray = getValidChildren(children) - const navWidth = resizeObserverEntries[0].contentRect.width - overflowEffect(navWidth, childArray, childWidthArray, noIconChildWidthArray, callback) - } + const childArray = getValidChildren(children) + const navWidth = resizeObserverEntries[0].contentRect.width + overflowEffect(navWidth, childArray, childWidthArray, noIconChildWidthArray, callback) }, - [callback, childWidthArray, noIconChildWidthArray, children, overflow] + [callback, childWidthArray, noIconChildWidthArray, children] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) return ( diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 5e8a7fea4b7..d41032ff920 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -119,10 +119,16 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { } export const HorizontalScrollNav = (args: UnderlineNavProps) => { + const [selectedIndex, setSelectedIndex] = React.useState(2) return ( - - {items.map(item => ( - + + {items.map((item, index) => ( + setSelectedIndex(index)} + > {item} ))} From b2f4738e430bd9b477f0023c7778812f805e096a Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Tue, 6 Sep 2022 16:59:53 +1000 Subject: [PATCH 03/29] overflow and scroll behaviour and styles --- src/UnderlineNav2/UnderlineNav.tsx | 77 +++++++++++++++++++----------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 98271fe08db..ba2dfff2834 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -12,6 +12,7 @@ type ChildWidthArray = Array<{width: number}> type ResponsiveProps = { items: Array actions: Array + overflowStyles: React.CSSProperties } const overflowEffect = ( width: number, @@ -21,14 +22,16 @@ const overflowEffect = ( callback: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true + if (childWidthArray.length === 0) { - callback({items: childArray, actions: []}, iconsVisible) + callback({items: childArray, actions: [], overflowStyles: {}}, iconsVisible) } // do this only for overflow const numberOfItemsPossible = calculatePossibleItems(childWidthArray, width) const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, width) const items: Array = [] const actions: Array = [] + const overflowStyles: React.CSSProperties = {whiteSpace: 'nowrap'} // First we check if we can fit all the items with icons if (childArray.length <= numberOfItemsPossible) { @@ -39,12 +42,13 @@ const overflowEffect = ( items.push(...childArray) } else { iconsVisible = false - // determine the media query pointer. // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition const isCoarsePointer = window.matchMedia && window.matchMedia('(pointer:coarse)').matches + // TODO: refactor this to avoid nested if else if (isCoarsePointer) { - // TODO: handle scroll overflow here for media course pointer + items.push(...childArray) + overflowStyles.overflowX = 'auto' } else { // This is only for the overflow behaviour (for fine pointers) // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu @@ -58,7 +62,7 @@ const overflowEffect = ( } } - callback({items, actions}, iconsVisible) + callback({items, actions, overflowStyles}, iconsVisible) } export type {ResponsiveProps} @@ -110,18 +114,13 @@ export const UnderlineNav = forwardRef( const styles = { display: 'flex', - justifyContent: align === 'right' ? 'flex-end' : 'space-between', + justifyContent: align === 'right' ? 'flex-end' : 'flex-start', borderBottom: '1px solid', borderBottomColor: 'border.muted', align: 'row', alignItems: 'center' } - const overflowStyles = { - overflowX: 'auto', - whiteSpace: 'nowrap' - } - const ulStyles = { display: 'flex', listStyle: 'none', @@ -143,7 +142,8 @@ export const UnderlineNav = forwardRef( const [responsiveProps, setResponsiveProps] = useState({ items: getValidChildren(children), - actions: [] + actions: [], + overflowStyles: {} }) const callback = useCallback((props: ResponsiveProps, displayIcons: boolean) => { @@ -178,6 +178,25 @@ export const UnderlineNav = forwardRef( [callback, childWidthArray, noIconChildWidthArray, children] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) + + const dividerStyle = { + display: 'inline-block', + borderLeft: '1px solid', + width: '1px', + borderColor: 'border.muted', + marginRight: 1 + } + + const moreBtnStyles = { + //set margin 0 here because safari puts extra margin around the button, rest is to reset style to make it look like a list element + margin: 0, + border: 0, + background: 'transparent', + fontWeight: 'normal', + boxShadow: 'none', + paddingY: 1, + paddingX: 2 + } return ( - (overflowStyles, ulStyles)}> + (responsiveProps.overflowStyles, ulStyles)}> {responsiveProps.items} {actions.length > 0 && ( - - {/* set margin 0 here because safari puts extra margin around the button */} - More - - - {actions.map((action, index) => { - const {children: actionElementChildren, ...actionElementProps} = action.props - return ( - - {actionElementChildren} - - ) - })} - - - + + + + More + + + {actions.map((action, index) => { + const {children: actionElementChildren, ...actionElementProps} = action.props + return ( + + {actionElementChildren} + + ) + })} + + + + )} From ce08e12867093803db568db79abb40cbb3690825 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 8 Sep 2022 08:06:59 +1000 Subject: [PATCH 04/29] introduce scroll arrow buttons --- src/UnderlineNav2/UnderlineNav.tsx | 93 ++++++++++++++++++++++++-- src/UnderlineNav2/UnderlineNavItem.tsx | 1 + 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index ba2dfff2834..2ebe393faf0 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -1,12 +1,14 @@ -import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react' +import React, {useRef, useEffect, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react' import Box from '../Box' import {merge, SxProp, BetterSystemStyleObject} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' import {ActionMenu} from '../ActionMenu' +import {IconButton} from '../Button/IconButton' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' +import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' type ChildWidthArray = Array<{width: number}> type ResponsiveProps = { @@ -14,11 +16,19 @@ type ResponsiveProps = { actions: Array overflowStyles: React.CSSProperties } + +const handleArrowBtnsVisibility = ( + scrollOffsets: {scrollLeft: number; scrollRight: number}, + callback: (scroll: {scrollLeft: number; scrollRight: number}) => void +) => { + callback(scrollOffsets) +} const overflowEffect = ( width: number, childArray: Array, childWidthArray: ChildWidthArray, noIconChildWidthArray: ChildWidthArray, + isCoarsePointer: boolean, callback: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true @@ -42,13 +52,11 @@ const overflowEffect = ( items.push(...childArray) } else { iconsVisible = false - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - const isCoarsePointer = window.matchMedia && window.matchMedia('(pointer:coarse)').matches - // TODO: refactor this to avoid nested if else if (isCoarsePointer) { items.push(...childArray) overflowStyles.overflowX = 'auto' + overflowStyles.scrollbarWidth = 'none' } else { // This is only for the overflow behaviour (for fine pointers) // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu @@ -71,6 +79,12 @@ function getValidChildren(children: React.ReactNode) { return React.Children.toArray(children).filter(child => React.isValidElement(child)) as React.ReactElement[] } +function calculateScrollOffset(scrollableList: RefObject) { + const {scrollLeft, scrollWidth, clientWidth} = scrollableList.current as HTMLElement + const scrollRight = scrollWidth - scrollLeft - clientWidth + return {scrollLeft, scrollRight} +} + function calculatePossibleItems(childWidthArray: ChildWidthArray, width: number) { let breakpoint = childWidthArray.length - 1 let sumsOfChildWidth = 0 @@ -102,6 +116,20 @@ export const UnderlineNav = forwardRef( ) => { const backupRef = useRef(null) const newRef = (forwardedRef ?? backupRef) as MutableRefObject + const listRef = useRef(null) + + const [isCoarsePointer, setIsCoarsePointer] = useState(false) + + // Determine the pointer type on mount + useEffect(() => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + setIsCoarsePointer(true) //(window.matchMedia && window.matchMedia('(pointer:coarse)').matches) + // eslint-disable-next-line github/prefer-observers + listRef.current?.addEventListener('scroll', (event: any) => { + const scrollOffsets = calculateScrollOffset(listRef) + handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) + }) + }, []) // This might change if we decide tab through the navigation items rather than navigationg with the arrow keys. // TBD. In the meantime keeping it as a menu with the focus trap. @@ -134,6 +162,11 @@ export const UnderlineNav = forwardRef( const [iconsVisible, setIconsVisible] = useState(true) + const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ + scrollLeft: 0, + scrollRight: 0 + }) + const afterSelectHandler = (event: React.MouseEvent | React.KeyboardEvent) => { if (!event.defaultPrevented) { if (typeof afterSelect === 'function') afterSelect(event) @@ -151,6 +184,10 @@ export const UnderlineNav = forwardRef( setIconsVisible(displayIcons) }, []) + const updateOffsetValues = useCallback((scrollOffsets: {scrollLeft: number; scrollRight: number}) => { + setScrollValues(scrollOffsets) + }, []) + const actions = responsiveProps.actions const [childWidthArray, setChildWidthArray] = useState([]) const setChildrenWidth = useCallback(size => { @@ -173,9 +210,13 @@ export const UnderlineNav = forwardRef( (resizeObserverEntries: ResizeObserverEntry[]) => { const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width - overflowEffect(navWidth, childArray, childWidthArray, noIconChildWidthArray, callback) + const scrollOffsets = calculateScrollOffset(listRef) + + overflowEffect(navWidth, childArray, childWidthArray, noIconChildWidthArray, isCoarsePointer, callback) + + handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) }, - [callback, childWidthArray, noIconChildWidthArray, children] + [callback, updateOffsetValues, childWidthArray, noIconChildWidthArray, children, isCoarsePointer] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) @@ -197,6 +238,19 @@ export const UnderlineNav = forwardRef( paddingY: 1, paddingX: 2 } + + const arrowBtnStyles = { + ...moreBtnStyles, + paddingX: 1 + } + + const scroll = (event: React.MouseEvent, direction: string) => { + event.preventDefault() + event.stopPropagation() + const ScrollAmount = direction === 'left' ? -200 : 200 + const element = document.querySelector('nav > ul') + element?.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) + } return ( - (responsiveProps.overflowStyles, ulStyles)}> + {scrollValues.scrollLeft > 0 ? ( + ) => scroll(e, 'left')} + icon={ChevronLeftIcon} + sx={arrowBtnStyles} + /> + ) : ( + '' + )} + (responsiveProps.overflowStyles, ulStyles)} + ref={listRef as RefObject} + > {responsiveProps.items} + {scrollValues.scrollRight > 0 ? ( + ) => scroll(e, 'right')} + icon={ChevronRightIcon} + sx={arrowBtnStyles} + /> + ) : ( + '' + )} {actions.length > 0 && ( diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 88d3a350914..ebb5135f37d 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -74,6 +74,7 @@ export const UnderlineNavItem = forwardRef( 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 + From 5fa9687ee7c05dac484ba3d9ded9b4066353e910 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 12 Sep 2022 08:16:39 +1000 Subject: [PATCH 05/29] scroll behaviour in dept --- src/UnderlineNav2/UnderlineNav.tsx | 178 ++++++++++----------- src/UnderlineNav2/UnderlineNavItem.tsx | 83 +--------- src/UnderlineNav2/examples.stories.tsx | 42 +++-- src/UnderlineNav2/getUnderlineNavStyles.ts | 163 +++++++++++++++++++ src/UnderlineNav2/types.ts | 6 + 5 files changed, 281 insertions(+), 191 deletions(-) create mode 100644 src/UnderlineNav2/getUnderlineNavStyles.ts diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 2ebe393faf0..6c8dc5a232c 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -1,6 +1,15 @@ -import React, {useRef, useEffect, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react' +import React, { + useRef, + useLayoutEffect, + forwardRef, + useCallback, + useState, + MutableRefObject, + RefObject, + useEffect +} from 'react' import Box from '../Box' -import {merge, SxProp, BetterSystemStyleObject} from '../sx' +import {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' import {ActionMenu} from '../ActionMenu' import {IconButton} from '../Button/IconButton' @@ -9,12 +18,28 @@ import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' +import CounterLabel from '../CounterLabel' +import {useTheme} from '../ThemeProvider' +import {ChildWidthArray, ResponsiveProps} from './types' + +import { + moreBtnStyles, + getDividerStyle, + getNavStyles, + ulStyles, + leftArrowBtnStyles, + rightArrowBtnStyles, + hiddenBtn +} from './getUnderlineNavStyles' -type ChildWidthArray = Array<{width: number}> -type ResponsiveProps = { - items: Array - actions: Array - overflowStyles: React.CSSProperties +export type UnderlineNavProps = { + label: string + as?: React.ElementType + align?: 'right' + sx?: SxProp + variant?: 'default' | 'small' + afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void + children: React.ReactNode } const handleArrowBtnsVisibility = ( @@ -73,13 +98,11 @@ const overflowEffect = ( callback({items, actions, overflowStyles}, iconsVisible) } -export type {ResponsiveProps} - function getValidChildren(children: React.ReactNode) { return React.Children.toArray(children).filter(child => React.isValidElement(child)) as React.ReactElement[] } -function calculateScrollOffset(scrollableList: RefObject) { +function calculateScrollOffset(scrollableList: RefObject) { const {scrollLeft, scrollWidth, clientWidth} = scrollableList.current as HTMLElement const scrollRight = scrollWidth - scrollLeft - clientWidth return {scrollLeft, scrollRight} @@ -99,16 +122,6 @@ function calculatePossibleItems(childWidthArray: ChildWidthArray, width: number) return breakpoint } -export type UnderlineNavProps = { - label: string - as?: React.ElementType - align?: 'right' - sx?: SxProp - variant?: 'default' | 'small' - afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void - children: React.ReactNode -} - export const UnderlineNav = forwardRef( ( {as = 'nav', align, label, sx: sxProp = {}, afterSelect, variant = 'default', children}: UnderlineNavProps, @@ -118,19 +131,18 @@ export const UnderlineNav = forwardRef( const newRef = (forwardedRef ?? backupRef) as MutableRefObject const listRef = useRef(null) + const {theme} = useTheme() + const [isCoarsePointer, setIsCoarsePointer] = useState(false) - // Determine the pointer type on mount - useEffect(() => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - setIsCoarsePointer(true) //(window.matchMedia && window.matchMedia('(pointer:coarse)').matches) - // eslint-disable-next-line github/prefer-observers - listRef.current?.addEventListener('scroll', (event: any) => { - const scrollOffsets = calculateScrollOffset(listRef) - handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) - }) - }, []) + const [selectedLink, setSelectedLink] = useState | undefined>(undefined) + + const [iconsVisible, setIconsVisible] = useState(true) + const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ + scrollLeft: 0, + scrollRight: 0 + }) // This might change if we decide tab through the navigation items rather than navigationg with the arrow keys. // TBD. In the meantime keeping it as a menu with the focus trap. // ref: https://www.w3.org/WAI/ARIA/apg/example-index/menubar/menubar-navigation.html (Keyboard Support) @@ -140,33 +152,6 @@ export const UnderlineNav = forwardRef( focusOutBehavior: 'wrap' }) - const styles = { - display: 'flex', - justifyContent: align === 'right' ? 'flex-end' : 'flex-start', - borderBottom: '1px solid', - borderBottomColor: 'border.muted', - align: 'row', - alignItems: 'center' - } - - const ulStyles = { - display: 'flex', - listStyle: 'none', - padding: '0', - margin: '0', - marginBottom: '-1px', - alignItems: 'center' - } - - const [selectedLink, setSelectedLink] = useState | undefined>(undefined) - - const [iconsVisible, setIconsVisible] = useState(true) - - const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ - scrollLeft: 0, - scrollRight: 0 - }) - const afterSelectHandler = (event: React.MouseEvent | React.KeyboardEvent) => { if (!event.defaultPrevented) { if (typeof afterSelect === 'function') afterSelect(event) @@ -220,37 +205,31 @@ export const UnderlineNav = forwardRef( ) useResizeObserver(resizeObserverCallback, newRef as RefObject) - const dividerStyle = { - display: 'inline-block', - borderLeft: '1px solid', - width: '1px', - borderColor: 'border.muted', - marginRight: 1 - } + // Determine the pointer type on mount + useLayoutEffect(() => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + setIsCoarsePointer(window.matchMedia && window.matchMedia('(pointer:coarse)').matches) + // eslint-disable-next-line github/prefer-observers + listRef.current?.addEventListener('scroll', () => { + const scrollOffsets = calculateScrollOffset(listRef) - const moreBtnStyles = { - //set margin 0 here because safari puts extra margin around the button, rest is to reset style to make it look like a list element - margin: 0, - border: 0, - background: 'transparent', - fontWeight: 'normal', - boxShadow: 'none', - paddingY: 1, - paddingX: 2 - } + handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) + }) + }, [updateOffsetValues]) - const arrowBtnStyles = { - ...moreBtnStyles, - paddingX: 1 - } + useEffect(() => { + // scroll the selected link into the view + if (selectedLink) { + selectedLink.current?.scrollIntoView({behavior: 'smooth', block: 'nearest', inline: 'nearest'}) + } + }, [selectedLink]) - const scroll = (event: React.MouseEvent, direction: string) => { - event.preventDefault() - event.stopPropagation() - const ScrollAmount = direction === 'left' ? -200 : 200 - const element = document.querySelector('nav > ul') - element?.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) + const scrollWithButton = (event: React.MouseEvent, direction: -1 | 1) => { + if (!listRef.current) return + const ScrollAmount = direction * 200 + listRef.current.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) } + return ( - - {scrollValues.scrollLeft > 0 ? ( + (getNavStyles(theme, align), sxProp)} + aria-label={label} + ref={newRef} + > + {scrollValues.scrollLeft > 0 && ( ) => scroll(e, 'left')} + onClick={(e: React.MouseEvent) => scrollWithButton(e, -1)} icon={ChevronLeftIcon} - sx={arrowBtnStyles} + sx={scrollValues.scrollLeft > 0 ? leftArrowBtnStyles : hiddenBtn} /> - ) : ( - '' )} {responsiveProps.items} - {scrollValues.scrollRight > 0 ? ( + {scrollValues.scrollRight > 0 && ( ) => scroll(e, 'right')} + onClick={(e: React.MouseEvent) => scrollWithButton(e, 1)} icon={ChevronRightIcon} - sx={arrowBtnStyles} + sx={scrollValues.scrollRight > 0 ? rightArrowBtnStyles : hiddenBtn} /> - ) : ( - '' )} {actions.length > 0 && ( - + More @@ -304,7 +285,10 @@ export const UnderlineNav = forwardRef( const {children: actionElementChildren, ...actionElementProps} = action.props return ( - {actionElementChildren} + + {actionElementChildren} + {actionElementProps.counter} + ) })} diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index ebb5135f37d..4f551ae89e3 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -1,11 +1,12 @@ import React, {forwardRef, useLayoutEffect, useRef, useContext, MutableRefObject, RefObject} from 'react' import Box from '../Box' -import {merge, SxProp, BetterSystemStyleObject} from '../sx' +import {merge, SxProp} from '../sx' import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' -import {Theme, useTheme} from '../ThemeProvider' +import {useTheme} from '../ThemeProvider' +import {getLinkStyles, wrapperStyles, iconWrapStyles, textStyles, counterStyles} from './getUnderlineNavStyles' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -86,82 +87,6 @@ export const UnderlineNavItem = forwardRef( preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject) }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth, setNoIconChildrenWidth]) - const iconWrapStyles = { - alignItems: 'center', - display: 'inline-flex', - marginRight: 2 - } - - const textStyles: BetterSystemStyleObject = { - whiteSpace: 'nowrap' - } - - const wrapperStyles = { - display: 'inline-flex', - paddingY: 1, - paddingX: 2, - borderRadius: 2 - } - const smallVariantLinkStyles = { - paddingY: 1, - fontSize: 0 - } - const defaultVariantLinkStyles = { - paddingY: 2, - fontSize: 1 - } - - // eslint-disable-next-line no-shadow - const linkStyles = (theme?: Theme) => ({ - position: 'relative', - display: 'inline-flex', - color: 'fg.default', - textAlign: 'center', - textDecoration: 'none', - paddingX: 1, - ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), - '&:hover > div[data-component="wrapper"] ': { - backgroundColor: theme?.colors.neutral.muted, - transition: 'background .12s ease-out' - }, - '&:focus': { - outline: 0, - '& > div[data-component="wrapper"]': { - boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}` - }, - // where focus-visible is supported, remove the focus box-shadow - '&:not(:focus-visible) > div[data-component="wrapper"]': { - boxShadow: 'none' - } - }, - '&:focus-visible > div[data-component="wrapper"]': { - boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}` - }, - // renders a visibly hidden "copy" of the label in bold, reserving box space for when label becomes bold on selected - '& span[data-content]::before': { - content: 'attr(data-content)', - display: 'block', - height: 0, - fontWeight: '600', - visibility: 'hidden' - }, - // selected state styles - '&::after': { - position: 'absolute', - right: '50%', - bottom: 0, - width: `calc(100% - 8px)`, - height: 2, - content: '""', - bg: selectedLink === ref ? theme?.colors.primer.border.active : 'transparent', - borderRadius: 0, - transform: 'translate(50%, -50%)' - } - }) - - const counterStyles = { - marginLeft: 2 - } const keyPressHandler = React.useCallback( event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { @@ -193,7 +118,7 @@ export const UnderlineNavItem = forwardRef( onKeyPress={keyPressHandler} onClick={clickHandler} {...(selectedLink === ref ? {'aria-current': 'page'} : {})} - sx={merge(linkStyles(theme), sxProp as SxProp)} + sx={merge(getLinkStyles(theme, variant, selectedLink, ref), sxProp as SxProp)} {...props} ref={ref} > diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index d41032ff920..3977af2696e 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -1,5 +1,16 @@ import React from 'react' -import {EyeIcon, CodeIcon, IssueOpenedIcon, GitPullRequestIcon, CommentDiscussionIcon} from '@primer/octicons-react' +import { + EyeIcon, + CodeIcon, + IssueOpenedIcon, + GitPullRequestIcon, + CommentDiscussionIcon, + ProjectIcon, + ShieldIcon, + GearIcon, + GraphIcon, + PlayIcon +} from '@primer/octicons-react' import {Meta} from '@storybook/react' import {UnderlineNav, UnderlineNavProps} from './index' import {BaseStyles, ThemeProvider} from '..' @@ -86,17 +97,17 @@ export const rightAlign = (args: UnderlineNavProps) => { ) } -const items: string[] = [ - 'Item 1', - 'Looooong Item', - 'Looooooonger item', - 'Item 4', - 'Item 5', - 'Item 6', - 'Item 7', - 'Item 8', - 'Item 9', - 'Item 10' +const items: any[] = [ + {title: 'Code', icon: CodeIcon}, + {title: 'Issues', icon: IssueOpenedIcon, counter: 12}, + {title: 'Pull Requests', icon: GitPullRequestIcon, counter: 6}, + {title: 'Discussions', icon: CommentDiscussionIcon, counter: 7}, + {title: 'Actions', icon: PlayIcon, counter: 1}, + {title: 'Projects', icon: ProjectIcon, counter: 22}, + {title: 'Wiki', icon: EyeIcon, counter: 9}, + {title: 'Security', icon: ShieldIcon, counter: 7}, + {title: 'Insights', icon: GraphIcon, counter: 123}, + {title: 'Settings', icon: GearIcon, counter: 7} ] export const InternalResponsiveNav = (args: UnderlineNavProps) => { @@ -106,12 +117,13 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { {items.map((item, index) => ( setSelectedIndex(index)} + counter={item.counter} > - {item} + {item.title} ))} diff --git a/src/UnderlineNav2/getUnderlineNavStyles.ts b/src/UnderlineNav2/getUnderlineNavStyles.ts new file mode 100644 index 00000000000..37518689924 --- /dev/null +++ b/src/UnderlineNav2/getUnderlineNavStyles.ts @@ -0,0 +1,163 @@ +import {Theme} from '../ThemeProvider' +import {BetterSystemStyleObject} from '../sx' +import {UnderlineNavProps} from './UnderlineNav' + +export const iconWrapStyles = { + alignItems: 'center', + display: 'inline-flex', + marginRight: 2 +} + +export const textStyles: BetterSystemStyleObject = { + whiteSpace: 'nowrap' +} + +export const wrapperStyles = { + display: 'inline-flex', + paddingY: 1, + paddingX: 2, + borderRadius: 2 +} + +const smallVariantLinkStyles = { + paddingY: 1, + fontSize: 0 +} +const defaultVariantLinkStyles = { + paddingY: 2, + fontSize: 1 +} + +export const counterStyles = { + marginLeft: 2 +} + +export const getNavStyles = (theme?: Theme, props?: Partial>) => ({ + display: 'flex', + justifyContent: props?.align === 'right' ? 'flex-end' : 'flex-start', + borderBottom: '1px solid', + borderBottomColor: `${theme?.colors.border.muted}`, + align: 'row', + alignItems: 'center' +}) + +export const ulStyles = { + display: 'flex', + listStyle: 'none', + padding: '0', + paddingX: 1, + margin: '0', + marginBottom: '-1px', + alignItems: 'center' +} + +export const getDividerStyle = (theme?: Theme) => ({ + display: 'inline-block', + borderLeft: '1px solid', + width: '1px', + borderLeftColor: `${theme?.colors.border.muted}`, + marginRight: 1 +}) + +export const moreBtnStyles = { + //set margin 0 here because safari puts extra margin around the button, rest is to reset style to make it look like a list element + margin: 0, + border: 0, + background: 'transparent', + fontWeight: 'normal', + boxShadow: 'none', + paddingY: 1, + paddingX: 2 +} + +export const arrowBtnStyles = { + ...moreBtnStyles, + paddingX: 0, + paddingY: 0, + position: 'relative' +} + +export const hiddenBtn = { + ...arrowBtnStyles, + display: 'none' +} + +export const leftArrowBtnStyles = { + ...arrowBtnStyles, + '&::after': { + content: '""', + position: 'absolute', + background: 'linear-gradient(to left,#fff0,#fff)', + height: '100%', + width: '30px', + zIndex: 1, + left: '16px', + pointerEvents: 'none' + } +} + +export const rightArrowBtnStyles = { + ...arrowBtnStyles, + '&::before': { + position: 'absolute', + background: 'linear-gradient(to right,#fff0,#fff)', + content: '""', + height: '100%', + left: '-30px', + width: '30px', + zIndex: 1, + pointerEvents: 'none' + } +} + +export const getLinkStyles = ( + theme?: Theme, + props?: Partial>, + selectedLink?: HTMLElement, + ref?: React.RefObject +) => ({ + position: 'relative', + display: 'inline-flex', + color: 'fg.default', + textAlign: 'center', + textDecoration: 'none', + paddingX: 1, + ...(props?.variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), + '&:hover > div[data-component="wrapper"] ': { + backgroundColor: theme?.colors.neutral.muted, + transition: 'background .12s ease-out' + }, + '&:focus': { + outline: 0, + '& > div[data-component="wrapper"]': { + boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}` + }, + // where focus-visible is supported, remove the focus box-shadow + '&:not(:focus-visible) > div[data-component="wrapper"]': { + boxShadow: 'none' + } + }, + '&:focus-visible > div[data-component="wrapper"]': { + boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}` + }, + // renders a visibly hidden "copy" of the label in bold, reserving box space for when label becomes bold on selected + '& span[data-content]::before': { + content: 'attr(data-content)', + display: 'block', + height: 0, + fontWeight: '600', + visibility: 'hidden' + }, + // selected state styles + '&::after': { + position: 'absolute', + right: '50%', + bottom: 0, + width: `calc(100% - 8px)`, + height: 2, + content: '""', + bg: selectedLink === ref ? theme?.colors.primer.border.active : 'transparent', + borderRadius: 0, + transform: 'translate(50%, -50%)' + } +}) diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index e69de29bb2d..d5ecfd56b65 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -0,0 +1,6 @@ +export type ChildWidthArray = Array<{width: number}> +export type ResponsiveProps = { + items: Array + actions: Array + overflowStyles: React.CSSProperties +} From 9c8c8c79a3ec33d6742dc70b7aece43b2e01b279 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 12 Sep 2022 09:32:11 +1000 Subject: [PATCH 06/29] revert back stories to original state --- src/UnderlineNav2/examples.stories.tsx | 52 +++++++++----------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 3977af2696e..22b5e25e244 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -1,16 +1,5 @@ import React from 'react' -import { - EyeIcon, - CodeIcon, - IssueOpenedIcon, - GitPullRequestIcon, - CommentDiscussionIcon, - ProjectIcon, - ShieldIcon, - GearIcon, - GraphIcon, - PlayIcon -} from '@primer/octicons-react' +import {EyeIcon, CodeIcon, IssueOpenedIcon, GitPullRequestIcon, CommentDiscussionIcon} from '@primer/octicons-react' import {Meta} from '@storybook/react' import {UnderlineNav, UnderlineNavProps} from './index' import {BaseStyles, ThemeProvider} from '..' @@ -97,17 +86,17 @@ export const rightAlign = (args: UnderlineNavProps) => { ) } -const items: any[] = [ - {title: 'Code', icon: CodeIcon}, - {title: 'Issues', icon: IssueOpenedIcon, counter: 12}, - {title: 'Pull Requests', icon: GitPullRequestIcon, counter: 6}, - {title: 'Discussions', icon: CommentDiscussionIcon, counter: 7}, - {title: 'Actions', icon: PlayIcon, counter: 1}, - {title: 'Projects', icon: ProjectIcon, counter: 22}, - {title: 'Wiki', icon: EyeIcon, counter: 9}, - {title: 'Security', icon: ShieldIcon, counter: 7}, - {title: 'Insights', icon: GraphIcon, counter: 123}, - {title: 'Settings', icon: GearIcon, counter: 7} +const items: string[] = [ + 'Item 1', + 'Looooong Item', + 'Looooooonger item', + 'Item 4', + 'Item 5', + 'Item 6', + 'Item 7', + 'Item 8', + 'Item 9', + 'Item 10' ] export const InternalResponsiveNav = (args: UnderlineNavProps) => { @@ -117,13 +106,12 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { {items.map((item, index) => ( setSelectedIndex(index)} - counter={item.counter} > - {item.title} + {item} ))} @@ -131,16 +119,10 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { } export const HorizontalScrollNav = (args: UnderlineNavProps) => { - const [selectedIndex, setSelectedIndex] = React.useState(2) return ( - {items.map((item, index) => ( - setSelectedIndex(index)} - > + {items.map(item => ( + {item} ))} From 26c4e7bff2d0add9df4ce8b4328a6c387c8bf964 Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Mon, 12 Sep 2022 15:18:31 +1000 Subject: [PATCH 07/29] Fix types and align/variant issues --- src/UnderlineNav2/UnderlineNav.tsx | 36 +++++++------ src/UnderlineNav2/UnderlineNavItem.tsx | 6 +-- .../{getUnderlineNavStyles.ts => styles.ts} | 50 ++++++++++--------- 3 files changed, 46 insertions(+), 46 deletions(-) rename src/UnderlineNav2/{getUnderlineNavStyles.ts => styles.ts} (84%) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 6c8dc5a232c..0d7392c4baa 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -30,7 +30,7 @@ import { leftArrowBtnStyles, rightArrowBtnStyles, hiddenBtn -} from './getUnderlineNavStyles' +} from './styles' export type UnderlineNavProps = { label: string @@ -245,18 +245,17 @@ export const UnderlineNav = forwardRef( (getNavStyles(theme, align), sxProp)} + sx={merge(getNavStyles(theme, {align}), sxProp)} aria-label={label} ref={newRef} > - {scrollValues.scrollLeft > 0 && ( - ) => scrollWithButton(e, -1)} - icon={ChevronLeftIcon} - sx={scrollValues.scrollLeft > 0 ? leftArrowBtnStyles : hiddenBtn} - /> - )} + ) => scrollWithButton(e, -1)} + icon={ChevronLeftIcon} + sx={scrollValues.scrollLeft > 0 ? leftArrowBtnStyles : hiddenBtn} + /> + (responsiveProps.overflowStyles, ulStyles)} @@ -264,15 +263,14 @@ export const UnderlineNav = forwardRef( > {responsiveProps.items} - {scrollValues.scrollRight > 0 && ( - ) => scrollWithButton(e, 1)} - icon={ChevronRightIcon} - sx={scrollValues.scrollRight > 0 ? rightArrowBtnStyles : hiddenBtn} - /> - )} + + ) => scrollWithButton(e, 1)} + icon={ChevronRightIcon} + sx={scrollValues.scrollRight > 0 ? rightArrowBtnStyles : hiddenBtn} + /> {actions.length > 0 && ( diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 4f551ae89e3..4735449abd0 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -6,7 +6,7 @@ import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/po import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' -import {getLinkStyles, wrapperStyles, iconWrapStyles, textStyles, counterStyles} from './getUnderlineNavStyles' +import {getLinkStyles, wrapperStyles, iconWrapStyles, textStyles, counterStyles} from './styles' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -62,7 +62,7 @@ export const UnderlineNavItem = forwardRef( forwardedRef ) => { const backupRef = useRef(null) - const ref = forwardedRef ?? backupRef + const ref = (forwardedRef ?? backupRef) as RefObject const { setChildrenWidth, setNoIconChildrenWidth, @@ -118,7 +118,7 @@ export const UnderlineNavItem = forwardRef( onKeyPress={keyPressHandler} onClick={clickHandler} {...(selectedLink === ref ? {'aria-current': 'page'} : {})} - sx={merge(getLinkStyles(theme, variant, selectedLink, ref), sxProp as SxProp)} + sx={merge(getLinkStyles(theme, {variant}, selectedLink, ref), sxProp as SxProp)} {...props} ref={ref} > diff --git a/src/UnderlineNav2/getUnderlineNavStyles.ts b/src/UnderlineNav2/styles.ts similarity index 84% rename from src/UnderlineNav2/getUnderlineNavStyles.ts rename to src/UnderlineNav2/styles.ts index 37518689924..eb2e5933422 100644 --- a/src/UnderlineNav2/getUnderlineNavStyles.ts +++ b/src/UnderlineNav2/styles.ts @@ -74,46 +74,48 @@ export const arrowBtnStyles = { ...moreBtnStyles, paddingX: 0, paddingY: 0, - position: 'relative' + opacity: 1, + transition: 'opacity 1s ease' } export const hiddenBtn = { ...arrowBtnStyles, - display: 'none' + opacity: 0 } export const leftArrowBtnStyles = { - ...arrowBtnStyles, - '&::after': { - content: '""', - position: 'absolute', - background: 'linear-gradient(to left,#fff0,#fff)', - height: '100%', - width: '30px', - zIndex: 1, - left: '16px', - pointerEvents: 'none' - } + ...arrowBtnStyles + // '&::after': { + // content: '""', + // position: 'absolute', + // background: 'linear-gradient(to left,#fff0,#fff)', + // height: '100%', + // width: '30px', + // zIndex: 1, + // left: '16px', + // pointerEvents: 'none' + // } } export const rightArrowBtnStyles = { ...arrowBtnStyles, - '&::before': { - position: 'absolute', - background: 'linear-gradient(to right,#fff0,#fff)', - content: '""', - height: '100%', - left: '-30px', - width: '30px', - zIndex: 1, - pointerEvents: 'none' - } + opacity: 1 + // '&::before': { + // position: 'absolute', + // background: 'linear-gradient(to right,#fff0,#fff)', + // content: '""', + // height: '100%', + // left: '-30px', + // width: '30px', + // zIndex: 1, + // pointerEvents: 'none' + // } } export const getLinkStyles = ( theme?: Theme, props?: Partial>, - selectedLink?: HTMLElement, + selectedLink?: React.RefObject, ref?: React.RefObject ) => ({ position: 'relative', From 912fc49670d582f234d4d238037b8fb53202cede Mon Sep 17 00:00:00 2001 From: Pavithra Kodmad Date: Mon, 12 Sep 2022 15:44:39 +1000 Subject: [PATCH 08/29] Use separate component for arrow buttons --- src/UnderlineNav2/UnderlineNav.tsx | 34 +++--------- src/UnderlineNav2/UnderlineNavArrowButton.tsx | 53 +++++++++++++++++++ src/UnderlineNav2/styles.ts | 49 +++++++++-------- src/UnderlineNav2/types.ts | 2 + 4 files changed, 86 insertions(+), 52 deletions(-) create mode 100644 src/UnderlineNav2/UnderlineNavArrowButton.tsx diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 0d7392c4baa..aa7bc97ce13 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -12,25 +12,16 @@ import Box from '../Box' import {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' import {ActionMenu} from '../ActionMenu' -import {IconButton} from '../Button/IconButton' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' -import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' -import {ChildWidthArray, ResponsiveProps} from './types' - -import { - moreBtnStyles, - getDividerStyle, - getNavStyles, - ulStyles, - leftArrowBtnStyles, - rightArrowBtnStyles, - hiddenBtn -} from './styles' +import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' + +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, hiddenBtn} from './styles' +import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' export type UnderlineNavProps = { label: string @@ -224,7 +215,7 @@ export const UnderlineNav = forwardRef( } }, [selectedLink]) - const scrollWithButton = (event: React.MouseEvent, direction: -1 | 1) => { + const onScrollWithButton: OnScrollWithButtonEventType = (event, direction) => { if (!listRef.current) return const ScrollAmount = direction * 200 listRef.current.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) @@ -249,12 +240,7 @@ export const UnderlineNav = forwardRef( aria-label={label} ref={newRef} > - ) => scrollWithButton(e, -1)} - icon={ChevronLeftIcon} - sx={scrollValues.scrollLeft > 0 ? leftArrowBtnStyles : hiddenBtn} - /> + 0} onScrollWithButton={onScrollWithButton} /> - ) => scrollWithButton(e, 1)} - icon={ChevronRightIcon} - sx={scrollValues.scrollRight > 0 ? rightArrowBtnStyles : hiddenBtn} - /> + 0} onScrollWithButton={onScrollWithButton} /> {actions.length > 0 && ( diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx new file mode 100644 index 00000000000..39174950cc3 --- /dev/null +++ b/src/UnderlineNav2/UnderlineNavArrowButton.tsx @@ -0,0 +1,53 @@ +import React from 'react' +import {IconButton, Box} from '..' +import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' +import { + arrowParentStyles, + hiddenBtn, + leftArrowFadeEffectStyles, + rightArrowFadeEffectStyles, + arrowBtnStyles +} from './styles' +import {OnScrollWithButtonEventType} from './types' + +const LeftArrowButton = ({ + show, + onScrollWithButton +}: { + show: boolean + onScrollWithButton: OnScrollWithButtonEventType +}) => { + return ( + + ) => onScrollWithButton(e, -1)} + icon={ChevronLeftIcon} + sx={show ? arrowBtnStyles : hiddenBtn} + /> + {show && } + + ) +} + +const RightArrowButton = ({ + show, + onScrollWithButton +}: { + show: boolean + onScrollWithButton: OnScrollWithButtonEventType +}) => { + return ( + + {show && } + ) => onScrollWithButton(e, 1)} + icon={ChevronRightIcon} + sx={show ? arrowBtnStyles : hiddenBtn} + /> + + ) +} + +export {LeftArrowButton, RightArrowButton} diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index eb2e5933422..a0d9e688044 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -70,8 +70,15 @@ export const moreBtnStyles = { paddingX: 2 } +export const arrowParentStyles: BetterSystemStyleObject = { + position: 'relative', + zIndex: 1, + height: '100%' +} + export const arrowBtnStyles = { ...moreBtnStyles, + position: 'absolute', paddingX: 0, paddingY: 0, opacity: 1, @@ -83,33 +90,25 @@ export const hiddenBtn = { opacity: 0 } -export const leftArrowBtnStyles = { - ...arrowBtnStyles - // '&::after': { - // content: '""', - // position: 'absolute', - // background: 'linear-gradient(to left,#fff0,#fff)', - // height: '100%', - // width: '30px', - // zIndex: 1, - // left: '16px', - // pointerEvents: 'none' - // } +export const leftArrowFadeEffectStyles: BetterSystemStyleObject = { + content: '""', + position: 'absolute', + background: 'linear-gradient(to left,#fff0,#fff)', + height: '30p', + width: '30px', + left: '16px', + pointerEvents: 'none' } -export const rightArrowBtnStyles = { - ...arrowBtnStyles, - opacity: 1 - // '&::before': { - // position: 'absolute', - // background: 'linear-gradient(to right,#fff0,#fff)', - // content: '""', - // height: '100%', - // left: '-30px', - // width: '30px', - // zIndex: 1, - // pointerEvents: 'none' - // } +export const rightArrowFadeEffectStyles: BetterSystemStyleObject = { + position: 'absolute', + background: 'linear-gradient(to right,#fff0,#fff)', + content: '""', + height: '100%', + left: '-30px', + width: '30px', + + pointerEvents: 'none' } export const getLinkStyles = ( diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index d5ecfd56b65..60ea2ae825b 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -4,3 +4,5 @@ export type ResponsiveProps = { actions: Array overflowStyles: React.CSSProperties } + +export type OnScrollWithButtonEventType = (event: React.MouseEvent, direction: -1 | 1) => void From e5a08358ed386e5c65c3abc6b08a589193034fe2 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 12 Sep 2022 21:52:37 +1000 Subject: [PATCH 09/29] fade out affects on scroll ends --- src/UnderlineNav2/UnderlineNav.tsx | 11 ++- src/UnderlineNav2/UnderlineNavArrowButton.tsx | 40 ++++------ src/UnderlineNav2/styles.ts | 74 ++++++++++++------- 3 files changed, 72 insertions(+), 53 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index aa7bc97ce13..e079687e93a 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -20,7 +20,7 @@ import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' -import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, hiddenBtn} from './styles' +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles} from './styles' import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' export type UnderlineNavProps = { @@ -73,6 +73,15 @@ const overflowEffect = ( items.push(...childArray) overflowStyles.overflowX = 'auto' overflowStyles.scrollbarWidth = 'none' + + // Hide scrollbar on Firefox + overflowStyles.scrollbarWidth = 'none' + // Hide scrollbar on IE 10+ + overflowStyles.msOverflowStyle = 'none' + // Hide scrollbar on Chrome, Safari, Edge + overflowStyles['&::-webkit-scrollbar'] = { + display: 'none' + } } else { // This is only for the overflow behaviour (for fine pointers) // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx index 39174950cc3..dc93bf947fb 100644 --- a/src/UnderlineNav2/UnderlineNavArrowButton.tsx +++ b/src/UnderlineNav2/UnderlineNavArrowButton.tsx @@ -1,13 +1,7 @@ import React from 'react' -import {IconButton, Box} from '..' +import {IconButton} from '../Button/IconButton' import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' -import { - arrowParentStyles, - hiddenBtn, - leftArrowFadeEffectStyles, - rightArrowFadeEffectStyles, - arrowBtnStyles -} from './styles' +import {leftArrawHiddenBtn, rightArrowHiddenBtn, leftArrowVisibleBtn, rightArrowVisibleBtn} from './styles' import {OnScrollWithButtonEventType} from './types' const LeftArrowButton = ({ @@ -18,15 +12,12 @@ const LeftArrowButton = ({ onScrollWithButton: OnScrollWithButtonEventType }) => { return ( - - ) => onScrollWithButton(e, -1)} - icon={ChevronLeftIcon} - sx={show ? arrowBtnStyles : hiddenBtn} - /> - {show && } - + ) => onScrollWithButton(e, -1)} + icon={ChevronLeftIcon} + sx={show ? leftArrowVisibleBtn : leftArrawHiddenBtn} + /> ) } @@ -38,15 +29,12 @@ const RightArrowButton = ({ onScrollWithButton: OnScrollWithButtonEventType }) => { return ( - - {show && } - ) => onScrollWithButton(e, 1)} - icon={ChevronRightIcon} - sx={show ? arrowBtnStyles : hiddenBtn} - /> - + ) => onScrollWithButton(e, 1)} + icon={ChevronRightIcon} + sx={show ? rightArrowVisibleBtn : rightArrowHiddenBtn} + /> ) } diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index a0d9e688044..ac9e8e54ff3 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -38,7 +38,8 @@ export const getNavStyles = (theme?: Theme, props?: Partial Date: Tue, 13 Sep 2022 14:15:29 +1000 Subject: [PATCH 10/29] type issues and some refactor --- src/UnderlineNav2/UnderlineNav.tsx | 38 +++++++++++++----------------- src/UnderlineNav2/styles.ts | 15 ++++++++++++ src/UnderlineNav2/types.ts | 3 ++- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index e079687e93a..c472ff7a65b 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -9,7 +9,7 @@ import React, { useEffect } from 'react' import Box from '../Box' -import {merge, BetterSystemStyleObject, SxProp} from '../sx' +import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' @@ -20,8 +20,9 @@ import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' -import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles} from './styles' +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, scrollStyles, moreMenuStyles} from './styles' import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' +import styled from 'styled-components' export type UnderlineNavProps = { label: string @@ -33,6 +34,11 @@ export type UnderlineNavProps = { children: React.ReactNode } +// Needed this because passing a ref using HTMLULListElement to `Box` causes a type error +const NavigationList = styled.ul` + ${sx}; +` + const handleArrowBtnsVisibility = ( scrollOffsets: {scrollLeft: number; scrollRight: number}, callback: (scroll: {scrollLeft: number; scrollRight: number}) => void @@ -48,16 +54,16 @@ const overflowEffect = ( callback: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true + let overflowStyles: BetterSystemStyleObject | null = {} if (childWidthArray.length === 0) { - callback({items: childArray, actions: [], overflowStyles: {}}, iconsVisible) + callback({items: childArray, actions: [], overflowStyles}, iconsVisible) } // do this only for overflow const numberOfItemsPossible = calculatePossibleItems(childWidthArray, width) const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, width) const items: Array = [] const actions: Array = [] - const overflowStyles: React.CSSProperties = {whiteSpace: 'nowrap'} // First we check if we can fit all the items with icons if (childArray.length <= numberOfItemsPossible) { @@ -70,20 +76,12 @@ const overflowEffect = ( iconsVisible = false if (isCoarsePointer) { + // Scroll behaviour for coarse pointer devices items.push(...childArray) - overflowStyles.overflowX = 'auto' - overflowStyles.scrollbarWidth = 'none' - - // Hide scrollbar on Firefox - overflowStyles.scrollbarWidth = 'none' - // Hide scrollbar on IE 10+ - overflowStyles.msOverflowStyle = 'none' - // Hide scrollbar on Chrome, Safari, Edge - overflowStyles['&::-webkit-scrollbar'] = { - display: 'none' - } + overflowStyles = scrollStyles } else { - // This is only for the overflow behaviour (for fine pointers) + // More menu behaviour for fine pointer devices + overflowStyles = moreMenuStyles // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu for (const [index, child] of childArray.entries()) { if (index < numberOfItemsWithoutIconPossible) { @@ -251,13 +249,9 @@ export const UnderlineNav = forwardRef( > 0} onScrollWithButton={onScrollWithButton} /> - (responsiveProps.overflowStyles, ulStyles)} - ref={listRef as RefObject} - > + (responsiveProps.overflowStyles, ulStyles)} ref={listRef}> {responsiveProps.items} - + 0} onScrollWithButton={onScrollWithButton} /> diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index ac9e8e54ff3..c4a752e4ae6 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -184,3 +184,18 @@ export const getLinkStyles = ( transform: 'translate(50%, -50%)' } }) + +export const scrollStyles: BetterSystemStyleObject = { + whiteSpace: 'nowrap', + overflowX: 'auto', + // Hiding scrollbar on Firefox + scrollbarWidth: 'none', + // Hiding scrollbar on IE 10+ + msOverflowStyle: 'none', + // Hiding scrollbar on Chrome, Safari and Opera + '&::-webkit-scrollbar': { + display: 'none' + } +} + +export const moreMenuStyles: BetterSystemStyleObject = {whiteSpace: 'nowrap'} diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index 60ea2ae825b..635213764e3 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -1,8 +1,9 @@ +import {BetterSystemStyleObject} from '../sx' export type ChildWidthArray = Array<{width: number}> export type ResponsiveProps = { items: Array actions: Array - overflowStyles: React.CSSProperties + overflowStyles: BetterSystemStyleObject } export type OnScrollWithButtonEventType = (event: React.MouseEvent, direction: -1 | 1) => void From e7aae171238a992ab43d0a42e4dc6553367a9bac Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Tue, 13 Sep 2022 17:41:41 +1000 Subject: [PATCH 11/29] style refactor & theming & scrollIntoView from primer behaviour --- src/UnderlineNav2/UnderlineNav.test.tsx | 8 ++-- src/UnderlineNav2/UnderlineNav.tsx | 17 +++++-- src/UnderlineNav2/UnderlineNavArrowButton.tsx | 9 ++-- src/UnderlineNav2/styles.ts | 48 +++++++++++-------- 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index df2090df2a7..399b122ebdf 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -3,9 +3,9 @@ import '@testing-library/jest-dom/extend-expect' import {render} from '@testing-library/react' import {UnderlineNav} from '.' - +// TODO: Fix the scrollintoview is not a function issue that affects all of the tests describe('UnderlineNav', () => { - test('selected nav', () => { + test.skip('selected nav', () => { const {getByText} = render( Item 1 @@ -17,7 +17,7 @@ describe('UnderlineNav', () => { expect(selectedNavLink?.getAttribute('aria-current')).toBe('page') }) - test('basic nav functionality', () => { + test.skip('basic nav functionality', () => { const {container} = render( Item 1 @@ -30,7 +30,7 @@ describe('UnderlineNav', () => { expect(nav.getAttribute('aria-label')).toBe('Test nav') }) - test('respect align prop', () => { + test.skip('respect align prop', () => { const {container} = render( Item 1 diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index c472ff7a65b..0cc8a5152ca 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -15,7 +15,7 @@ import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import {useFocusZone} from '../hooks/useFocusZone' -import {FocusKeys} from '@primer/behaviors' +import {FocusKeys, scrollIntoView} from '@primer/behaviors' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' @@ -24,6 +24,8 @@ import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, scrollStyles, mo import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' import styled from 'styled-components' +import type {ScrollIntoViewOptions} from '@primer/behaviors' + export type UnderlineNavProps = { label: string as?: React.ElementType @@ -34,6 +36,15 @@ export type UnderlineNavProps = { children: React.ReactNode } +const ARROW_BTN_WIDTH = 36 + +const underlineNavScrollMargins: ScrollIntoViewOptions = { + startMargin: ARROW_BTN_WIDTH, + endMargin: ARROW_BTN_WIDTH, + direction: 'horizontal', + behavior: 'smooth' +} + // Needed this because passing a ref using HTMLULListElement to `Box` causes a type error const NavigationList = styled.ul` ${sx}; @@ -217,8 +228,8 @@ export const UnderlineNav = forwardRef( useEffect(() => { // scroll the selected link into the view - if (selectedLink) { - selectedLink.current?.scrollIntoView({behavior: 'smooth', block: 'nearest', inline: 'nearest'}) + if (selectedLink && selectedLink.current && listRef.current) { + scrollIntoView(selectedLink.current, listRef.current, underlineNavScrollMargins) } }, [selectedLink]) diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx index dc93bf947fb..b36192aefbe 100644 --- a/src/UnderlineNav2/UnderlineNavArrowButton.tsx +++ b/src/UnderlineNav2/UnderlineNavArrowButton.tsx @@ -1,8 +1,9 @@ import React from 'react' import {IconButton} from '../Button/IconButton' import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' -import {leftArrawHiddenBtn, rightArrowHiddenBtn, leftArrowVisibleBtn, rightArrowVisibleBtn} from './styles' +import {getLeftArrowHiddenBtn, getRightArrowHiddenBtn, getLeftArrowVisibleBtn, getRightArrowVisibleBtn} from './styles' import {OnScrollWithButtonEventType} from './types' +import {useTheme} from '../ThemeProvider' const LeftArrowButton = ({ show, @@ -11,12 +12,13 @@ const LeftArrowButton = ({ show: boolean onScrollWithButton: OnScrollWithButtonEventType }) => { + const {theme} = useTheme() return ( ) => onScrollWithButton(e, -1)} icon={ChevronLeftIcon} - sx={show ? leftArrowVisibleBtn : leftArrawHiddenBtn} + sx={show ? getLeftArrowVisibleBtn(theme) : getLeftArrowHiddenBtn(theme)} /> ) } @@ -28,12 +30,13 @@ const RightArrowButton = ({ show: boolean onScrollWithButton: OnScrollWithButtonEventType }) => { + const {theme} = useTheme() return ( ) => onScrollWithButton(e, 1)} icon={ChevronRightIcon} - sx={show ? rightArrowVisibleBtn : rightArrowHiddenBtn} + sx={show ? getRightArrowVisibleBtn(theme) : getRightArrowHiddenBtn(theme)} /> ) } diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index c4a752e4ae6..b147e7bf0ac 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -71,36 +71,42 @@ export const moreBtnStyles = { paddingX: 2 } -export const arrowBtnStyles = { - ...moreBtnStyles, - background: 'white', - position: 'absolute', +export const getArrowBtnStyles = (theme?: Theme) => ({ + fontWeight: 'normal', + boxShadow: 'none', + margin: 0, + border: 0, + borderRadius: 0, paddingX: 0, paddingY: 0, + background: theme?.colors.canvas.default, + position: 'absolute', opacity: 1, transition: 'opacity 250ms ease-out', - zIndex: 1 -} + zIndex: 1, + '&:hover:not([disabled]), &:focus-visible': { + background: theme?.colors.canvas.default + } +}) -export const leftArrawHiddenBtn = { - ...arrowBtnStyles, +export const getLeftArrowHiddenBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), opacity: 0, top: 0, bottom: 0, left: 0 -} +}) -export const rightArrowHiddenBtn = { - ...arrowBtnStyles, +export const getRightArrowHiddenBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), opacity: 0, top: 0, bottom: 0, right: 0 -} +}) -// TODO: make the linear gradient and background theme aware -export const leftArrowVisibleBtn = { - ...arrowBtnStyles, +export const getLeftArrowVisibleBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), top: 0, bottom: 0, left: 0, @@ -108,30 +114,30 @@ export const leftArrowVisibleBtn = { content: '""', position: 'absolute', top: 0, - background: 'linear-gradient(to left,#fff0,#fff)', + background: `linear-gradient(to left,#fff0,${theme?.colors.canvas.default})`, height: '100%', width: '20px', right: '-20px', pointerEvents: 'none' } -} +}) -export const rightArrowVisibleBtn = { - ...arrowBtnStyles, +export const getRightArrowVisibleBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), top: 0, bottom: 0, right: 0, '&::before': { position: 'absolute', top: 0, - background: 'linear-gradient(to right,#fff0,#fff)', + background: `linear-gradient(to right,#fff0,${theme?.colors.canvas.default})`, content: '""', height: '100%', left: '-20px', width: '20px', pointerEvents: 'none' } -} +}) export const getLinkStyles = ( theme?: Theme, From 2ca3a037c1f0dc78ebde83a126d3a20aa8f0f2a0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 14 Sep 2022 12:03:33 +1000 Subject: [PATCH 12/29] Update documentation --- docs/content/drafts/UnderlineNav2.mdx | 121 ++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 18 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index 402b9054ef7..ccfbc6d4755 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -1,38 +1,129 @@ --- title: UnderlineNav v2 +componentId: underline_nav_2 status: Draft description: Use an underlined nav to allow tab like navigation with overflow behaviour in your UI. +source: https://github.com/primer/react/tree/main/src/UnderlineNav2 +storybook: https://primer.style/react/storybook/?path=/story/layout-underlinenav --- +```js +import {UnderlineNav} from '@primer/react/drafts' +``` + ## Examples ### Simple ```jsx live drafts - + Item 1 Item 2 Item 3 ``` +### Small variant + +```jsx live drafts + + Item 1 + Item 2 + +``` + ### With icons ```jsx live drafts - - - Item 1 + + + Code + + + Issues + + + Pull requests + + Discussions + + Actions + + + Projects - Item 2 ``` -### Small variant +### Overflow Behaviour + +#### Items without Icons + +When overflow occurs, the component first hides icons if present to optimize for space and show as many items as possible. +If there is still overflow, the component will behave depending on the pointer. Please see below to see these behaviours. ```jsx live drafts - - Item 1 - Item 2 + + + Code + + + Issues + + + Pull requests + + Discussions + + Actions + + + Projects + + Security + Insights + + Settings + + +``` + +#### Display `More` menu for fine pointer devices + +This overflow behaviour will occur on fine pointer devices that have fine pointers. + +#### Scroll for coarse pointer devices + +The scroll behaviour will occur on devices that have coarse pointers. + +Depending on your device when viweing this example, you may see the scroll behaviour or the `More` menu behaviour. + +```jsx live drafts + + + Code + + + Issues + + + Pull requests + + Discussions + + Actions + + + Projects + + Security + + Insights + + + Settings + + Wiki ``` @@ -44,12 +135,6 @@ description: Use an underlined nav to allow tab like navigation with overflow be - Date: Wed, 14 Sep 2022 14:46:27 +1000 Subject: [PATCH 13/29] Update documentation --- docs/content/drafts/UnderlineNav2.mdx | 10 +++--- src/UnderlineNav2/examples.stories.tsx | 42 +++++++++++++++++--------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index ccfbc6d4755..f95e549ed92 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -74,15 +74,15 @@ If there is still overflow, the component will behave depending on the pointer. Pull requests Discussions - + Actions - + Projects - Security - Insights - + Security + Insights + Settings diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 22b5e25e244..8ccd472bff6 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -1,5 +1,17 @@ import React from 'react' -import {EyeIcon, CodeIcon, IssueOpenedIcon, GitPullRequestIcon, CommentDiscussionIcon} from '@primer/octicons-react' +import { + IconProps, + EyeIcon, + CodeIcon, + IssueOpenedIcon, + GitPullRequestIcon, + CommentDiscussionIcon, + PlayIcon, + ProjectIcon, + GraphIcon, + ShieldLockIcon, + GearIcon +} from '@primer/octicons-react' import {Meta} from '@storybook/react' import {UnderlineNav, UnderlineNavProps} from './index' import {BaseStyles, ThemeProvider} from '..' @@ -86,17 +98,16 @@ export const rightAlign = (args: UnderlineNavProps) => { ) } -const items: string[] = [ - 'Item 1', - 'Looooong Item', - 'Looooooonger item', - 'Item 4', - 'Item 5', - 'Item 6', - 'Item 7', - 'Item 8', - 'Item 9', - 'Item 10' +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', icon: PlayIcon, counter: 4}, + {navigation: 'Projects', icon: ProjectIcon, counter: 9}, + {navigation: 'Insights', icon: GraphIcon}, + {navigation: 'Settings', icon: GearIcon, counter: 10}, + {navigation: 'Security', icon: ShieldLockIcon} ] export const InternalResponsiveNav = (args: UnderlineNavProps) => { @@ -106,12 +117,13 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { {items.map((item, index) => ( setSelectedIndex(index)} + counter={item.counter} > - {item} + {item.navigation} ))} From d650a99a91d21e8d6a40d813bb6a51f773222ba0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 14 Sep 2022 14:48:26 +1000 Subject: [PATCH 14/29] add changeset --- .changeset/sweet-eggs-complain.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sweet-eggs-complain.md diff --git a/.changeset/sweet-eggs-complain.md b/.changeset/sweet-eggs-complain.md new file mode 100644 index 00000000000..2e58e48957e --- /dev/null +++ b/.changeset/sweet-eggs-complain.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Introducing overflow behaviour for fine and coarse pointer devices From 09189106db96a8f9a447efbe4e42d60e691a683e Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 14 Sep 2022 14:59:26 +1000 Subject: [PATCH 15/29] remove scroll story until finding a way to simulate coarse pointer --- src/UnderlineNav2/examples.stories.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 8ccd472bff6..4583cc5f86c 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -129,15 +129,3 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { ) } - -export const HorizontalScrollNav = (args: UnderlineNavProps) => { - return ( - - {items.map(item => ( - - {item} - - ))} - - ) -} From 33a7cc0f30d15fd2ddfbb0c26c7dcdf840c0f534 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 14 Sep 2022 19:01:11 +1000 Subject: [PATCH 16/29] add single variant selection to ActionList --- src/UnderlineNav2/UnderlineNav.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 0cc8a5152ca..eb6c8f4c681 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -272,7 +272,7 @@ export const UnderlineNav = forwardRef( More - + {actions.map((action, index) => { const {children: actionElementChildren, ...actionElementProps} = action.props return ( From 1cebcd208f4b0cdf34db3a9307ec3d58dd4b3fae Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Sun, 18 Sep 2022 18:40:11 +1000 Subject: [PATCH 17/29] take more button into account when calculating & code review feedback --- src/UnderlineNav2/UnderlineNav.tsx | 30 +++++++++++++++++++------- src/UnderlineNav2/UnderlineNavItem.tsx | 14 ++++++------ src/UnderlineNav2/examples.stories.tsx | 16 +++++++------- src/UnderlineNav2/styles.ts | 9 +++----- 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index eb6c8f4c681..6e7f623da93 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -57,7 +57,8 @@ const handleArrowBtnsVisibility = ( callback(scrollOffsets) } const overflowEffect = ( - width: number, + navWidth: number, + moreMenuWidth: number, childArray: Array, childWidthArray: ChildWidthArray, noIconChildWidthArray: ChildWidthArray, @@ -70,9 +71,9 @@ const overflowEffect = ( if (childWidthArray.length === 0) { callback({items: childArray, actions: [], overflowStyles}, iconsVisible) } - // do this only for overflow - const numberOfItemsPossible = calculatePossibleItems(childWidthArray, width) - const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, width) + + const numberOfItemsPossible = calculatePossibleItems(childWidthArray, navWidth) + const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, navWidth, moreMenuWidth) const items: Array = [] const actions: Array = [] @@ -117,17 +118,20 @@ function calculateScrollOffset(scrollableList: RefObject) { return {scrollLeft, scrollRight} } -function calculatePossibleItems(childWidthArray: ChildWidthArray, width: number) { +function calculatePossibleItems(childWidthArray: ChildWidthArray, navWidth: number, moreMenuWidth = 0) { + // 100 is the margin that we want to leave on the right side of the nav + const widthToFit = navWidth - moreMenuWidth - 100 let breakpoint = childWidthArray.length - 1 let sumsOfChildWidth = 0 for (const [index, childWidth] of childWidthArray.entries()) { - if (sumsOfChildWidth > 0.8 * width) { + if (sumsOfChildWidth > widthToFit) { breakpoint = index break } else { sumsOfChildWidth = sumsOfChildWidth + childWidth.width } } + return breakpoint } @@ -139,6 +143,7 @@ export const UnderlineNav = forwardRef( const backupRef = useRef(null) const newRef = (forwardedRef ?? backupRef) as MutableRefObject const listRef = useRef(null) + const moreMenuRef = useRef(null) const {theme} = useTheme() @@ -204,9 +209,18 @@ export const UnderlineNav = forwardRef( (resizeObserverEntries: ResizeObserverEntry[]) => { const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width + const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 const scrollOffsets = calculateScrollOffset(listRef) - overflowEffect(navWidth, childArray, childWidthArray, noIconChildWidthArray, isCoarsePointer, callback) + overflowEffect( + navWidth, + moreMenuWidth, + childArray, + childWidthArray, + noIconChildWidthArray, + isCoarsePointer, + callback + ) handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) }, @@ -267,7 +281,7 @@ export const UnderlineNav = forwardRef( 0} onScrollWithButton={onScrollWithButton} /> {actions.length > 0 && ( - + More diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 4735449abd0..b9bb09e83d7 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -6,7 +6,7 @@ import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/po import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' -import {getLinkStyles, wrapperStyles, iconWrapStyles, textStyles, counterStyles} from './styles' +import {getLinkStyles, wrapperStyles, iconWrapStyles, counterStyles} from './styles' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -37,7 +37,7 @@ export type UnderlineNavItemProps = { /** * Icon before the text */ - leadingIcon?: React.FunctionComponent + icon?: React.FunctionComponent as?: React.ElementType /** * Counter @@ -56,7 +56,7 @@ export const UnderlineNavItem = forwardRef( counter, onSelect, selected: preSelected = false, - leadingIcon: LeadingIcon, + icon: Icon, ...props }, forwardedRef @@ -123,9 +123,9 @@ export const UnderlineNavItem = forwardRef( ref={ref} > - {iconsVisible && LeadingIcon && ( - - + {iconsVisible && Icon && ( + + )} {children && ( @@ -133,7 +133,7 @@ export const UnderlineNavItem = forwardRef( as="span" data-component="text" data-content={children} - sx={selectedLink === ref ? {fontWeight: 600, ...{textStyles}} : {textStyles}} + sx={selectedLink === ref ? {fontWeight: 600} : {}} > {children} diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 4583cc5f86c..c82954551dc 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -60,17 +60,17 @@ export const DefaultNav = (args: UnderlineNavProps) => { export const withIcons = (args: UnderlineNavProps) => { return ( - Code - + Code + Issues - + Pull Requests - + Discussions - Item 1 + Item 1 ) } @@ -78,10 +78,10 @@ export const withIcons = (args: UnderlineNavProps) => { export const withCounterLabels = (args: UnderlineNavProps) => { return ( - + Code - + Issues @@ -118,7 +118,7 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { {items.map((item, index) => ( setSelectedIndex(index)} counter={item.counter} diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index b147e7bf0ac..55bb4e892d9 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -8,10 +8,6 @@ export const iconWrapStyles = { marginRight: 2 } -export const textStyles: BetterSystemStyleObject = { - whiteSpace: 'nowrap' -} - export const wrapperStyles = { display: 'inline-flex', paddingY: 1, @@ -34,6 +30,7 @@ export const counterStyles = { export const getNavStyles = (theme?: Theme, props?: Partial>) => ({ display: 'flex', + paddingX: 2, justifyContent: props?.align === 'right' ? 'flex-end' : 'flex-start', borderBottom: '1px solid', borderBottomColor: `${theme?.colors.border.muted}`, @@ -46,7 +43,6 @@ export const ulStyles = { display: 'flex', listStyle: 'none', padding: '0', - paddingX: 1, margin: '0', marginBottom: '-1px', alignItems: 'center' @@ -175,7 +171,8 @@ export const getLinkStyles = ( display: 'block', height: 0, fontWeight: '600', - visibility: 'hidden' + visibility: 'hidden', + whiteSpace: 'nowrap' }, // selected state styles '&::after': { From eade73d521565c137395cb0a790d4721ca2274cc Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Sun, 18 Sep 2022 18:47:25 +1000 Subject: [PATCH 18/29] remove scroll behaviour - due to accessibility concerns --- src/UnderlineNav2/UnderlineNav.tsx | 120 ++---------------- src/UnderlineNav2/UnderlineNavArrowButton.tsx | 44 ------- src/UnderlineNav2/styles.ts | 81 ------------ src/UnderlineNav2/types.ts | 2 - 4 files changed, 14 insertions(+), 233 deletions(-) delete mode 100644 src/UnderlineNav2/UnderlineNavArrowButton.tsx diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 6e7f623da93..07df49eff6b 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -1,13 +1,4 @@ -import React, { - useRef, - useLayoutEffect, - forwardRef, - useCallback, - useState, - MutableRefObject, - RefObject, - useEffect -} from 'react' +import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react' import Box from '../Box' import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' @@ -15,17 +6,14 @@ import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import {useFocusZone} from '../hooks/useFocusZone' -import {FocusKeys, scrollIntoView} from '@primer/behaviors' +import {FocusKeys} from '@primer/behaviors' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' -import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' +import {ChildWidthArray, ResponsiveProps} from './types' -import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, scrollStyles, moreMenuStyles} from './styles' -import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, moreMenuStyles} from './styles' import styled from 'styled-components' -import type {ScrollIntoViewOptions} from '@primer/behaviors' - export type UnderlineNavProps = { label: string as?: React.ElementType @@ -36,33 +24,16 @@ export type UnderlineNavProps = { children: React.ReactNode } -const ARROW_BTN_WIDTH = 36 - -const underlineNavScrollMargins: ScrollIntoViewOptions = { - startMargin: ARROW_BTN_WIDTH, - endMargin: ARROW_BTN_WIDTH, - direction: 'horizontal', - behavior: 'smooth' -} - // Needed this because passing a ref using HTMLULListElement to `Box` causes a type error const NavigationList = styled.ul` ${sx}; ` - -const handleArrowBtnsVisibility = ( - scrollOffsets: {scrollLeft: number; scrollRight: number}, - callback: (scroll: {scrollLeft: number; scrollRight: number}) => void -) => { - callback(scrollOffsets) -} const overflowEffect = ( navWidth: number, moreMenuWidth: number, childArray: Array, childWidthArray: ChildWidthArray, noIconChildWidthArray: ChildWidthArray, - isCoarsePointer: boolean, callback: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true @@ -87,20 +58,14 @@ const overflowEffect = ( } else { iconsVisible = false - if (isCoarsePointer) { - // Scroll behaviour for coarse pointer devices - items.push(...childArray) - overflowStyles = scrollStyles - } else { - // More menu behaviour for fine pointer devices - overflowStyles = moreMenuStyles - // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu - for (const [index, child] of childArray.entries()) { - if (index < numberOfItemsWithoutIconPossible) { - items.push(child) - } else { - actions.push(child) - } + // More menu behaviour + overflowStyles = moreMenuStyles + // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + for (const [index, child] of childArray.entries()) { + if (index < numberOfItemsWithoutIconPossible) { + items.push(child) + } else { + actions.push(child) } } } @@ -112,12 +77,6 @@ function getValidChildren(children: React.ReactNode) { return React.Children.toArray(children).filter(child => React.isValidElement(child)) as React.ReactElement[] } -function calculateScrollOffset(scrollableList: RefObject) { - const {scrollLeft, scrollWidth, clientWidth} = scrollableList.current as HTMLElement - const scrollRight = scrollWidth - scrollLeft - clientWidth - return {scrollLeft, scrollRight} -} - function calculatePossibleItems(childWidthArray: ChildWidthArray, navWidth: number, moreMenuWidth = 0) { // 100 is the margin that we want to leave on the right side of the nav const widthToFit = navWidth - moreMenuWidth - 100 @@ -147,16 +106,9 @@ export const UnderlineNav = forwardRef( const {theme} = useTheme() - const [isCoarsePointer, setIsCoarsePointer] = useState(false) - const [selectedLink, setSelectedLink] = useState | undefined>(undefined) const [iconsVisible, setIconsVisible] = useState(true) - - const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ - scrollLeft: 0, - scrollRight: 0 - }) // This might change if we decide tab through the navigation items rather than navigationg with the arrow keys. // TBD. In the meantime keeping it as a menu with the focus trap. // ref: https://www.w3.org/WAI/ARIA/apg/example-index/menubar/menubar-navigation.html (Keyboard Support) @@ -183,10 +135,6 @@ export const UnderlineNav = forwardRef( setIconsVisible(displayIcons) }, []) - const updateOffsetValues = useCallback((scrollOffsets: {scrollLeft: number; scrollRight: number}) => { - setScrollValues(scrollOffsets) - }, []) - const actions = responsiveProps.actions const [childWidthArray, setChildWidthArray] = useState([]) const setChildrenWidth = useCallback(size => { @@ -210,49 +158,13 @@ export const UnderlineNav = forwardRef( const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 - const scrollOffsets = calculateScrollOffset(listRef) - - overflowEffect( - navWidth, - moreMenuWidth, - childArray, - childWidthArray, - noIconChildWidthArray, - isCoarsePointer, - callback - ) - handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) + overflowEffect(navWidth, moreMenuWidth, childArray, childWidthArray, noIconChildWidthArray, callback) }, - [callback, updateOffsetValues, childWidthArray, noIconChildWidthArray, children, isCoarsePointer] + [callback, childWidthArray, noIconChildWidthArray, children] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) - // Determine the pointer type on mount - useLayoutEffect(() => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - setIsCoarsePointer(window.matchMedia && window.matchMedia('(pointer:coarse)').matches) - // eslint-disable-next-line github/prefer-observers - listRef.current?.addEventListener('scroll', () => { - const scrollOffsets = calculateScrollOffset(listRef) - - handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) - }) - }, [updateOffsetValues]) - - useEffect(() => { - // scroll the selected link into the view - if (selectedLink && selectedLink.current && listRef.current) { - scrollIntoView(selectedLink.current, listRef.current, underlineNavScrollMargins) - } - }, [selectedLink]) - - const onScrollWithButton: OnScrollWithButtonEventType = (event, direction) => { - if (!listRef.current) return - const ScrollAmount = direction * 200 - listRef.current.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) - } - return ( - 0} onScrollWithButton={onScrollWithButton} /> - (responsiveProps.overflowStyles, ulStyles)} ref={listRef}> {responsiveProps.items} - 0} onScrollWithButton={onScrollWithButton} /> - {actions.length > 0 && ( diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx deleted file mode 100644 index b36192aefbe..00000000000 --- a/src/UnderlineNav2/UnderlineNavArrowButton.tsx +++ /dev/null @@ -1,44 +0,0 @@ -import React 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' - -const LeftArrowButton = ({ - show, - onScrollWithButton -}: { - show: boolean - onScrollWithButton: OnScrollWithButtonEventType -}) => { - const {theme} = useTheme() - return ( - ) => onScrollWithButton(e, -1)} - icon={ChevronLeftIcon} - sx={show ? getLeftArrowVisibleBtn(theme) : getLeftArrowHiddenBtn(theme)} - /> - ) -} - -const RightArrowButton = ({ - show, - onScrollWithButton -}: { - show: boolean - onScrollWithButton: OnScrollWithButtonEventType -}) => { - const {theme} = useTheme() - return ( - ) => onScrollWithButton(e, 1)} - icon={ChevronRightIcon} - sx={show ? getRightArrowVisibleBtn(theme) : getRightArrowHiddenBtn(theme)} - /> - ) -} - -export {LeftArrowButton, RightArrowButton} diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index 55bb4e892d9..0213791f0ef 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -67,74 +67,6 @@ export const moreBtnStyles = { paddingX: 2 } -export const getArrowBtnStyles = (theme?: Theme) => ({ - fontWeight: 'normal', - boxShadow: 'none', - margin: 0, - border: 0, - borderRadius: 0, - paddingX: 0, - paddingY: 0, - background: theme?.colors.canvas.default, - position: 'absolute', - opacity: 1, - transition: 'opacity 250ms ease-out', - zIndex: 1, - '&:hover:not([disabled]), &:focus-visible': { - background: theme?.colors.canvas.default - } -}) - -export const getLeftArrowHiddenBtn = (theme?: Theme) => ({ - ...getArrowBtnStyles(theme), - opacity: 0, - top: 0, - bottom: 0, - left: 0 -}) - -export const getRightArrowHiddenBtn = (theme?: Theme) => ({ - ...getArrowBtnStyles(theme), - opacity: 0, - top: 0, - bottom: 0, - right: 0 -}) - -export const getLeftArrowVisibleBtn = (theme?: Theme) => ({ - ...getArrowBtnStyles(theme), - top: 0, - bottom: 0, - left: 0, - '&::after': { - content: '""', - position: 'absolute', - top: 0, - background: `linear-gradient(to left,#fff0,${theme?.colors.canvas.default})`, - height: '100%', - width: '20px', - right: '-20px', - pointerEvents: 'none' - } -}) - -export const getRightArrowVisibleBtn = (theme?: Theme) => ({ - ...getArrowBtnStyles(theme), - top: 0, - bottom: 0, - right: 0, - '&::before': { - position: 'absolute', - top: 0, - background: `linear-gradient(to right,#fff0,${theme?.colors.canvas.default})`, - content: '""', - height: '100%', - left: '-20px', - width: '20px', - pointerEvents: 'none' - } -}) - export const getLinkStyles = ( theme?: Theme, props?: Partial>, @@ -188,17 +120,4 @@ export const getLinkStyles = ( } }) -export const scrollStyles: BetterSystemStyleObject = { - whiteSpace: 'nowrap', - overflowX: 'auto', - // Hiding scrollbar on Firefox - scrollbarWidth: 'none', - // Hiding scrollbar on IE 10+ - msOverflowStyle: 'none', - // Hiding scrollbar on Chrome, Safari and Opera - '&::-webkit-scrollbar': { - display: 'none' - } -} - export const moreMenuStyles: BetterSystemStyleObject = {whiteSpace: 'nowrap'} diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index 635213764e3..881825e3f54 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -5,5 +5,3 @@ export type ResponsiveProps = { actions: Array overflowStyles: BetterSystemStyleObject } - -export type OnScrollWithButtonEventType = (event: React.MouseEvent, direction: -1 | 1) => void From 50e8d782cc7b84680b5a267d522aca931b940b53 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 19 Sep 2022 09:16:02 +1000 Subject: [PATCH 19/29] hover state remove on mobile and improvments on more btn functionality --- src/UnderlineNav2/UnderlineNav.tsx | 10 ++++------ src/UnderlineNav2/styles.ts | 8 +++++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 07df49eff6b..152ce3c2532 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -78,13 +78,12 @@ function getValidChildren(children: React.ReactNode) { } function calculatePossibleItems(childWidthArray: ChildWidthArray, navWidth: number, moreMenuWidth = 0) { - // 100 is the margin that we want to leave on the right side of the nav - const widthToFit = navWidth - moreMenuWidth - 100 + const widthToFit = navWidth - moreMenuWidth let breakpoint = childWidthArray.length - 1 let sumsOfChildWidth = 0 for (const [index, childWidth] of childWidthArray.entries()) { if (sumsOfChildWidth > widthToFit) { - breakpoint = index + breakpoint = index - 1 break } else { sumsOfChildWidth = sumsOfChildWidth + childWidth.width @@ -158,10 +157,9 @@ export const UnderlineNav = forwardRef( const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 - overflowEffect(navWidth, moreMenuWidth, childArray, childWidthArray, noIconChildWidthArray, callback) }, - [callback, childWidthArray, noIconChildWidthArray, children] + [callback, childWidthArray, noIconChildWidthArray, children, moreMenuRef] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) @@ -194,7 +192,7 @@ export const UnderlineNav = forwardRef( More - + {actions.map((action, index) => { const {children: actionElementChildren, ...actionElementProps} = action.props return ( diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index 0213791f0ef..12ab638badc 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -80,9 +80,11 @@ export const getLinkStyles = ( textDecoration: 'none', paddingX: 1, ...(props?.variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), - '&:hover > div[data-component="wrapper"] ': { - backgroundColor: theme?.colors.neutral.muted, - transition: 'background .12s ease-out' + '@media (hover:hover)': { + '&:hover > div[data-component="wrapper"] ': { + backgroundColor: theme?.colors.neutral.muted, + transition: 'background .12s ease-out' + } }, '&:focus': { outline: 0, From 3b7ae574614f0cbe0e8a154f2c118bf5020b7195 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 19 Sep 2022 09:24:25 +1000 Subject: [PATCH 20/29] remove prop controls and align story --- src/UnderlineNav2/examples.stories.tsx | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index c82954551dc..3485f0b8cab 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -18,22 +18,6 @@ import {BaseStyles, ThemeProvider} from '..' export default { title: 'Layout/UnderlineNav/examples', - argTypes: { - align: { - defaultValue: 'left', - control: { - type: 'radio', - options: ['left', 'right'] - } - }, - variant: { - defaultValue: 'default', - control: { - type: 'radio', - options: ['default', 'small'] - } - } - }, decorators: [ Story => { return ( @@ -88,16 +72,6 @@ export const withCounterLabels = (args: UnderlineNavProps) => { ) } -export const rightAlign = (args: UnderlineNavProps) => { - return ( - - Item 1 - Item 2dsjsjskdjkajsdhkajsdkasj - Item 3 - - ) -} - const items: {navigation: string; icon: React.FC; counter?: number}[] = [ {navigation: 'Code', icon: CodeIcon}, {navigation: 'Issues', icon: IssueOpenedIcon, counter: 120}, From 66885807613ef44b561e7a960b67a3f6dcfc79b7 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 19 Sep 2022 11:41:49 +1000 Subject: [PATCH 21/29] update tests and docs --- docs/content/drafts/UnderlineNav2.mdx | 34 +++--------------- src/UnderlineNav2/UnderlineNav.test.tsx | 48 ++++++++++++++++++++----- src/UnderlineNav2/UnderlineNavItem.tsx | 1 - 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index f95e549ed92..66d6fd56c11 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -23,15 +23,6 @@ import {UnderlineNav} from '@primer/react/drafts' ``` -### Small variant - -```jsx live drafts - - Item 1 - Item 2 - -``` - ### With icons ```jsx live drafts @@ -57,10 +48,10 @@ import {UnderlineNav} from '@primer/react/drafts' ### Overflow Behaviour -#### Items without Icons - When overflow occurs, the component first hides icons if present to optimize for space and show as many items as possible. -If there is still overflow, the component will behave depending on the pointer. Please see below to see these behaviours. +If there is still overflow, the component will hide items that don't fit into screen under the `More` menu + +#### Items without Icons ```jsx live drafts @@ -88,15 +79,7 @@ If there is still overflow, the component will behave depending on the pointer. ``` -#### Display `More` menu for fine pointer devices - -This overflow behaviour will occur on fine pointer devices that have fine pointers. - -#### Scroll for coarse pointer devices - -The scroll behaviour will occur on devices that have coarse pointers. - -Depending on your device when viweing this example, you may see the scroll behaviour or the `More` menu behaviour. +#### Display `More` menu ```jsx live drafts @@ -135,13 +118,6 @@ Depending on your device when viweing this example, you may see the scroll behav - - - + { - test.skip('selected nav', () => { + test('selected nav', () => { const {getByText} = render( Item 1 @@ -17,7 +17,7 @@ describe('UnderlineNav', () => { expect(selectedNavLink?.getAttribute('aria-current')).toBe('page') }) - test.skip('basic nav functionality', () => { + test('basic nav functionality', () => { const {container} = render( Item 1 @@ -30,15 +30,47 @@ describe('UnderlineNav', () => { expect(nav.getAttribute('aria-label')).toBe('Test nav') }) - test.skip('respect align prop', () => { + test('with icons', () => { const {container} = render( + + Code + + Issues + + Pull Request + + ) + const nav = container.getElementsByTagName('nav')[0] + expect(nav.getElementsByTagName('svg').length).toEqual(2) + }) + test('should fire onSelect on click and keypress', async () => { + const onSelect = jest.fn() + const {getByText} = render( + + Item 1 + Item 2 + Item 3 + + ) + const item = getByText('Item 1') + fireEvent.click(item) + expect(onSelect).toHaveBeenCalledTimes(1) + fireEvent.keyPress(item, {key: 'Enter', code: 13, charCode: 13}) + expect(onSelect).toHaveBeenCalledTimes(2) + }) + test('respect counter prop', () => { + const {getByText} = render( - Item 1 + + Item 1 + Item 2 Item 3 ) - const nav = container.getElementsByTagName('nav')[0] - expect(nav).toHaveStyle(`justify-content:flex-end`) + const item = getByText('Item 1').closest('a') + const counter = item?.getElementsByTagName('span')[2] + expect(counter?.className).toContain('CounterLabel') + expect(counter?.textContent).toBe('8') }) }) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index b9bb09e83d7..65df8b14409 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -98,7 +98,6 @@ export const UnderlineNavItem = forwardRef( }, [onSelect, afterSelect, ref, setSelectedLink] ) - const clickHandler = React.useCallback( event => { if (!event.defaultPrevented) { From a8c4bf414975e0942395d5c410a14229db365b73 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 19 Sep 2022 12:05:27 +1000 Subject: [PATCH 22/29] update docs --- docs/content/drafts/UnderlineNav2.mdx | 50 +++++++++++++-------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index 66d6fd56c11..0d6846df184 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -27,20 +27,20 @@ import {UnderlineNav} from '@primer/react/drafts' ```jsx live drafts - + Code - + Issues - + Pull requests - Discussions - + Discussions + Actions - + Projects @@ -55,25 +55,25 @@ If there is still overflow, the component will hide items that don't fit into sc ```jsx live drafts - + Code - + Issues - + Pull requests - Discussions - + Discussions + Actions - + Projects - Security - Insights - + Security + Insights + Settings @@ -83,30 +83,30 @@ If there is still overflow, the component will hide items that don't fit into sc ```jsx live drafts - + Code - + Issues - + Pull requests - Discussions - + Discussions + Actions - + Projects - Security - + Security + Insights - + Settings - Wiki + Wiki ``` From a4503085039d669a9127cda31d3d51b55afb0a01 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 19 Sep 2022 13:19:30 +1000 Subject: [PATCH 23/29] overflow effect improvments --- docs/content/drafts/UnderlineNav2.mdx | 2 +- src/UnderlineNav2/UnderlineNav.tsx | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index 0d6846df184..f24f49f6c54 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -71,7 +71,7 @@ If there is still overflow, the component will hide items that don't fit into sc Projects - Security + Security Insights Settings diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 152ce3c2532..4bd9431618f 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -23,7 +23,9 @@ export type UnderlineNavProps = { afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void children: React.ReactNode } - +// When page is loaded, we don't have ref for the more button as it is not on the DOM yet. +// However, we need to calculate number of possible items when the more button present as well. So using the width of the more button as a constant. +const MORE_BTN_WIDTH = 86 // Needed this because passing a ref using HTMLULListElement to `Box` causes a type error const NavigationList = styled.ul` ${sx}; @@ -44,7 +46,13 @@ const overflowEffect = ( } const numberOfItemsPossible = calculatePossibleItems(childWidthArray, navWidth) - const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, navWidth, moreMenuWidth) + const numberOfItemsWithoutIconPossible = calculatePossibleItems(noIconChildWidthArray, navWidth) + // We need take more menu width into account when calculating the number of items possible + const numberOfItemsPossibleWithMoreMenu = calculatePossibleItems( + noIconChildWidthArray, + navWidth, + moreMenuWidth || MORE_BTN_WIDTH + ) const items: Array = [] const actions: Array = [] @@ -62,7 +70,7 @@ const overflowEffect = ( overflowStyles = moreMenuStyles // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu for (const [index, child] of childArray.entries()) { - if (index < numberOfItemsWithoutIconPossible) { + if (index < numberOfItemsPossibleWithMoreMenu) { items.push(child) } else { actions.push(child) From a7624af70ccf69d5ee6adf041019fa825ceb78c8 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 21 Sep 2022 08:50:15 +1000 Subject: [PATCH 24/29] Revert "remove scroll behaviour - due to accessibility concerns" This reverts commit eade73d521565c137395cb0a790d4721ca2274cc. --- src/UnderlineNav2/UnderlineNav.tsx | 121 ++++++++++++++++-- src/UnderlineNav2/UnderlineNavArrowButton.tsx | 44 +++++++ src/UnderlineNav2/styles.ts | 81 ++++++++++++ src/UnderlineNav2/types.ts | 2 + 4 files changed, 234 insertions(+), 14 deletions(-) create mode 100644 src/UnderlineNav2/UnderlineNavArrowButton.tsx diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 4bd9431618f..4b0376de395 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -1,4 +1,13 @@ -import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react' +import React, { + useRef, + useLayoutEffect, + forwardRef, + useCallback, + useState, + MutableRefObject, + RefObject, + useEffect +} from 'react' import Box from '../Box' import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' @@ -6,14 +15,17 @@ import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import {useFocusZone} from '../hooks/useFocusZone' -import {FocusKeys} from '@primer/behaviors' +import {FocusKeys, scrollIntoView} from '@primer/behaviors' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' -import {ChildWidthArray, ResponsiveProps} from './types' +import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' -import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, moreMenuStyles} from './styles' +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, scrollStyles, moreMenuStyles} from './styles' +import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' import styled from 'styled-components' +import type {ScrollIntoViewOptions} from '@primer/behaviors' + export type UnderlineNavProps = { label: string as?: React.ElementType @@ -26,16 +38,33 @@ export type UnderlineNavProps = { // When page is loaded, we don't have ref for the more button as it is not on the DOM yet. // However, we need to calculate number of possible items when the more button present as well. So using the width of the more button as a constant. const MORE_BTN_WIDTH = 86 +const ARROW_BTN_WIDTH = 36 + +const underlineNavScrollMargins: ScrollIntoViewOptions = { + startMargin: ARROW_BTN_WIDTH, + endMargin: ARROW_BTN_WIDTH, + direction: 'horizontal', + behavior: 'smooth' +} + // Needed this because passing a ref using HTMLULListElement to `Box` causes a type error const NavigationList = styled.ul` ${sx}; ` + +const handleArrowBtnsVisibility = ( + scrollOffsets: {scrollLeft: number; scrollRight: number}, + callback: (scroll: {scrollLeft: number; scrollRight: number}) => void +) => { + callback(scrollOffsets) +} const overflowEffect = ( navWidth: number, moreMenuWidth: number, childArray: Array, childWidthArray: ChildWidthArray, noIconChildWidthArray: ChildWidthArray, + isCoarsePointer: boolean, callback: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true @@ -66,14 +95,20 @@ const overflowEffect = ( } else { iconsVisible = false - // More menu behaviour - overflowStyles = moreMenuStyles - // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu - for (const [index, child] of childArray.entries()) { - if (index < numberOfItemsPossibleWithMoreMenu) { - items.push(child) - } else { - actions.push(child) + if (isCoarsePointer) { + // Scroll behaviour for coarse pointer devices + items.push(...childArray) + overflowStyles = scrollStyles + } else { + // More menu behaviour for fine pointer devices + overflowStyles = moreMenuStyles + // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + for (const [index, child] of childArray.entries()) { + if (index < numberOfItemsPossibleWithMoreMenu) { + items.push(child) + } else { + actions.push(child) + } } } } @@ -85,6 +120,12 @@ function getValidChildren(children: React.ReactNode) { return React.Children.toArray(children).filter(child => React.isValidElement(child)) as React.ReactElement[] } +function calculateScrollOffset(scrollableList: RefObject) { + const {scrollLeft, scrollWidth, clientWidth} = scrollableList.current as HTMLElement + const scrollRight = scrollWidth - scrollLeft - clientWidth + return {scrollLeft, scrollRight} +} + function calculatePossibleItems(childWidthArray: ChildWidthArray, navWidth: number, moreMenuWidth = 0) { const widthToFit = navWidth - moreMenuWidth let breakpoint = childWidthArray.length - 1 @@ -113,9 +154,16 @@ export const UnderlineNav = forwardRef( const {theme} = useTheme() + const [isCoarsePointer, setIsCoarsePointer] = useState(false) + const [selectedLink, setSelectedLink] = useState | undefined>(undefined) const [iconsVisible, setIconsVisible] = useState(true) + + const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ + scrollLeft: 0, + scrollRight: 0 + }) // This might change if we decide tab through the navigation items rather than navigationg with the arrow keys. // TBD. In the meantime keeping it as a menu with the focus trap. // ref: https://www.w3.org/WAI/ARIA/apg/example-index/menubar/menubar-navigation.html (Keyboard Support) @@ -142,6 +190,10 @@ export const UnderlineNav = forwardRef( setIconsVisible(displayIcons) }, []) + const updateOffsetValues = useCallback((scrollOffsets: {scrollLeft: number; scrollRight: number}) => { + setScrollValues(scrollOffsets) + }, []) + const actions = responsiveProps.actions const [childWidthArray, setChildWidthArray] = useState([]) const setChildrenWidth = useCallback(size => { @@ -165,12 +217,49 @@ export const UnderlineNav = forwardRef( const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 - overflowEffect(navWidth, moreMenuWidth, childArray, childWidthArray, noIconChildWidthArray, callback) + const scrollOffsets = calculateScrollOffset(listRef) + + overflowEffect( + navWidth, + moreMenuWidth, + childArray, + childWidthArray, + noIconChildWidthArray, + isCoarsePointer, + callback + ) + + handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) }, - [callback, childWidthArray, noIconChildWidthArray, children, moreMenuRef] + [callback, updateOffsetValues, childWidthArray, noIconChildWidthArray, children, isCoarsePointer, moreMenuRef] ) useResizeObserver(resizeObserverCallback, newRef as RefObject) + // Determine the pointer type on mount + useLayoutEffect(() => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + setIsCoarsePointer(window.matchMedia && window.matchMedia('(pointer:coarse)').matches) + // eslint-disable-next-line github/prefer-observers + listRef.current?.addEventListener('scroll', () => { + const scrollOffsets = calculateScrollOffset(listRef) + + handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) + }) + }, [updateOffsetValues]) + + useEffect(() => { + // scroll the selected link into the view + if (selectedLink && selectedLink.current && listRef.current) { + scrollIntoView(selectedLink.current, listRef.current, underlineNavScrollMargins) + } + }, [selectedLink]) + + const onScrollWithButton: OnScrollWithButtonEventType = (event, direction) => { + if (!listRef.current) return + const ScrollAmount = direction * 200 + listRef.current.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) + } + return ( + 0} onScrollWithButton={onScrollWithButton} /> + (responsiveProps.overflowStyles, ulStyles)} ref={listRef}> {responsiveProps.items} + 0} onScrollWithButton={onScrollWithButton} /> + {actions.length > 0 && ( diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx new file mode 100644 index 00000000000..b36192aefbe --- /dev/null +++ b/src/UnderlineNav2/UnderlineNavArrowButton.tsx @@ -0,0 +1,44 @@ +import React 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' + +const LeftArrowButton = ({ + show, + onScrollWithButton +}: { + show: boolean + onScrollWithButton: OnScrollWithButtonEventType +}) => { + const {theme} = useTheme() + return ( + ) => onScrollWithButton(e, -1)} + icon={ChevronLeftIcon} + sx={show ? getLeftArrowVisibleBtn(theme) : getLeftArrowHiddenBtn(theme)} + /> + ) +} + +const RightArrowButton = ({ + show, + onScrollWithButton +}: { + show: boolean + onScrollWithButton: OnScrollWithButtonEventType +}) => { + const {theme} = useTheme() + return ( + ) => onScrollWithButton(e, 1)} + icon={ChevronRightIcon} + sx={show ? getRightArrowVisibleBtn(theme) : getRightArrowHiddenBtn(theme)} + /> + ) +} + +export {LeftArrowButton, RightArrowButton} diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index 12ab638badc..6e8a40716c9 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -67,6 +67,74 @@ export const moreBtnStyles = { paddingX: 2 } +export const getArrowBtnStyles = (theme?: Theme) => ({ + fontWeight: 'normal', + boxShadow: 'none', + margin: 0, + border: 0, + borderRadius: 0, + paddingX: 0, + paddingY: 0, + background: theme?.colors.canvas.default, + position: 'absolute', + opacity: 1, + transition: 'opacity 250ms ease-out', + zIndex: 1, + '&:hover:not([disabled]), &:focus-visible': { + background: theme?.colors.canvas.default + } +}) + +export const getLeftArrowHiddenBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), + opacity: 0, + top: 0, + bottom: 0, + left: 0 +}) + +export const getRightArrowHiddenBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), + opacity: 0, + top: 0, + bottom: 0, + right: 0 +}) + +export const getLeftArrowVisibleBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), + top: 0, + bottom: 0, + left: 0, + '&::after': { + content: '""', + position: 'absolute', + top: 0, + background: `linear-gradient(to left,#fff0,${theme?.colors.canvas.default})`, + height: '100%', + width: '20px', + right: '-20px', + pointerEvents: 'none' + } +}) + +export const getRightArrowVisibleBtn = (theme?: Theme) => ({ + ...getArrowBtnStyles(theme), + top: 0, + bottom: 0, + right: 0, + '&::before': { + position: 'absolute', + top: 0, + background: `linear-gradient(to right,#fff0,${theme?.colors.canvas.default})`, + content: '""', + height: '100%', + left: '-20px', + width: '20px', + pointerEvents: 'none' + } +}) + export const getLinkStyles = ( theme?: Theme, props?: Partial>, @@ -122,4 +190,17 @@ export const getLinkStyles = ( } }) +export const scrollStyles: BetterSystemStyleObject = { + whiteSpace: 'nowrap', + overflowX: 'auto', + // Hiding scrollbar on Firefox + scrollbarWidth: 'none', + // Hiding scrollbar on IE 10+ + msOverflowStyle: 'none', + // Hiding scrollbar on Chrome, Safari and Opera + '&::-webkit-scrollbar': { + display: 'none' + } +} + export const moreMenuStyles: BetterSystemStyleObject = {whiteSpace: 'nowrap'} diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index 881825e3f54..635213764e3 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -5,3 +5,5 @@ export type ResponsiveProps = { actions: Array overflowStyles: BetterSystemStyleObject } + +export type OnScrollWithButtonEventType = (event: React.MouseEvent, direction: -1 | 1) => void From c9652efec0c9156992a91abf9f5924ef989d12c4 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 21 Sep 2022 13:54:50 +1000 Subject: [PATCH 25/29] scroll behaviour feedback --- docs/content/drafts/UnderlineNav2.mdx | 6 +- src/UnderlineNav2/UnderlineNav.tsx | 91 ++++++++++++--------------- src/UnderlineNav2/styles.ts | 6 +- 3 files changed, 47 insertions(+), 56 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index f24f49f6c54..56403990365 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -48,8 +48,7 @@ import {UnderlineNav} from '@primer/react/drafts' ### Overflow Behaviour -When overflow occurs, the component first hides icons if present to optimize for space and show as many items as possible. -If there is still overflow, the component will hide items that don't fit into screen under the `More` menu +When overflow occurs, the component first hides icons if present to optimize for space and show as many items as possible. (Only for fine pointer devices) #### Items without Icons @@ -81,7 +80,10 @@ If there is still overflow, the component will hide items that don't fit into sc #### Display `More` menu +If there is still overflow, the component will behave depending on the pointer. + ```jsx live drafts +// TODO: Use Match Media for coarse pointer example Code diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 4b0376de395..a8fa12b4f89 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -1,13 +1,4 @@ -import React, { - useRef, - useLayoutEffect, - forwardRef, - useCallback, - useState, - MutableRefObject, - RefObject, - useEffect -} from 'react' +import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject, useEffect} from 'react' import Box from '../Box' import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx' import {UnderlineNavContext} from './UnderlineNavContext' @@ -15,6 +6,7 @@ import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' import {useFocusZone} from '../hooks/useFocusZone' +import {useMedia} from '../hooks/useMedia' import {FocusKeys, scrollIntoView} from '@primer/behaviors' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' @@ -53,9 +45,12 @@ const NavigationList = styled.ul` ` const handleArrowBtnsVisibility = ( - scrollOffsets: {scrollLeft: number; scrollRight: number}, + scrollableList: RefObject, callback: (scroll: {scrollLeft: number; scrollRight: number}) => void ) => { + const {scrollLeft, scrollWidth, clientWidth} = scrollableList.current as HTMLElement + const scrollRight = scrollWidth - scrollLeft - clientWidth + const scrollOffsets = {scrollLeft, scrollRight} callback(scrollOffsets) } const overflowEffect = ( @@ -85,24 +80,26 @@ const overflowEffect = ( const items: Array = [] const actions: Array = [] - // First we check if we can fit all the items with 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 (isCoarsePointer) { + // If it is a coarse pointer, we never show the icons even if they fit into the screen. iconsVisible = false items.push(...childArray) + // If we have more items than we can fit, we add the scroll styles + if (childArray.length > numberOfItemsWithoutIconPossible) { + overflowStyles = scrollStyles + } } else { - iconsVisible = false - - if (isCoarsePointer) { - // Scroll behaviour for coarse pointer devices + // For fine pointer devices, first we check if we can fit all the items with 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 + iconsVisible = false items.push(...childArray) - overflowStyles = scrollStyles } else { - // More menu behaviour for fine pointer devices - overflowStyles = moreMenuStyles // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + iconsVisible = false + overflowStyles = moreMenuStyles for (const [index, child] of childArray.entries()) { if (index < numberOfItemsPossibleWithMoreMenu) { items.push(child) @@ -116,17 +113,11 @@ const overflowEffect = ( callback({items, actions, overflowStyles}, iconsVisible) } -function getValidChildren(children: React.ReactNode) { +const getValidChildren = (children: React.ReactNode) => { return React.Children.toArray(children).filter(child => React.isValidElement(child)) as React.ReactElement[] } -function calculateScrollOffset(scrollableList: RefObject) { - const {scrollLeft, scrollWidth, clientWidth} = scrollableList.current as HTMLElement - const scrollRight = scrollWidth - scrollLeft - clientWidth - return {scrollLeft, scrollRight} -} - -function calculatePossibleItems(childWidthArray: ChildWidthArray, navWidth: number, moreMenuWidth = 0) { +const calculatePossibleItems = (childWidthArray: ChildWidthArray, navWidth: number, moreMenuWidth = 0) => { const widthToFit = navWidth - moreMenuWidth let breakpoint = childWidthArray.length - 1 let sumsOfChildWidth = 0 @@ -154,7 +145,7 @@ export const UnderlineNav = forwardRef( const {theme} = useTheme() - const [isCoarsePointer, setIsCoarsePointer] = useState(false) + const isCoarsePointer = useMedia('(pointer: coarse)') const [selectedLink, setSelectedLink] = useState | undefined>(undefined) @@ -194,6 +185,16 @@ export const UnderlineNav = forwardRef( setScrollValues(scrollOffsets) }, []) + const scrollOnList = useCallback(() => { + handleArrowBtnsVisibility(listRef, updateOffsetValues) + }, [updateOffsetValues]) + + const onScrollWithButton: OnScrollWithButtonEventType = (event, direction) => { + if (!listRef.current) return + const ScrollAmount = direction * 200 + listRef.current.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) + } + const actions = responsiveProps.actions const [childWidthArray, setChildWidthArray] = useState([]) const setChildrenWidth = useCallback(size => { @@ -217,8 +218,6 @@ export const UnderlineNav = forwardRef( const childArray = getValidChildren(children) const navWidth = resizeObserverEntries[0].contentRect.width const moreMenuWidth = moreMenuRef.current?.getBoundingClientRect().width ?? 0 - const scrollOffsets = calculateScrollOffset(listRef) - overflowEffect( navWidth, moreMenuWidth, @@ -229,23 +228,19 @@ export const UnderlineNav = forwardRef( callback ) - handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) + handleArrowBtnsVisibility(listRef, updateOffsetValues) }, [callback, updateOffsetValues, childWidthArray, noIconChildWidthArray, children, isCoarsePointer, moreMenuRef] ) + useResizeObserver(resizeObserverCallback, newRef as RefObject) - // Determine the pointer type on mount - useLayoutEffect(() => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - setIsCoarsePointer(window.matchMedia && window.matchMedia('(pointer:coarse)').matches) + useEffect(() => { + const listEl = listRef.current // eslint-disable-next-line github/prefer-observers - listRef.current?.addEventListener('scroll', () => { - const scrollOffsets = calculateScrollOffset(listRef) - - handleArrowBtnsVisibility(scrollOffsets, updateOffsetValues) - }) - }, [updateOffsetValues]) + listEl?.addEventListener('scroll', scrollOnList) + return () => listEl?.removeEventListener('scroll', scrollOnList) + }, [scrollOnList]) useEffect(() => { // scroll the selected link into the view @@ -254,12 +249,6 @@ export const UnderlineNav = forwardRef( } }, [selectedLink]) - const onScrollWithButton: OnScrollWithButtonEventType = (event, direction) => { - if (!listRef.current) return - const ScrollAmount = direction * 200 - listRef.current.scrollBy({left: ScrollAmount, top: 0, behavior: 'smooth'}) - } - return ( ({ margin: 0, border: 0, borderRadius: 0, - paddingX: 0, + paddingX: 2, paddingY: 0, background: theme?.colors.canvas.default, position: 'absolute', @@ -113,7 +113,7 @@ export const getLeftArrowVisibleBtn = (theme?: Theme) => ({ background: `linear-gradient(to left,#fff0,${theme?.colors.canvas.default})`, height: '100%', width: '20px', - right: '-20px', + right: '-15px', pointerEvents: 'none' } }) @@ -129,8 +129,8 @@ export const getRightArrowVisibleBtn = (theme?: Theme) => ({ background: `linear-gradient(to right,#fff0,${theme?.colors.canvas.default})`, content: '""', height: '100%', - left: '-20px', width: '20px', + left: '-15px', pointerEvents: 'none' } }) From 4a822ca4e72ed1d4a2c634991b5b4edc924e3ee6 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 21 Sep 2022 14:17:46 +1000 Subject: [PATCH 26/29] update tests --- src/UnderlineNav2/UnderlineNav.test.tsx | 21 +++++++++++++++++++++ src/UnderlineNav2/UnderlineNav.tsx | 11 +++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index 3fa49091a3b..c10eb2d266a 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -4,6 +4,27 @@ import {fireEvent, render} from '@testing-library/react' import {CodeIcon, EyeIcon} from '@primer/octicons-react' import {UnderlineNav} from '.' + +// window.matchMedia() is not implemented by JSDOM so we have to create a mock: +// https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation(query => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), // deprecated + removeListener: jest.fn(), // deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn() + })) +}) + +Object.defineProperty(window.Element.prototype, 'scrollTo', { + value: jest.fn(), + writable: true +}) describe('UnderlineNav', () => { test('selected nav', () => { const {getByText} = render( diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index a8fa12b4f89..226d6522811 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -8,6 +8,7 @@ import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver import {useFocusZone} from '../hooks/useFocusZone' import {useMedia} from '../hooks/useMedia' import {FocusKeys, scrollIntoView} from '@primer/behaviors' +import type {ScrollIntoViewOptions} from '@primer/behaviors' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' import {ChildWidthArray, ResponsiveProps, OnScrollWithButtonEventType} from './types' @@ -16,8 +17,6 @@ import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, scrollStyles, mo import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton' import styled from 'styled-components' -import type {ScrollIntoViewOptions} from '@primer/behaviors' - export type UnderlineNavProps = { label: string as?: React.ElementType @@ -268,13 +267,17 @@ export const UnderlineNav = forwardRef( aria-label={label} ref={newRef} > - 0} onScrollWithButton={onScrollWithButton} /> + {isCoarsePointer && ( + 0} onScrollWithButton={onScrollWithButton} /> + )} (responsiveProps.overflowStyles, ulStyles)} ref={listRef}> {responsiveProps.items} - 0} onScrollWithButton={onScrollWithButton} /> + {isCoarsePointer && ( + 0} onScrollWithButton={onScrollWithButton} /> + )} {actions.length > 0 && ( From e9142f714b452863c3c78f69348e3d66a46a1975 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 23 Sep 2022 08:55:33 +1000 Subject: [PATCH 27/29] keyboard navigation tab through --- src/UnderlineNav2/UnderlineNav.tsx | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 226d6522811..50900cc5fa1 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -5,9 +5,8 @@ import {UnderlineNavContext} from './UnderlineNavContext' import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' -import {useFocusZone} from '../hooks/useFocusZone' import {useMedia} from '../hooks/useMedia' -import {FocusKeys, scrollIntoView} from '@primer/behaviors' +import {scrollIntoView} from '@primer/behaviors' import type {ScrollIntoViewOptions} from '@primer/behaviors' import CounterLabel from '../CounterLabel' import {useTheme} from '../ThemeProvider' @@ -154,14 +153,6 @@ export const UnderlineNav = forwardRef( scrollLeft: 0, scrollRight: 0 }) - // This might change if we decide tab through the navigation items rather than navigationg with the arrow keys. - // TBD. In the meantime keeping it as a menu with the focus trap. - // ref: https://www.w3.org/WAI/ARIA/apg/example-index/menubar/menubar-navigation.html (Keyboard Support) - useFocusZone({ - containerRef: backupRef, - bindKeys: FocusKeys.ArrowHorizontal | FocusKeys.HomeAndEnd, - focusOutBehavior: 'wrap' - }) const afterSelectHandler = (event: React.MouseEvent | React.KeyboardEvent) => { if (!event.defaultPrevented) { @@ -261,7 +252,6 @@ export const UnderlineNav = forwardRef( }} > (getNavStyles(theme, {align}), sxProp)} aria-label={label} From 580ce519e13926b1fe826fcb8e3ea8600b7bbb82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Wed, 28 Sep 2022 12:19:12 +1000 Subject: [PATCH 28/29] Update .changeset/sweet-eggs-complain.md Co-authored-by: Cole Bemis --- .changeset/sweet-eggs-complain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/sweet-eggs-complain.md b/.changeset/sweet-eggs-complain.md index 2e58e48957e..7d922deb655 100644 --- a/.changeset/sweet-eggs-complain.md +++ b/.changeset/sweet-eggs-complain.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -Introducing overflow behaviour for fine and coarse pointer devices +UnderlineNav2: Introducing overflow behavior for fine and coarse pointer devices From 80e52e3a1695c5767d95f4f806ff86a74a264b90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Wed, 28 Sep 2022 13:08:29 +1000 Subject: [PATCH 29/29] Update docs/content/drafts/UnderlineNav2.mdx --- docs/content/drafts/UnderlineNav2.mdx | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index 56403990365..dab8814d770 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -83,7 +83,6 @@ When overflow occurs, the component first hides icons if present to optimize for If there is still overflow, the component will behave depending on the pointer. ```jsx live drafts -// TODO: Use Match Media for coarse pointer example Code