From 133695491286e49e2d15cdb82cdd7b924a269c8b Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 26 Aug 2022 18:39:26 +1000 Subject: [PATCH 01/18] UnderlineNav states with counters --- src/UnderlineNav2/UnderlineNav.tsx | 2 -- src/UnderlineNav2/UnderlineNavLink.tsx | 48 ++++++++++++++++++-------- src/UnderlineNav2/examples.stories.tsx | 23 +++++++----- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index dc99dddd689..a08ca7d099b 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -87,8 +87,6 @@ export const UnderlineNav = forwardRef( const styles = { display: 'flex', justifyContent: align === 'right' ? 'flex-end' : 'space-between', - borderBottom: '1px solid', - borderBottomColor: 'border.muted', align: 'row', alignItems: 'center' } diff --git a/src/UnderlineNav2/UnderlineNavLink.tsx b/src/UnderlineNav2/UnderlineNavLink.tsx index aa417edf98b..b7e6dd04750 100644 --- a/src/UnderlineNav2/UnderlineNavLink.tsx +++ b/src/UnderlineNav2/UnderlineNavLink.tsx @@ -4,6 +4,7 @@ import {merge, SxProp, BetterSystemStyleObject} from '../sx' import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' +import CounterLabel from '../CounterLabel' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -36,6 +37,10 @@ export type UnderlineNavLinkProps = { */ leadingIcon?: React.FunctionComponent as?: React.ElementType + /** + * Counter + */ + counter?: number } & SxProp & LinkProps @@ -46,6 +51,7 @@ export const UnderlineNavLink = forwardRef( as: Component = 'a', href = '#', children, + counter, onSelect, selected: preSelected = false, leadingIcon: LeadingIcon, @@ -70,30 +76,36 @@ export const UnderlineNavLink = forwardRef( const textStyles: BetterSystemStyleObject = { whiteSpace: 'nowrap' } - const smallVariantLinkStyles = { - paddingY: 2, - fontSize: 0 - } - const defaultVariantLinkStyles = { - paddingY: 3, - fontSize: 1 - } + // Styling the anchor tag const linkStyles = { display: 'inline-flex', color: 'fg.default', textAlign: 'center', - borderBottom: '2px solid transparent', - borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', textDecoration: 'none', paddingX: 2, - marginRight: 3, - ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), - '&:hover, &:focus': { - borderColor: selectedLink === ref ? 'primer.border.active' : 'neutral.muted', + paddingY: 1, + ...(variant === 'small' ? {fontSize: 0} : {fontSize: 1}), + '&:hover': { + backgroundColor: 'neutral.muted', + borderRadius: 2 + }, + '&:focus': { + outlineColor: 'fg.accent', + borderRadius: 2, transition: '0.2s ease' } } + // Styling the li list item + const listItemStyles = { + ...(variant === 'small' ? {paddingY: 1} : {paddingY: 2}), + borderBottom: '2px solid transparent', + borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', + marginRight: 3 + } + const counterStyles = { + marginLeft: 2 + } const keyPressHandler = React.useCallback( event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { @@ -118,7 +130,7 @@ export const UnderlineNavLink = forwardRef( [onSelect, afterSelect, ref, setSelectedLink] ) return ( - + )} + {children && ( {children} )} + {counter && ( + + {counter} + + )} ) diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 4a0bab143d6..2c88e415a0d 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -1,8 +1,7 @@ import React from 'react' -import {EyeIcon, CodeIcon, IssueOpenedIcon} 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 CounterLabel from '../CounterLabel' import {BaseStyles, ThemeProvider} from '..' export default { @@ -49,10 +48,21 @@ export const DefaultNav = (args: UnderlineNavProps) => { export const withIcons = (args: UnderlineNavProps) => { return ( + + Code + + + Issues + + + Pull Requests + + + Discussions + Item 1 - Item 2 ) } @@ -63,11 +73,8 @@ export const withCounterLabels = (args: UnderlineNavProps) => { Code - - Issues{' '} - - 12 - + + Issues ) From 77e5d788abc1a9807644d7bef2b2b9811bbc67e1 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 26 Aug 2022 22:02:57 +1000 Subject: [PATCH 02/18] update li and a hover and focus states --- src/UnderlineNav2/UnderlineNavLink.tsx | 23 ++++++++++++----------- src/UnderlineNav2/examples.stories.tsx | 4 ++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNavLink.tsx b/src/UnderlineNav2/UnderlineNavLink.tsx index b7e6dd04750..456c74c2a7c 100644 --- a/src/UnderlineNav2/UnderlineNavLink.tsx +++ b/src/UnderlineNav2/UnderlineNavLink.tsx @@ -81,16 +81,11 @@ export const UnderlineNavLink = forwardRef( const linkStyles = { display: 'inline-flex', color: 'fg.default', - textAlign: 'center', textDecoration: 'none', paddingX: 2, paddingY: 1, ...(variant === 'small' ? {fontSize: 0} : {fontSize: 1}), - '&:hover': { - backgroundColor: 'neutral.muted', - borderRadius: 2 - }, - '&:focus': { + '&:focus ': { outlineColor: 'fg.accent', borderRadius: 2, transition: '0.2s ease' @@ -98,10 +93,18 @@ export const UnderlineNavLink = forwardRef( } // Styling the li list item const listItemStyles = { + textAlign: 'center', ...(variant === 'small' ? {paddingY: 1} : {paddingY: 2}), borderBottom: '2px solid transparent', borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', - marginRight: 3 + marginRight: 3, + '&:hover': { + cursor: 'pointer' + }, + '&:hover > a': { + backgroundColor: 'neutral.muted', + borderRadius: 2 + } } const counterStyles = { marginLeft: 2 @@ -130,16 +133,14 @@ export const UnderlineNavLink = forwardRef( [onSelect, afterSelect, ref, setSelectedLink] ) return ( - + {LeadingIcon && ( diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 2c88e415a0d..2803a6d2b96 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -51,13 +51,13 @@ export const withIcons = (args: UnderlineNavProps) => { Code - + Issues Pull Requests - + Discussions From 5011257ca6143fce88533c15f22a079b25ea12b9 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 12:03:30 +1000 Subject: [PATCH 03/18] add a wrapper to style hover --- ...erlineNavLink.tsx => UnderlineNavItem.tsx} | 103 +++++++++++------- src/UnderlineNav2/examples.stories.tsx | 48 ++++---- src/UnderlineNav2/index.ts | 6 +- 3 files changed, 88 insertions(+), 69 deletions(-) rename src/UnderlineNav2/{UnderlineNavLink.tsx => UnderlineNavItem.tsx} (70%) diff --git a/src/UnderlineNav2/UnderlineNavLink.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx similarity index 70% rename from src/UnderlineNav2/UnderlineNavLink.tsx rename to src/UnderlineNav2/UnderlineNavItem.tsx index 456c74c2a7c..a40a2b8453a 100644 --- a/src/UnderlineNav2/UnderlineNavLink.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -19,7 +19,7 @@ type LinkProps = { referrerPolicy?: React.AnchorHTMLAttributes['referrerPolicy'] } -export type UnderlineNavLinkProps = { +export type UnderlineNavItemProps = { /** * Primary content for an NavLink */ @@ -44,7 +44,7 @@ export type UnderlineNavLinkProps = { } & SxProp & LinkProps -export const UnderlineNavLink = forwardRef( +export const UnderlineNavItem = forwardRef( ( { sx: sxProp = {}, @@ -67,6 +67,8 @@ export const UnderlineNavLink = forwardRef( setChildrenWidth({width: domRect.width}) preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject) }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth]) + + // Styles const iconWrapStyles = { alignItems: 'center', display: 'inline-flex', @@ -77,38 +79,52 @@ export const UnderlineNavLink = forwardRef( whiteSpace: 'nowrap' } - // Styling the anchor tag - const linkStyles = { + const wrapperStyles = { display: 'inline-flex', - color: 'fg.default', - textDecoration: 'none', - paddingX: 2, paddingY: 1, - ...(variant === 'small' ? {fontSize: 0} : {fontSize: 1}), - '&:focus ': { - outlineColor: 'fg.accent', - borderRadius: 2, - transition: '0.2s ease' - } + paddingX: 1 + // '&:before': { + // content: '""', + // position: 'absolute', + // left: '-10px', + // top: '-20px', + // bottom: '-40px', + // right: '-5px', + // border: '2px solid', + // borderRightWidth: '4px', + // borderLeftWidth: '5px' + // } + } + const smallVariantLinkStyles = { + paddingY: 1, + fontSize: 0 } - // Styling the li list item - const listItemStyles = { + const defaultVariantLinkStyles = { + paddingY: 2, + fontSize: 1 + } + + const linkStyles = { + display: 'inline-flex', + color: 'fg.default', textAlign: 'center', - ...(variant === 'small' ? {paddingY: 1} : {paddingY: 2}), borderBottom: '2px solid transparent', borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', + textDecoration: 'none', + paddingX: 1, marginRight: 3, - '&:hover': { - cursor: 'pointer' - }, - '&:hover > a': { + ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), + '&:hover > div[data-component="wrapper"] ': { backgroundColor: 'neutral.muted', borderRadius: 2 + }, + '&:focus': { + outlineColor: 'fg.accent', + borderRadius: 2, + outlineOffset: '-6px', + transition: '0.2s ease' } } - const counterStyles = { - marginLeft: 2 - } const keyPressHandler = React.useCallback( event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { @@ -133,33 +149,36 @@ export const UnderlineNavLink = forwardRef( [onSelect, afterSelect, ref, setSelectedLink] ) return ( - + - {LeadingIcon && ( - - - - )} - - {children && ( - - {children} - - )} - {counter && ( - - {counter} - - )} + + {LeadingIcon && ( + + + + )} + {children && ( + + {children} + + )} + {counter && ( + + {counter} + + )} + ) } -) as PolymorphicForwardRefComponent<'a', UnderlineNavLinkProps> +) as PolymorphicForwardRefComponent<'a', UnderlineNavItemProps> diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 2803a6d2b96..2cf9031b4a9 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -38,9 +38,9 @@ export default { export const DefaultNav = (args: UnderlineNavProps) => { return ( - Item 1 - Item 2 - Item 3 + Item 1 + Item 2 + Item 3 ) } @@ -48,21 +48,21 @@ export const DefaultNav = (args: UnderlineNavProps) => { export const withIcons = (args: UnderlineNavProps) => { return ( - + Code - - + + Issues - - + + Pull Requests - - + + Discussions - - + + Item 1 - + ) } @@ -70,12 +70,12 @@ export const withIcons = (args: UnderlineNavProps) => { export const withCounterLabels = (args: UnderlineNavProps) => { return ( - + Code - - + + Issues - + ) } @@ -83,9 +83,9 @@ export const withCounterLabels = (args: UnderlineNavProps) => { export const rightAlign = (args: UnderlineNavProps) => { return ( - Item 1 - Item 2dsjsjskdjkajsdhkajsdkasj - Item 3 + Item 1 + Item 2dsjsjskdjkajsdhkajsdkasj + Item 3 ) } @@ -109,14 +109,14 @@ export const InternalResponsiveNav = (args: UnderlineNavProps) => { return ( {items.map((item, index) => ( - setSelectedIndex(index)} > {item} - + ))} ) @@ -126,9 +126,9 @@ export const HorizontalScrollNav = (args: UnderlineNavProps) => { return ( {items.map(item => ( - + {item} - + ))} ) diff --git a/src/UnderlineNav2/index.ts b/src/UnderlineNav2/index.ts index d29b9d7503f..4b0458df4e8 100644 --- a/src/UnderlineNav2/index.ts +++ b/src/UnderlineNav2/index.ts @@ -1,10 +1,10 @@ import {UnderlineNav as Nav, UnderlineNavProps} from './UnderlineNav' -import {UnderlineNavLink, UnderlineNavLinkProps} from './UnderlineNavLink' +import {UnderlineNavItem, UnderlineNavItemProps} from './UnderlineNavItem' const UnderlineNav = Object.assign(Nav, { - Link: UnderlineNavLink + Item: UnderlineNavItem }) export default UnderlineNav -export type {UnderlineNavProps, UnderlineNavLinkProps} +export type {UnderlineNavProps, UnderlineNavItemProps} From 7ea1e53e580f17922883ccbc917d1d36ec85ed3c Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 14:51:19 +1000 Subject: [PATCH 04/18] Fix rounded bordem bottom when rounding the outline --- src/UnderlineNav2/UnderlineNav.tsx | 3 ++- src/UnderlineNav2/UnderlineNavItem.tsx | 30 ++++++++++++-------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index a08ca7d099b..c7dec9cd8f8 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -96,7 +96,8 @@ export const UnderlineNav = forwardRef( listStyle: 'none', padding: '0', margin: '0', - marginBottom: '-1px' + marginBottom: '-1px', + alignItems: 'center' } const [selectedLink, setSelectedLink] = useState | undefined>(undefined) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index a40a2b8453a..4275679deab 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -68,7 +68,6 @@ export const UnderlineNavItem = forwardRef( preSelected && selectedLink === undefined && setSelectedLink(ref as RefObject) }, [ref, preSelected, selectedLink, setSelectedLink, setChildrenWidth]) - // Styles const iconWrapStyles = { alignItems: 'center', display: 'inline-flex', @@ -83,17 +82,6 @@ export const UnderlineNavItem = forwardRef( display: 'inline-flex', paddingY: 1, paddingX: 1 - // '&:before': { - // content: '""', - // position: 'absolute', - // left: '-10px', - // top: '-20px', - // bottom: '-40px', - // right: '-5px', - // border: '2px solid', - // borderRightWidth: '4px', - // borderLeftWidth: '5px' - // } } const smallVariantLinkStyles = { paddingY: 1, @@ -108,8 +96,6 @@ export const UnderlineNavItem = forwardRef( display: 'inline-flex', color: 'fg.default', textAlign: 'center', - borderBottom: '2px solid transparent', - borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', textDecoration: 'none', paddingX: 1, marginRight: 3, @@ -125,6 +111,17 @@ export const UnderlineNavItem = forwardRef( transition: '0.2s ease' } } + + const borderStyles = { + // How to use primer primitives for space 3? + width: `calc(100% - 16px)`, + height: 2, + backgroundColor: 'primer.border.active' + } + + const counterStyles = { + marginLeft: 2 + } const keyPressHandler = React.useCallback( event => { if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { @@ -149,7 +146,7 @@ export const UnderlineNavItem = forwardRef( [onSelect, afterSelect, ref, setSelectedLink] ) return ( - + )} {counter && ( - + {counter} )} + {selectedLink === ref && } ) } From a78bfec010812e56028486277f75e83d3fcdc561 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 15:03:30 +1000 Subject: [PATCH 05/18] fix padding and color name --- src/UnderlineNav2/UnderlineNavItem.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 4275679deab..57f2702f4b6 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -81,7 +81,7 @@ export const UnderlineNavItem = forwardRef( const wrapperStyles = { display: 'inline-flex', paddingY: 1, - paddingX: 1 + paddingX: 2 } const smallVariantLinkStyles = { paddingY: 1, @@ -105,7 +105,7 @@ export const UnderlineNavItem = forwardRef( borderRadius: 2 }, '&:focus': { - outlineColor: 'fg.accent', + outlineColor: 'accent.fg', borderRadius: 2, outlineOffset: '-6px', transition: '0.2s ease' From d465f462942255c9922bfd3133171d5d0d36152a Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 15:15:33 +1000 Subject: [PATCH 06/18] move radius to wrapper initial style --- src/UnderlineNav2/UnderlineNavItem.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 57f2702f4b6..1b684993b7e 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -81,7 +81,8 @@ export const UnderlineNavItem = forwardRef( const wrapperStyles = { display: 'inline-flex', paddingY: 1, - paddingX: 2 + paddingX: 2, + borderRadius: 2 } const smallVariantLinkStyles = { paddingY: 1, @@ -101,8 +102,7 @@ export const UnderlineNavItem = forwardRef( marginRight: 3, ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), '&:hover > div[data-component="wrapper"] ': { - backgroundColor: 'neutral.muted', - borderRadius: 2 + backgroundColor: 'neutral.muted' }, '&:focus': { outlineColor: 'accent.fg', From 8e17fe979d6ac3617a92df61bf135b6ea9ddc328 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 15:44:21 +1000 Subject: [PATCH 07/18] use px rather than primitive --- src/UnderlineNav2/UnderlineNavItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 1b684993b7e..c87267290af 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -106,7 +106,7 @@ export const UnderlineNavItem = forwardRef( }, '&:focus': { outlineColor: 'accent.fg', - borderRadius: 2, + borderRadius: '12px', outlineOffset: '-6px', transition: '0.2s ease' } From cbfdcff5fd2e7976bbe04d979fa44ef7c254a6c0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Tue, 30 Aug 2022 15:43:10 +1000 Subject: [PATCH 08/18] remove margin and change the width of select state line --- src/UnderlineNav2/UnderlineNavItem.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index c87267290af..f93b74524d6 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -99,7 +99,6 @@ export const UnderlineNavItem = forwardRef( textAlign: 'center', textDecoration: 'none', paddingX: 1, - marginRight: 3, ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), '&:hover > div[data-component="wrapper"] ': { backgroundColor: 'neutral.muted' @@ -114,7 +113,7 @@ export const UnderlineNavItem = forwardRef( const borderStyles = { // How to use primer primitives for space 3? - width: `calc(100% - 16px)`, + width: `calc(100% - 8px)`, height: 2, backgroundColor: 'primer.border.active' } @@ -146,7 +145,7 @@ export const UnderlineNavItem = forwardRef( [onSelect, afterSelect, ref, setSelectedLink] ) return ( - + Date: Tue, 30 Aug 2022 15:58:26 +1000 Subject: [PATCH 09/18] add the underline back --- src/UnderlineNav2/UnderlineNav.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index c7dec9cd8f8..73bde905d1e 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -87,6 +87,8 @@ export const UnderlineNav = forwardRef( const styles = { display: 'flex', justifyContent: align === 'right' ? 'flex-end' : 'space-between', + borderBottom: '1px solid', + borderBottomColor: 'border.muted', align: 'row', alignItems: 'center' } From fa2650db836331ababc3ccde46802da9684ec472 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 1 Sep 2022 14:47:58 +1000 Subject: [PATCH 10/18] focus and selected border re-implement --- src/UnderlineNav2/UnderlineNavItem.tsx | 54 +++++++++++++++++++------- src/UnderlineNav2/examples.stories.tsx | 12 ++---- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index f93b74524d6..58d0281a487 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -5,6 +5,7 @@ import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' +import {get} from '../constants' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -94,30 +95,51 @@ export const UnderlineNavItem = forwardRef( } const linkStyles = { + position: 'relative', display: 'inline-flex', color: 'fg.default', textAlign: 'center', textDecoration: 'none', paddingX: 1, + borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), '&:hover > div[data-component="wrapper"] ': { - backgroundColor: 'neutral.muted' + backgroundColor: 'neutral.muted', + transition: 'background .12s ease-out' }, '&:focus': { - outlineColor: 'accent.fg', - borderRadius: '12px', - outlineOffset: '-6px', - transition: '0.2s ease' + outline: 0, + '& > div[data-component="wrapper"]': { + boxShadow: `inset 0 0 0 2px #0969da` + }, + '&:not(:focus-visible) > div[data-component="wrapper"]': { + boxShadow: 'none' + } + }, + '&:focus-visible > div[data-component="wrapper"]': { + boxShadow: `inset 0 0 0 2px #0969da` + }, + '& span[data-content]::before': { + content: 'attr(data-content)', + display: 'block', + height: 0, + fontWeight: '600', + visibility: 'hidden' + }, + '&::after': { + position: 'absolute', + right: '50%', + // 48px total height / 2 (24px) + 1px + bottom: 'calc(50% - 23px)', + width: `calc(100% - 8px)`, + height: 2, + content: '""', + bg: selectedLink === ref ? 'primer.border.active' : 'transparent', + borderRadius: 0, + transform: 'translate(50%, -50%)' } } - const borderStyles = { - // How to use primer primitives for space 3? - width: `calc(100% - 8px)`, - height: 2, - backgroundColor: 'primer.border.active' - } - const counterStyles = { marginLeft: 2 } @@ -163,7 +185,12 @@ export const UnderlineNavItem = forwardRef( )} {children && ( - + {children} )} @@ -174,7 +201,6 @@ export const UnderlineNavItem = forwardRef( )} - {selectedLink === ref && } ) } diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index 2cf9031b4a9..bf644a14d54 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -48,21 +48,17 @@ export const DefaultNav = (args: UnderlineNavProps) => { export const withIcons = (args: UnderlineNavProps) => { return ( - - Code - - + Code + Issues Pull Requests - + Discussions - - Item 1 - + Item 1 ) } From a899582f5248fc2e421593f86e58c9648daee2aa Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Thu, 1 Sep 2022 21:25:55 +1000 Subject: [PATCH 11/18] add comments --- src/UnderlineNav2/UnderlineNavItem.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 58d0281a487..62c66cc8ccc 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -5,7 +5,6 @@ import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' -import {get} from '../constants' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -112,6 +111,7 @@ export const UnderlineNavItem = forwardRef( '& > div[data-component="wrapper"]': { boxShadow: `inset 0 0 0 2px #0969da` }, + // where focus-visible is supported, remove the focus box-shadow '&:not(:focus-visible) > div[data-component="wrapper"]': { boxShadow: 'none' } @@ -119,6 +119,7 @@ export const UnderlineNavItem = forwardRef( '&:focus-visible > div[data-component="wrapper"]': { boxShadow: `inset 0 0 0 2px #0969da` }, + // 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', @@ -126,10 +127,10 @@ export const UnderlineNavItem = forwardRef( fontWeight: '600', visibility: 'hidden' }, + // selected state styles '&::after': { position: 'absolute', right: '50%', - // 48px total height / 2 (24px) + 1px bottom: 'calc(50% - 23px)', width: `calc(100% - 8px)`, height: 2, From fcf6cf5e0723dd08e5f69b9daf086ade18980beb Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 2 Sep 2022 21:29:40 +1000 Subject: [PATCH 12/18] focus trap and fixes --- src/UnderlineNav2/UnderlineNav.tsx | 14 +++++++++++++- src/UnderlineNav2/UnderlineNavItem.tsx | 7 ++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 73bde905d1e..09ed33bcceb 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -5,6 +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 {FocusKeys} from '@primer/behaviors' type Overflow = 'auto' | 'menu' | 'scroll' type ChildWidthArray = Array<{width: number}> @@ -84,6 +86,16 @@ export const UnderlineNav = forwardRef( ) => { const backupRef = useRef(null) const newRef = (forwardedRef ?? backupRef) as MutableRefObject + + // 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 styles = { display: 'flex', justifyContent: align === 'right' ? 'flex-end' : 'space-between', @@ -144,7 +156,7 @@ export const UnderlineNav = forwardRef( - + (overflowStyles, ulStyles)}> {responsiveProps.items} diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 62c66cc8ccc..8e82bb76fb8 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -5,6 +5,7 @@ import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' +import {get} from '../constants' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -109,7 +110,7 @@ export const UnderlineNavItem = forwardRef( '&:focus': { outline: 0, '& > div[data-component="wrapper"]': { - boxShadow: `inset 0 0 0 2px #0969da` + boxShadow: `inset 0 0 0 2px ${get('colors.accent.fg')}` }, // where focus-visible is supported, remove the focus box-shadow '&:not(:focus-visible) > div[data-component="wrapper"]': { @@ -117,7 +118,7 @@ export const UnderlineNavItem = forwardRef( } }, '&:focus-visible > div[data-component="wrapper"]': { - boxShadow: `inset 0 0 0 2px #0969da` + boxShadow: `inset 0 0 0 2px ${get('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': { @@ -131,7 +132,7 @@ export const UnderlineNavItem = forwardRef( '&::after': { position: 'absolute', right: '50%', - bottom: 'calc(50% - 23px)', + bottom: 0, width: `calc(100% - 8px)`, height: 2, content: '""', From f73f9ea50edb1d464ef13d75f438c9743a3334c0 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 10:16:44 +1000 Subject: [PATCH 13/18] theme aware underlineNav and fix safari extra margin issue --- src/UnderlineNav2/UnderlineNav.tsx | 2 +- src/UnderlineNav2/UnderlineNavItem.tsx | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index 09ed33bcceb..f5c94fbe2b4 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -163,7 +163,7 @@ export const UnderlineNav = forwardRef( {actions.length > 0 && ( - More + More {actions.map((action, index) => { diff --git a/src/UnderlineNav2/UnderlineNavItem.tsx b/src/UnderlineNav2/UnderlineNavItem.tsx index 8e82bb76fb8..302ae12b411 100644 --- a/src/UnderlineNav2/UnderlineNavItem.tsx +++ b/src/UnderlineNav2/UnderlineNavItem.tsx @@ -5,7 +5,7 @@ import {IconProps} from '@primer/octicons-react' import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import CounterLabel from '../CounterLabel' -import {get} from '../constants' +import {Theme, useTheme} from '../ThemeProvider' // adopted from React.AnchorHTMLAttributes type LinkProps = { @@ -63,6 +63,7 @@ export const UnderlineNavItem = forwardRef( const backupRef = useRef(null) const ref = forwardedRef ?? backupRef const {setChildrenWidth, selectedLink, setSelectedLink, afterSelect, variant} = useContext(UnderlineNavContext) + const {theme} = useTheme() useLayoutEffect(() => { const domRect = (ref as MutableRefObject).current.getBoundingClientRect() setChildrenWidth({width: domRect.width}) @@ -94,23 +95,23 @@ export const UnderlineNavItem = forwardRef( fontSize: 1 } - const linkStyles = { + // eslint-disable-next-line no-shadow + const linkStyles = (theme?: Theme) => ({ position: 'relative', display: 'inline-flex', color: 'fg.default', textAlign: 'center', textDecoration: 'none', paddingX: 1, - borderColor: selectedLink === ref ? 'primer.border.active' : 'transparent', ...(variant === 'small' ? smallVariantLinkStyles : defaultVariantLinkStyles), '&:hover > div[data-component="wrapper"] ': { - backgroundColor: 'neutral.muted', + backgroundColor: theme?.colors.neutral.muted, transition: 'background .12s ease-out' }, '&:focus': { outline: 0, '& > div[data-component="wrapper"]': { - boxShadow: `inset 0 0 0 2px ${get('colors.accent.fg')}` + 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"]': { @@ -118,7 +119,7 @@ export const UnderlineNavItem = forwardRef( } }, '&:focus-visible > div[data-component="wrapper"]': { - boxShadow: `inset 0 0 0 2px ${get('colors.accent.fg')}` + 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': { @@ -136,11 +137,11 @@ export const UnderlineNavItem = forwardRef( width: `calc(100% - 8px)`, height: 2, content: '""', - bg: selectedLink === ref ? 'primer.border.active' : 'transparent', + bg: selectedLink === ref ? theme?.colors.primer.border.active : 'transparent', borderRadius: 0, transform: 'translate(50%, -50%)' } - } + }) const counterStyles = { marginLeft: 2 @@ -176,7 +177,7 @@ export const UnderlineNavItem = forwardRef( onKeyPress={keyPressHandler} onClick={clickHandler} {...(selectedLink === ref ? {'aria-current': 'page'} : {})} - sx={merge(linkStyles, sxProp as SxProp)} + sx={merge(linkStyles(theme), sxProp as SxProp)} {...props} ref={ref} > From 2a33c2ea8c24a7d0c0a74f25eb668807e04e2d54 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 10:20:17 +1000 Subject: [PATCH 14/18] add changeset --- .changeset/thirty-mayflies-travel.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thirty-mayflies-travel.md diff --git a/.changeset/thirty-mayflies-travel.md b/.changeset/thirty-mayflies-travel.md new file mode 100644 index 00000000000..f6144ed3e98 --- /dev/null +++ b/.changeset/thirty-mayflies-travel.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +UnderlineNav.Link renamed to UnderlineNav.Item along with updated styles From 11f2e83d8a5eb30482b866986d339b90447a4100 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 10:57:30 +1000 Subject: [PATCH 15/18] update docs and tests --- docs/content/drafts/UnderlineNav2.mdx | 18 +++++++++--------- src/UnderlineNav2/UnderlineNav.test.tsx | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/content/drafts/UnderlineNav2.mdx b/docs/content/drafts/UnderlineNav2.mdx index 454c2116709..402b9054ef7 100644 --- a/docs/content/drafts/UnderlineNav2.mdx +++ b/docs/content/drafts/UnderlineNav2.mdx @@ -10,9 +10,9 @@ description: Use an underlined nav to allow tab like navigation with overflow be ```jsx live drafts - Item 1 - Item 2 - Item 3 + Item 1 + Item 2 + Item 3 ``` @@ -20,10 +20,10 @@ description: Use an underlined nav to allow tab like navigation with overflow be ```jsx live drafts - + Item 1 - - Item 2 + + Item 2 ``` @@ -31,8 +31,8 @@ description: Use an underlined nav to allow tab like navigation with overflow be ```jsx live drafts - Item 1 - Item 2 + Item 1 + Item 2 ``` @@ -65,7 +65,7 @@ description: Use an underlined nav to allow tab like navigation with overflow be -### UnderlineNav.Link +### UnderlineNav.Item diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index fb122084e77..5b81ced8e95 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -8,9 +8,9 @@ describe('UnderlineNav', () => { test('selected nav', () => { const {getByText} = render( - Item 1 - Item 2 - Item 3 + Item 1 + Item 2 + Item 3 ) const selectedNavLink = getByText('Item 1').closest('a') @@ -20,9 +20,9 @@ describe('UnderlineNav', () => { test('basic nav functionality', () => { const {container} = render( - Item 1 - Item 2 - Item 3 + Item 1 + Item 2 + Item 3 ) expect(container.getElementsByTagName('nav').length).toEqual(1) @@ -33,9 +33,9 @@ describe('UnderlineNav', () => { test('respect align prop', () => { const {container} = render( - Item 1 - Item 2 - Item 3 + Item 1 + Item 2 + Item 3 ) const nav = container.getElementsByTagName('nav')[0] From 879268122e589f39ce05f8d57493f82b9e171551 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 14:54:45 +1000 Subject: [PATCH 16/18] fix export issues --- src/UnderlineNav2/examples.stories.tsx | 2 +- src/UnderlineNav2/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/UnderlineNav2/examples.stories.tsx b/src/UnderlineNav2/examples.stories.tsx index bf644a14d54..5e8a7fea4b7 100644 --- a/src/UnderlineNav2/examples.stories.tsx +++ b/src/UnderlineNav2/examples.stories.tsx @@ -1,7 +1,7 @@ import React from 'react' import {EyeIcon, CodeIcon, IssueOpenedIcon, GitPullRequestIcon, CommentDiscussionIcon} from '@primer/octicons-react' import {Meta} from '@storybook/react' -import UnderlineNav, {UnderlineNavProps} from './index' +import {UnderlineNav, UnderlineNavProps} from './index' import {BaseStyles, ThemeProvider} from '..' export default { diff --git a/src/UnderlineNav2/index.ts b/src/UnderlineNav2/index.ts index 4b0458df4e8..1029f372e61 100644 --- a/src/UnderlineNav2/index.ts +++ b/src/UnderlineNav2/index.ts @@ -5,6 +5,6 @@ const UnderlineNav = Object.assign(Nav, { Item: UnderlineNavItem }) -export default UnderlineNav +export {UnderlineNav} export type {UnderlineNavProps, UnderlineNavItemProps} From e1f0a8c67dc53fcd7a7cafe4c9d74a2fc24ebc7c Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 15:08:05 +1000 Subject: [PATCH 17/18] add a comment and fix the test import --- src/UnderlineNav2/UnderlineNav.test.tsx | 2 +- src/UnderlineNav2/UnderlineNav.tsx | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/UnderlineNav2/UnderlineNav.test.tsx b/src/UnderlineNav2/UnderlineNav.test.tsx index 5b81ced8e95..df2090df2a7 100644 --- a/src/UnderlineNav2/UnderlineNav.test.tsx +++ b/src/UnderlineNav2/UnderlineNav.test.tsx @@ -2,7 +2,7 @@ import React from 'react' import '@testing-library/jest-dom/extend-expect' import {render} from '@testing-library/react' -import UnderlineNav from '.' +import {UnderlineNav} from '.' describe('UnderlineNav', () => { test('selected nav', () => { diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index f5c94fbe2b4..11f64403a30 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -163,6 +163,7 @@ export const UnderlineNav = forwardRef( {actions.length > 0 && ( + {/* set margin 0 here because safari puts extra margin around the button */} More From 0699ae0e2f97c924b10b3b378895384e13824546 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 17:37:52 +1000 Subject: [PATCH 18/18] update changeset to minor --- .changeset/thirty-mayflies-travel.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/thirty-mayflies-travel.md b/.changeset/thirty-mayflies-travel.md index f6144ed3e98..bd9f674a32e 100644 --- a/.changeset/thirty-mayflies-travel.md +++ b/.changeset/thirty-mayflies-travel.md @@ -1,5 +1,5 @@ --- -'@primer/react': major +'@primer/react': minor --- UnderlineNav.Link renamed to UnderlineNav.Item along with updated styles