From e03b5e4d3d7ad5f7761d59158e9223da746f4592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Tue, 25 Oct 2022 10:11:19 +1000 Subject: [PATCH] [UnderlineNav2]: Deprecate coarse input detection and its scroll behaviour (A11y sign-off remediations) (#2447) * Discard the pointer types and scroll behaviour * add changeset * remove arrow btn styles * introduce gap between items and remove 4px padding around the links * [UnderlineNav2]: React router implementation fixes & docs improvement (#2448) * react router implementation fixes * add changeset * [UnderlineNav2]: Add string type to the `counter` prop and display loading counter for all (#2449) * string type for counters and fix loading issue * add changeset * improve docs * update animation details * Inverte the pulse effect Co-authored-by: Daniel Guillan Co-authored-by: Daniel Guillan * Remove unnecessary styles (scroll behaviour styles) * improve docs Co-authored-by: Daniel Guillan --- .changeset/polite-dodos-behave.md | 5 + .changeset/tough-peas-sort.md | 5 + .changeset/witty-apples-think.md | 5 + docs/content/drafts/UnderlineNav2.mdx | 150 ++++++------ src/UnderlineNav2/LoadingCounter.tsx | 8 +- src/UnderlineNav2/UnderlineNav.test.tsx | 5 - src/UnderlineNav2/UnderlineNav.tsx | 230 +++++------------- src/UnderlineNav2/UnderlineNavArrowButton.tsx | 66 ----- src/UnderlineNav2/UnderlineNavContext.tsx | 4 +- src/UnderlineNav2/UnderlineNavItem.tsx | 22 +- src/UnderlineNav2/examples.stories.tsx | 6 +- src/UnderlineNav2/styles.ts | 88 ++----- src/UnderlineNav2/types.ts | 4 - 13 files changed, 189 insertions(+), 409 deletions(-) create mode 100644 .changeset/polite-dodos-behave.md create mode 100644 .changeset/tough-peas-sort.md create mode 100644 .changeset/witty-apples-think.md delete mode 100644 src/UnderlineNav2/UnderlineNavArrowButton.tsx diff --git a/.changeset/polite-dodos-behave.md b/.changeset/polite-dodos-behave.md new file mode 100644 index 00000000000..aa525472f62 --- /dev/null +++ b/.changeset/polite-dodos-behave.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +UnderlineNav2: Add support and docs for react router configuration diff --git a/.changeset/tough-peas-sort.md b/.changeset/tough-peas-sort.md new file mode 100644 index 00000000000..bf03ca68455 --- /dev/null +++ b/.changeset/tough-peas-sort.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +UnderlineNav2: Add string type to the `counter` prop and display loading counter for all diff --git a/.changeset/witty-apples-think.md b/.changeset/witty-apples-think.md new file mode 100644 index 00000000000..09aead3a864 --- /dev/null +++ b/.changeset/witty-apples-think.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +UnderlineNav2: Deprecate coarse input detection and its scroll behaviour diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index 25298112592..8bfbf710140 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -17,9 +17,9 @@ import {UnderlineNav} from '@primer/react/drafts' ```jsx live drafts - Item 1 - Item 2 - Item 3 + Code + Issues + Pull Requests ``` @@ -48,79 +48,78 @@ 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. (Only for fine pointer devices) - -#### Items Without Icons - -```jsx live drafts - - - Code - - - Issues - - - Pull Requests - - Discussions - - Actions - - - Projects - - Security - Insights - - Settings - - +Component first hides icons if they present to optimize for space and show as many items as possible. If there is still an overflow, it will display the items that don't fit in the `More` menu. + +```javascript noinline live drafts +const Navigation = () => { + const items = [ + {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} + ] + const [selectedIndex, setSelectedIndex] = React.useState(0) + return ( + + + {items.map((item, index) => ( + { + setSelectedIndex(index) + e.preventDefault() + }} + counter={item.counter} + > + {item.navigation} + + ))} + + + ) +} +render() ``` -#### Display `More` Menu - -If there is still overflow, the component will behave depending on the pointer. +### Loading State For Counters ```jsx live drafts - - + + Code - - Issues - - - Pull Requests - - Discussions - - Actions - - - Projects - - Security - - Insights - - - Settings - - Wiki + Issues + Pull Requests ``` -### Loading state for counters +### With React Router -```jsx live drafts - - - Item 1 - - Item 2 - Item 3 - +```jsx +import {Link} from 'react-router-dom' +import {UnderlineNav} from '@primer/react/drafts' +const Navigation = () => { + return ( + + + Code + + + Issues + + + Pull Requests + + + ) +} ``` ## Props @@ -128,16 +127,20 @@ If there is still overflow, the component will behave depending on the pointer. ### UnderlineNav + + diff --git a/src/UnderlineNav2/LoadingCounter.tsx b/src/UnderlineNav2/LoadingCounter.tsx index 9d25c2911d6..7e7d67b4366 100644 --- a/src/UnderlineNav2/LoadingCounter.tsx +++ b/src/UnderlineNav2/LoadingCounter.tsx @@ -2,13 +2,13 @@ import styled, {keyframes} from 'styled-components' import {get} from '../constants' const loading = keyframes` - from { opacity: 0.4; } - to { opacity: 0.8; } + from { opacity: 1; } + to { opacity: 0.2; } ` export const LoadingCounter = styled.span` - animation: ${loading} 1.2s linear infinite alternate; - background-color: ${get('colors.neutral.emphasis')}; + animation: ${loading} 1.2s ease-in-out infinite alternate; + background-color: ${get('colors.neutral.muted')}; border-color: ${get('colors.border.default')}; width: 1.5rem; height: 1rem; // 16px diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index b3345bdc47d..9231b4b9b90 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -30,11 +30,6 @@ Object.defineProperty(window, 'matchMedia', { })) }) -Object.defineProperty(window.Element.prototype, 'scrollTo', { - value: jest.fn(), - writable: true -}) - const ResponsiveUnderlineNav = ({ selectedItemText = 'Code', loadingCounters = false diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 5b60b502900..cf256f9409a 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -1,27 +1,15 @@ -import React, {useRef, 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' import {ActionMenu} from '../ActionMenu' import {ActionList} from '../ActionList' import {useResizeObserver, ResizeObserverEntry} from '../hooks/useResizeObserver' -import {useMedia} from '../hooks/useMedia' -import {scrollIntoView} from '@primer/behaviors' -import type {ScrollIntoViewOptions} 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, - menuItemStyles -} from './styles' -import {ArrowButton} from './UnderlineNavArrowButton' +import {moreBtnStyles, getDividerStyle, getNavStyles, ulStyles, menuItemStyles, GAP} from './styles' import styled from 'styled-components' import {LoadingCounter} from './LoadingCounter' @@ -41,14 +29,6 @@ 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` @@ -59,29 +39,17 @@ const MoreMenuListItem = styled.li` display: flex; ` -const handleArrowBtnsVisibility = ( - 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 = ( navWidth: number, moreMenuWidth: number, childArray: Array, childWidthArray: ChildWidthArray, noIconChildWidthArray: ChildWidthArray, - isCoarsePointer: boolean, updateListAndMenu: (props: ResponsiveProps, iconsVisible: boolean) => void ) => { let iconsVisible = true - let overflowStyles: BetterSystemStyleObject | null = {} if (childWidthArray.length === 0) { - updateListAndMenu({items: childArray, actions: [], overflowStyles}, iconsVisible) + updateListAndMenu({items: childArray, actions: []}, iconsVisible) } const numberOfItemsPossible = calculatePossibleItems(childWidthArray, navWidth) @@ -95,42 +63,31 @@ const overflowEffect = ( const items: Array = [] const actions: Array = [] - if (isCoarsePointer) { - // If it is a coarse pointer, we never show the icons even if they fit into the screen. + // 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) - // If we have more items than we can fit, we add the scroll styles - if (childArray.length > numberOfItemsWithoutIconPossible) { - overflowStyles = scrollStyles - } } else { - // 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) - } else { - // 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) - // keeping selected item always visible. - } else if (child.props.selected) { - // If selected item's index couldn't make the list, we swap it with the last item in the list. - const propsectiveAction = items.splice(numberOfItemsPossibleWithMoreMenu - 1, 1, child)[0] - actions.push(propsectiveAction) - } else { - actions.push(child) - } + // if we can't fit all the items without icons, we keep the icons hidden and show the rest in the menu + iconsVisible = false + for (const [index, child] of childArray.entries()) { + if (index < numberOfItemsPossibleWithMoreMenu) { + items.push(child) + // keeping selected item always visible. + } else if (child.props.selected) { + // If selected item's index couldn't make the list, we swap it with the last item in the list. + const propsectiveAction = items.splice(numberOfItemsPossibleWithMoreMenu - 1, 1, child)[0] + actions.push(propsectiveAction) + } else { + actions.push(child) } } } - updateListAndMenu({items, actions, overflowStyles}, iconsVisible) + updateListAndMenu({items, actions}, iconsVisible) } const getValidChildren = (children: React.ReactNode) => { @@ -146,7 +103,8 @@ const calculatePossibleItems = (childWidthArray: ChildWidthArray, navWidth: numb breakpoint = index - 1 break } else { - sumsOfChildWidth = sumsOfChildWidth + childWidth.width + // The the gap between items into account when calculating the number of items possible + sumsOfChildWidth = sumsOfChildWidth + childWidth.width + GAP } } @@ -203,10 +161,7 @@ export const UnderlineNav = forwardRef( // Add itemsToAddToMenu array's items to the menu at the index of the prospectiveListItem and remove 1 count of items (prospectiveListItem) updatedMenuItems.splice(indexOfProspectiveListItem, 1, ...itemsToAddToMenu) setSelectedLinkText(prospectiveListItem.props.children) - callback( - {items: updatedItemList, actions: updatedMenuItems, overflowStyles: responsiveProps.overflowStyles}, - false - ) + callback({items: updatedItemList, actions: updatedMenuItems}, false) } // How many items do we need to pull in to the menu to make room for the selected menu item. function getBreakpointForItemSwapping(widthToFitIntoList: number, availableSpace: number) { @@ -222,12 +177,8 @@ export const UnderlineNav = forwardRef( return breakpoint } - const isCoarsePointer = useMedia('(pointer: coarse)') - const [selectedLink, setSelectedLink] = useState | undefined>(undefined) - const [focusedLink, setFocusedLink] = useState | null>(null) - // selectedLinkText is needed to be able set the selected menu item as selectedLink. // This is needed because setSelectedLink only accepts ref but at the time of setting selected menu item as selectedLink, its ref as a list item is not available const [selectedLinkText, setSelectedLinkText] = useState('') @@ -236,12 +187,9 @@ export const UnderlineNav = forwardRef( React.MouseEvent | React.KeyboardEvent | null >(null) - const [iconsVisible, setIconsVisible] = useState(true) + const [asNavItem, setAsNavItem] = useState('a') - const [scrollValues, setScrollValues] = useState<{scrollLeft: number; scrollRight: number}>({ - scrollLeft: 0, - scrollRight: 0 - }) + const [iconsVisible, setIconsVisible] = useState(true) const afterSelectHandler = (event: React.MouseEvent | React.KeyboardEvent) => { if (!event.defaultPrevented) { @@ -251,8 +199,7 @@ export const UnderlineNav = forwardRef( const [responsiveProps, setResponsiveProps] = useState({ items: getValidChildren(children), - actions: [], - overflowStyles: {} + actions: [] }) const updateListAndMenu = useCallback((props: ResponsiveProps, displayIcons: boolean) => { @@ -260,20 +207,6 @@ export const UnderlineNav = forwardRef( setIconsVisible(displayIcons) }, []) - const updateOffsetValues = useCallback((scrollOffsets: {scrollLeft: number; scrollRight: number}) => { - 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 => { @@ -295,42 +228,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, - isCoarsePointer, - updateListAndMenu - ) - handleArrowBtnsVisibility(listRef, updateOffsetValues) + overflowEffect(navWidth, moreMenuWidth, childArray, childWidthArray, noIconChildWidthArray, updateListAndMenu) }, navRef as RefObject) - useEffect(() => { - const listEl = listRef.current - // eslint-disable-next-line github/prefer-observers - listEl?.addEventListener('scroll', scrollOnList) - return () => listEl?.removeEventListener('scroll', scrollOnList) - }, [scrollOnList]) - - function scrollLinkIntoView(link: RefObject) { - if (link.current && listRef.current) { - scrollIntoView(link.current, listRef.current, underlineNavScrollMargins) - return - } - } - - useEffect(() => { - // scroll the selected link into the view (coarse pointer behaviour) - selectedLink?.current && isCoarsePointer && scrollLinkIntoView(selectedLink) - }, [selectedLink, isCoarsePointer]) - - useEffect(() => { - // scroll the focused link into the view (coarse pointer behaviour) - focusedLink?.current && isCoarsePointer && scrollLinkIntoView(focusedLink) - }, [focusedLink, isCoarsePointer]) - if (!ariaLabel) { // eslint-disable-next-line no-console console.warn('Use the `aria-label` prop to provide an accessible label for assistive technology') @@ -346,7 +246,7 @@ export const UnderlineNav = forwardRef( setSelectedLink, selectedLinkText, setSelectedLinkText, - setFocusedLink, + setAsNavItem, selectEvent, afterSelect: afterSelectHandler, variant, @@ -360,17 +260,7 @@ export const UnderlineNav = forwardRef( aria-label={ariaLabel} ref={navRef} > - {isCoarsePointer && ( - 0} - onScrollWithButton={onScrollWithButton} - aria-label={ariaLabel} - /> - )} - - (responsiveProps.overflowStyles, ulStyles)} ref={listRef}> + {responsiveProps.items} {actions.length > 0 && ( @@ -382,28 +272,34 @@ export const UnderlineNav = forwardRef( {actions.map((action, index) => { const {children: actionElementChildren, ...actionElementProps} = action.props return ( - | React.KeyboardEvent) => { - swapMenuItemWithListItem(action, index, event, updateListAndMenu) - setSelectEvent(event) - }} - > - + | React.KeyboardEvent + ) => { + swapMenuItemWithListItem(action, index, event, updateListAndMenu) + setSelectEvent(event) + }} > - {actionElementChildren} - - {loadingCounters ? ( - - ) : ( - {actionElementProps.counter} - )} - - + + {actionElementChildren} + + {loadingCounters ? ( + + ) : ( + actionElementProps.counter !== undefined && ( + {actionElementProps.counter} + ) + )} + + + ) })} @@ -412,16 +308,6 @@ export const UnderlineNav = forwardRef( )} - - {isCoarsePointer && ( - 0} - onScrollWithButton={onScrollWithButton} - aria-label={ariaLabel} - /> - )} ) diff --git a/src/UnderlineNav2/UnderlineNavArrowButton.tsx b/src/UnderlineNav2/UnderlineNavArrowButton.tsx deleted file mode 100644 index 81d5822d592..00000000000 --- a/src/UnderlineNav2/UnderlineNavArrowButton.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import React, {useContext} from 'react' -import {IconButton} from '../Button/IconButton' -import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' -import {btnWrapperStyles, getArrowBtnStyles} from './styles' -import {OnScrollWithButtonEventType} from './types' -import {UnderlineNavContext} from './UnderlineNavContext' -import Box from '../Box' - -const ArrowButton = ({ - scrollValue, - type, - show, - onScrollWithButton, - 'aria-label': ariaLabel -}: { - scrollValue: number - type: 'left' | 'right' - show: boolean - onScrollWithButton: OnScrollWithButtonEventType - 'aria-label'?: React.AriaAttributes['aria-label'] -}) => { - const leftBtnRef = React.useRef(null) - const rightBtnRef = React.useRef(null) - const {theme} = useContext(UnderlineNavContext) - const direction = type === 'left' ? -1 : 1 - const ARROW_BTN_WIDTH = 44 // Min touch target size is 44px - - // re-trigger focus on the button with aria-disabled=true when it becomes hidden to communicate to screen readers that the button is no longer available - React.useEffect(() => { - const currentBtn = type === 'left' ? leftBtnRef.current : rightBtnRef.current - if (currentBtn?.getAttribute('aria-disabled') === 'true') { - currentBtn.focus() - } else { - // eslint-disable-next-line github/no-blur - currentBtn?.blur() - } - }, [show, type]) - - let translateX = 0 - let display = 'flex' - - // Determine the translateX value to transform for the slide in/out effect - if (scrollValue === 0) { - // If the scrollValue is 0, the buttons should be hidden - translateX = ARROW_BTN_WIDTH * direction - // This is mainly needed for the right arrow button. Because hiding translateX value for it is positive (44px) and this positive value was causing button to be visibly overflowed rathan than hiding. - display = 'none' - } else if (scrollValue <= ARROW_BTN_WIDTH) translateX = (ARROW_BTN_WIDTH - scrollValue) * direction - else translateX = 0 - - return ( - - ) => onScrollWithButton(e, direction)} - icon={type === 'left' ? ChevronLeftIcon : ChevronRightIcon} - sx={getArrowBtnStyles(theme, type)} - aria-disabled={!show} - /> - - ) -} - -export {ArrowButton} diff --git a/src/UnderlineNav2/UnderlineNavContext.tsx b/src/UnderlineNav2/UnderlineNavContext.tsx index 285810ba36f..08521017f7a 100644 --- a/src/UnderlineNav2/UnderlineNavContext.tsx +++ b/src/UnderlineNav2/UnderlineNavContext.tsx @@ -9,8 +9,8 @@ export const UnderlineNavContext = createContext<{ setSelectedLink: (ref: RefObject) => void selectedLinkText: string setSelectedLinkText: React.Dispatch> - setFocusedLink: React.Dispatch | null>> selectEvent: React.MouseEvent | React.KeyboardEvent | null + setAsNavItem: React.Dispatch> afterSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void variant: 'default' | 'small' loadingCounters: boolean @@ -23,8 +23,8 @@ export const UnderlineNavContext = createContext<{ setSelectedLink: () => null, selectedLinkText: '', setSelectedLinkText: () => null, - setFocusedLink: () => null, selectEvent: null, + setAsNavItem: () => null, variant: 'default', loadingCounters: false, iconsVisible: true diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 4aa9624cfd5..288a088e04f 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -42,7 +42,7 @@ export type UnderlineNavItemProps = { /** * Counter */ - counter?: number + counter?: number | string } & SxProp & LinkProps @@ -71,8 +71,8 @@ export const UnderlineNavItem = forwardRef( setSelectedLink, selectedLinkText, setSelectedLinkText, - setFocusedLink, selectEvent, + setAsNavItem, afterSelect, variant, loadingCounters, @@ -107,6 +107,7 @@ export const UnderlineNavItem = forwardRef( if (typeof onSelect === 'function' && selectEvent !== null) onSelect(selectEvent) setSelectedLinkText('') } + setAsNavItem(Component) }, [ ref, preSelected, @@ -117,7 +118,9 @@ export const UnderlineNavItem = forwardRef( setChildrenWidth, setNoIconChildrenWidth, onSelect, - selectEvent + selectEvent, + setAsNavItem, + Component ]) const keyPressHandler = React.useCallback( @@ -127,7 +130,6 @@ export const UnderlineNavItem = forwardRef( if (typeof afterSelect === 'function') afterSelect(event) } setSelectedLink(ref as RefObject) - event.preventDefault() }, [onSelect, afterSelect, ref, setSelectedLink] ) @@ -138,7 +140,6 @@ export const UnderlineNavItem = forwardRef( if (typeof afterSelect === 'function') afterSelect(event) } setSelectedLink(ref as RefObject) - event.preventDefault() }, [onSelect, afterSelect, ref, setSelectedLink] ) @@ -154,7 +155,6 @@ export const UnderlineNavItem = forwardRef( sx={merge(getLinkStyles(theme, {variant}, selectedLink, ref), sxProp as SxProp)} {...props} ref={ref} - onFocus={() => setFocusedLink(ref as RefObject)} > {iconsVisible && Icon && ( @@ -172,10 +172,16 @@ export const UnderlineNavItem = forwardRef( {children} )} - {counter && ( + {loadingCounters ? ( - {loadingCounters ? : {counter}} + + ) : ( + counter !== undefined && ( + + {counter} + + ) )} diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 04daa680922..8f224053ff7 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -72,14 +72,14 @@ export const withCounterLabels = () => { ) } -const items: {navigation: string; icon: React.FC; counter?: number}[] = [ +const items: {navigation: string; icon: React.FC; counter?: number | string}[] = [ {navigation: 'Code', icon: CodeIcon}, - {navigation: 'Issues', icon: IssueOpenedIcon, counter: 120}, + {navigation: 'Issues', icon: IssueOpenedIcon, counter: '12K'}, {navigation: 'Pull Requests', icon: GitPullRequestIcon, counter: 13}, {navigation: 'Discussions', icon: CommentDiscussionIcon, counter: 5}, {navigation: 'Actions', icon: PlayIcon, counter: 4}, {navigation: 'Projects', icon: ProjectIcon, counter: 9}, - {navigation: 'Insights', icon: GraphIcon}, + {navigation: 'Insights', icon: GraphIcon, counter: '0'}, {navigation: 'Settings', icon: GearIcon, counter: 10}, {navigation: 'Security', icon: ShieldLockIcon} ] diff --git a/src/UnderlineNav2/styles.ts b/src/UnderlineNav2/styles.ts index 6f3510a24f0..5b82b8190ce 100644 --- a/src/UnderlineNav2/styles.ts +++ b/src/UnderlineNav2/styles.ts @@ -1,7 +1,9 @@ import {Theme} from '../ThemeProvider' -import {BetterSystemStyleObject} from '../sx' import {UnderlineNavProps} from './UnderlineNav' +// The gap between the list items. It is a constant because the gap is used to calculate the possible number of items that can fit in the container. +export const GAP = 8 + export const iconWrapStyles = { alignItems: 'center', display: 'inline-flex', @@ -32,7 +34,7 @@ export const counterStyles = { export const getNavStyles = (theme?: Theme, props?: Partial>) => ({ display: 'flex', - paddingX: 2, + paddingX: 3, justifyContent: props?.align === 'right' ? 'flex-end' : 'flex-start', borderBottom: '1px solid', borderBottomColor: `${theme?.colors.border.muted}`, @@ -44,10 +46,13 @@ export const getNavStyles = (theme?: Theme, props?: Partial ({ @@ -69,57 +74,6 @@ export const moreBtnStyles = { paddingX: 2 } -export const btnWrapperStyles = ( - theme?: Theme, - direction = 'left', - show = false, - translateX = 0, - display = 'flex' -) => ({ - position: 'absolute', - zIndex: 1, - top: 0, - bottom: 0, - left: direction === 'left' ? 0 : 'auto', - right: direction === 'right' ? 0 : 'auto', - alignItems: 'center', - background: show - ? `linear-gradient(to ${direction} ,#fff0, ${theme?.colors.canvas.default} 14px, ${theme?.colors.canvas.default} 100%)` - : 'transparent', - transform: `translateX(${translateX}px)`, - display: `${display}` -}) - -export const getArrowBtnStyles = (theme?: Theme, direction = 'left') => ({ - fontWeight: 'normal', - boxShadow: 'none', - margin: 0, - border: 0, - borderRadius: 2, - paddingX: '14px', - paddingY: 0, - background: 'transparent', - height: '60%', - - '&:hover:not([disabled]), &:focus-visible': { - background: theme?.colors.canvas.default - }, - '&:focus:not(:disabled)': { - outline: 0, - boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}`, - background: `linear-gradient(to ${direction} ,#fff0, ${theme?.colors.canvas.default} 14px, ${theme?.colors.canvas.default} 100%)` - }, - // where focus-visible is supported, remove the focus box-shadow - '&:not(:focus-visible)': { - boxShadow: 'none', - background: `linear-gradient(to ${direction} ,#fff0, ${theme?.colors.canvas.default} 14px, ${theme?.colors.canvas.default} 100%)` - }, - '&:focus-visible:not(:disabled)': { - boxShadow: `inset 0 0 0 2px ${theme?.colors.accent.fg}`, - background: `linear-gradient(to ${direction} ,#fff0, ${theme?.colors.canvas.default} 14px, ${theme?.colors.canvas.default} 100%)` - } -}) - export const getLinkStyles = ( theme?: Theme, props?: Partial>, @@ -131,7 +85,6 @@ export const getLinkStyles = ( color: 'fg.default', textAlign: 'center', textDecoration: 'none', - paddingX: 1, ...(props?.variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), '@media (hover:hover)': { '&:hover > div[data-component="wrapper"] ': { @@ -166,7 +119,7 @@ export const getLinkStyles = ( position: 'absolute', right: '50%', bottom: 0, - width: `calc(100% - 8px)`, + width: '100%', height: 2, content: '""', bg: selectedLink === ref ? theme?.colors.primer.border.active : 'transparent', @@ -181,24 +134,11 @@ 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'} - export const menuItemStyles = { // This is needed to hide the selected check icon on the menu item. https://github.com/primer/react/blob/main/src/ActionList/Selection.tsx#L32 '& > span': { display: 'none' - } + }, + // To reset the style when the menu items are rendered as react router links + textDecoration: 'none' } diff --git a/src/UnderlineNav2/types.ts b/src/UnderlineNav2/types.ts index 7f49d92eb58..4dbef69a415 100644 --- a/src/UnderlineNav2/types.ts +++ b/src/UnderlineNav2/types.ts @@ -1,9 +1,5 @@ -import {BetterSystemStyleObject} from '../sx' export type ChildWidthArray = Array<{text: string; width: number}> export type ResponsiveProps = { items: Array actions: Array - overflowStyles: BetterSystemStyleObject } - -export type OnScrollWithButtonEventType = (event: React.MouseEvent, direction: -1 | 1) => void