From da778f2edf417a8956301802330cec7f4f65eee2 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 9 May 2023 16:50:27 -0400 Subject: [PATCH 01/35] Using button as the underlying tag for ActionList.Item content. --- src/ActionList/Item.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 60126e3528a..39bce8c9338 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -198,7 +198,17 @@ export const Item = React.forwardRef( {slots.leadingVisual} Date: Wed, 10 May 2023 15:39:51 -0400 Subject: [PATCH 02/35] Fix subnav layout. --- src/ActionList/Item.tsx | 5 +++-- src/ActionList/shared.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 39bce8c9338..c7e2a024b26 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -207,8 +207,9 @@ export const Item = React.forwardRef( backgroundColor: 'transparent', cursor: 'inherit', fontSize: 'inherit', + paddingLeft: props.as === 'button' ? 0 : 'initial', }} - as="button" + as={props.as === 'button' ? 'div' : 'button'} > ( id={labelId} sx={{ flexGrow: slots.description && slots.description.props.variant !== 'block' ? 0 : 1, - fontWeight: slots.description && slots.description.props.variant !== 'block' ? 'bold' : 'normal', + fontWeight: slots.description && slots.description.props.variant !== 'block' ? 'bold' : 'inherit', }} > {childrenWithoutSlots} diff --git a/src/ActionList/shared.ts b/src/ActionList/shared.ts index ff77e7975b8..1fed110abaf 100644 --- a/src/ActionList/shared.ts +++ b/src/ActionList/shared.ts @@ -40,6 +40,7 @@ export type ActionListItemProps = { * id to attach to the root element of the Item */ id?: string + as?: React.ElementType /** * Private API for use internally only. Used by LinkItem to wrap contents in an anchor */ From ea0d12aad24068bad8027a153f191a5984755b99 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Thu, 11 May 2023 16:01:27 -0400 Subject: [PATCH 03/35] Changing ActionList content to button if the top-level is not a button. ActionMenu is busted. --- src/ActionList/Item.tsx | 5 +- src/ActionList/shared.ts | 1 - src/NavList/NavList.test.tsx | 2 +- .../__snapshots__/NavList.test.tsx.snap | 84 +++++++++++++++---- 4 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index c7e2a024b26..524e3f322b3 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -128,6 +128,9 @@ export const Item = React.forwardRef( borderTopWidth: showDividers ? `1px` : '0', borderColor: 'var(--divider-color, transparent)', }, + 'button[data-component="ActionList.Item--DividerContainer"]': { + paddingLeft: 0, + }, // show between 2 items ':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider}, // hide divider after dividers & group header, with higher importance! @@ -207,8 +210,8 @@ export const Item = React.forwardRef( backgroundColor: 'transparent', cursor: 'inherit', fontSize: 'inherit', - paddingLeft: props.as === 'button' ? 0 : 'initial', }} + // @ts-ignore `as` prop may be passed to ActionList.Item, even if it isn't defined in ActionListItemProps. as={props.as === 'button' ? 'div' : 'button'} > diff --git a/src/ActionList/shared.ts b/src/ActionList/shared.ts index 1fed110abaf..ff77e7975b8 100644 --- a/src/ActionList/shared.ts +++ b/src/ActionList/shared.ts @@ -40,7 +40,6 @@ export type ActionListItemProps = { * id to attach to the root element of the Item */ id?: string - as?: React.ElementType /** * Private API for use internally only. Used by LinkItem to wrap contents in an anchor */ diff --git a/src/NavList/NavList.test.tsx b/src/NavList/NavList.test.tsx index 08fe311ae60..4f545362fee 100644 --- a/src/NavList/NavList.test.tsx +++ b/src/NavList/NavList.test.tsx @@ -211,7 +211,7 @@ describe('NavList.Item with NavList.SubNav', () => { , ) - const button = getByRole('button') + const button = getByRole('button', {name: 'Item'}) // Starts open expect(queryByRole('list', {name: 'Item'})).toBeVisible() diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index 0bf63a4a99d..5a85205fb26 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -19,6 +19,10 @@ exports[`NavList renders a simple list 1`] = ` -ms-flex-positive: 1; flex-grow: 1; min-width: 0; + border-style: none; + background-color: transparent; + cursor: inherit; + font-size: inherit; } .c6 { @@ -26,7 +30,7 @@ exports[`NavList renders a simple list 1`] = ` -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: 400; + font-weight: inherit; } .c1 { @@ -87,6 +91,10 @@ exports[`NavList renders a simple list 1`] = ` border-color: var(--divider-color,transparent); } +.c3 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c3:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -169,6 +177,10 @@ exports[`NavList renders a simple list 1`] = ` border-color: var(--divider-color,transparent); } +.c7 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c7:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -307,7 +319,7 @@ exports[`NavList renders a simple list 1`] = ` href="/" tabindex="0" > -
@@ -317,7 +329,7 @@ exports[`NavList renders a simple list 1`] = ` > Home -
+
  • -
    @@ -339,7 +351,7 @@ exports[`NavList renders a simple list 1`] = ` > About -
    +
  • -
    @@ -361,7 +373,7 @@ exports[`NavList renders a simple list 1`] = ` > Contact -
    +
    • @@ -389,6 +401,10 @@ exports[`NavList renders with groups 1`] = ` -ms-flex-positive: 1; flex-grow: 1; min-width: 0; + border-style: none; + background-color: transparent; + cursor: inherit; + font-size: inherit; } .c10 { @@ -396,7 +412,7 @@ exports[`NavList renders with groups 1`] = ` -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: 400; + font-weight: inherit; } .c2 { @@ -491,6 +507,10 @@ exports[`NavList renders with groups 1`] = ` border-color: var(--divider-color,transparent); } +.c7 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c7:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -573,6 +593,10 @@ exports[`NavList renders with groups 1`] = ` border-color: var(--divider-color,transparent); } +.c11 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c11:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -733,7 +757,7 @@ exports[`NavList renders with groups 1`] = ` href="/getting-started" tabindex="0" > -
    @@ -743,7 +767,7 @@ exports[`NavList renders with groups 1`] = ` > Getting started -
    + @@ -779,7 +803,7 @@ exports[`NavList renders with groups 1`] = ` href="/Avatar" tabindex="0" > -
    @@ -789,7 +813,7 @@ exports[`NavList renders with groups 1`] = ` > Avatar -
    + @@ -819,6 +843,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav -ms-flex-positive: 1; flex-grow: 1; min-width: 0; + border-style: none; + background-color: transparent; + cursor: inherit; + font-size: inherit; } .c7 { @@ -826,7 +854,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: 400; + font-weight: inherit; } .c2 { @@ -924,6 +952,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav border-color: var(--divider-color,transparent); } +.c11 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c11:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -1049,6 +1081,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav border-color: var(--divider-color,transparent); } +.c4 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c4:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -1247,7 +1283,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav href="#" tabindex="0" > -
    @@ -1257,7 +1293,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav > Sub Item -
    + @@ -1288,6 +1324,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t -ms-flex-positive: 1; flex-grow: 1; min-width: 0; + border-style: none; + background-color: transparent; + cursor: inherit; + font-size: inherit; } .c7 { @@ -1295,7 +1335,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: 400; + font-weight: inherit; } .c2 { @@ -1393,6 +1433,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t border-color: var(--divider-color,transparent); } +.c11 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c11:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -1525,6 +1569,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t border-color: var(--divider-color,transparent); } +.c4 button[data-component="ActionList.Item--DividerContainer"] { + padding-left: 0; +} + .c4:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } @@ -1728,7 +1776,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t href="#" tabindex="0" > -
    @@ -1738,7 +1786,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t > Sub Item -
    + From ee3e4a1326a52a0d65bd188bf4427e09c9184d55 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Fri, 12 May 2023 15:45:14 -0400 Subject: [PATCH 04/35] Fix issue where ActionList.Item content would render as a button inside ActionMenu, breaking keyboard navigation. --- src/ActionList/Item.tsx | 5 +- src/ActionMenu.tsx | 138 ------------------ .../ActionMenu.test.tsx | 0 src/ActionMenu/ActionMenu.tsx | 3 +- .../ActionMenu.types.test.tsx | 0 .../__snapshots__/ActionMenu.test.tsx.snap | 0 6 files changed, 6 insertions(+), 140 deletions(-) delete mode 100644 src/ActionMenu.tsx rename src/{__tests__ => ActionMenu}/ActionMenu.test.tsx (100%) rename src/{__tests__ => ActionMenu}/ActionMenu.types.test.tsx (100%) rename src/{__tests__ => ActionMenu}/__snapshots__/ActionMenu.test.tsx.snap (100%) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 524e3f322b3..bd7623a51a3 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -13,6 +13,7 @@ import {ActionListProps, ListContext} from './List' import {Selection} from './Selection' import {ActionListItemProps, getVariantStyles, ItemContext, TEXT_ROW_HEIGHT} from './shared' import {LeadingVisual, TrailingVisual} from './Visuals' +import {ActionMenu} from '../ActionMenu' const LiBox = styled.li(sx) @@ -39,6 +40,7 @@ export const Item = React.forwardRef( }) const {variant: listVariant, showDividers, selectionVariant: listSelectionVariant} = React.useContext(ListContext) const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext) + const menuContext = React.useContext(ActionMenu.MenuContext) const selectionVariant: ActionListProps['selectionVariant'] = listSelectionVariant @@ -212,7 +214,8 @@ export const Item = React.forwardRef( fontSize: 'inherit', }} // @ts-ignore `as` prop may be passed to ActionList.Item, even if it isn't defined in ActionListItemProps. - as={props.as === 'button' ? 'div' : 'button'} + // If this item is inside an ActionMenu, don't render an interactive button. + as={props.as === 'button' || menuContext.anchorId !== undefined ? 'div' : 'button'} > & { - onClose?: (gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab') => void -} -const MenuContext = React.createContext({renderAnchor: null, open: false}) - -export type ActionMenuProps = { - /** - * Recommended: `ActionMenu.Button` or `ActionMenu.Anchor` with `ActionMenu.Overlay` - */ - children: React.ReactElement[] | React.ReactElement - - /** - * If defined, will control the open/closed state of the overlay. Must be used in conjunction with `onOpenChange`. - */ - open?: boolean - - /** - * If defined, will control the open/closed state of the overlay. Must be used in conjunction with `open`. - */ - onOpenChange?: (s: boolean) => void -} & Pick - -const Menu: React.FC> = ({ - anchorRef: externalAnchorRef, - open, - onOpenChange, - children, -}: ActionMenuProps) => { - const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, onOpenChange, false) - const onOpen = React.useCallback(() => setCombinedOpenState(true), [setCombinedOpenState]) - const onClose = React.useCallback(() => setCombinedOpenState(false), [setCombinedOpenState]) - - const anchorRef = useProvidedRefOrCreate(externalAnchorRef) - const anchorId = useId() - let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null - - // 🚨 Hack for good API! - // we strip out Anchor from children and pass it to AnchoredOverlay to render - // with additional props for accessibility - const contents = React.Children.map(children, child => { - if (child.type === MenuButton || child.type === Anchor) { - renderAnchor = anchorProps => React.cloneElement(child, anchorProps) - return null - } - return child - }) - - return ( - - {contents} - - ) -} - -export type ActionMenuAnchorProps = {children: React.ReactElement} -const Anchor = React.forwardRef(({children, ...anchorProps}, anchorRef) => { - return React.cloneElement(children, {...anchorProps, ref: anchorRef}) -}) - -/** this component is syntactical sugar 🍭 */ -export type ActionMenuButtonProps = ButtonProps -const MenuButton = React.forwardRef(({...props}, anchorRef) => { - return ( - - @@ -435,11 +435,11 @@ exports[`Token components AvatarToken renders all sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -1536,11 +1536,11 @@ exports[`Token components AvatarToken renders with a remove button 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -1709,11 +1709,11 @@ exports[`Token components IssueLabelToken renders all sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -1882,11 +1882,11 @@ exports[`Token components IssueLabelToken renders all sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -2658,11 +2658,11 @@ exports[`Token components IssueLabelToken renders custom fill color 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -2831,11 +2831,11 @@ exports[`Token components IssueLabelToken renders default fill color 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3115,11 +3115,11 @@ exports[`Token components IssueLabelToken renders with a remove button 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3295,11 +3295,11 @@ exports[`Token components Token renders all sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -3475,11 +3475,11 @@ exports[`Token components Token renders all sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -4462,11 +4462,11 @@ exports[`Token components Token renders with a remove button 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > diff --git a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap index 476f0bd791c..b3b1834f906 100644 --- a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap +++ b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap @@ -76,6 +76,18 @@ exports[`Pagination renders consistently 1`] = ` border-color: transparent; } +.c2[aria-disabled], +.c2[aria-disabled]:hover { + color: #8c959f; + cursor: default; + background-color: transparent; + border-color: transparent; + font-size: inherit; + font-family: inherit; + padding-top: inherit; + padding-bottom: inherit; +} + .c2[aria-disabled], .c2[aria-disabled]:hover, .c2[role='presentation'], @@ -123,12 +135,12 @@ exports[`Pagination renders consistently 1`] = ` className="c1" display="inline-block" > - Previous - + @@ -2520,11 +2520,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -2571,11 +2571,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -2622,11 +2622,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -2673,11 +2673,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -2724,11 +2724,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -2775,11 +2775,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -2826,11 +2826,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -3194,11 +3194,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3245,11 +3245,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3296,11 +3296,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3347,11 +3347,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3398,11 +3398,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3449,11 +3449,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3500,11 +3500,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -3551,11 +3551,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={15} > @@ -14718,11 +14718,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -14769,11 +14769,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -14820,11 +14820,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -14871,11 +14871,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -14922,11 +14922,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -14973,11 +14973,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -15024,11 +15024,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > @@ -15075,11 +15075,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 12 12" + viewBox="0 0 16 16" width={12} > From 6e6ccdd58919cbe440a0865cd042330502ff5e37 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Wed, 17 May 2023 10:41:01 -0400 Subject: [PATCH 23/35] Update snapshots. --- .../__snapshots__/Token.test.tsx.snap | 44 ++++----- .../TextInputWithTokens.test.tsx.snap | 96 +++++++++---------- 2 files changed, 70 insertions(+), 70 deletions(-) diff --git a/src/Token/__tests__/__snapshots__/Token.test.tsx.snap b/src/Token/__tests__/__snapshots__/Token.test.tsx.snap index 0eb72bbc832..f8b70368c1f 100644 --- a/src/Token/__tests__/__snapshots__/Token.test.tsx.snap +++ b/src/Token/__tests__/__snapshots__/Token.test.tsx.snap @@ -212,11 +212,11 @@ exports[`Token components AvatarToken renders all sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -435,11 +435,11 @@ exports[`Token components AvatarToken renders all sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -1536,11 +1536,11 @@ exports[`Token components AvatarToken renders with a remove button 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -1709,11 +1709,11 @@ exports[`Token components IssueLabelToken renders all sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -1882,11 +1882,11 @@ exports[`Token components IssueLabelToken renders all sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -2658,11 +2658,11 @@ exports[`Token components IssueLabelToken renders custom fill color 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -2831,11 +2831,11 @@ exports[`Token components IssueLabelToken renders default fill color 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3115,11 +3115,11 @@ exports[`Token components IssueLabelToken renders with a remove button 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3295,11 +3295,11 @@ exports[`Token components Token renders all sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -3475,11 +3475,11 @@ exports[`Token components Token renders all sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -4462,11 +4462,11 @@ exports[`Token components Token renders with a remove button 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > diff --git a/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap b/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap index f2119138454..bd1f7ba606b 100644 --- a/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap +++ b/src/__tests__/__snapshots__/TextInputWithTokens.test.tsx.snap @@ -2469,11 +2469,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2520,11 +2520,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2571,11 +2571,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2622,11 +2622,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2673,11 +2673,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2724,11 +2724,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2775,11 +2775,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -2826,11 +2826,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -3194,11 +3194,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3245,11 +3245,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3296,11 +3296,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3347,11 +3347,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3398,11 +3398,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3449,11 +3449,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3500,11 +3500,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -3551,11 +3551,11 @@ exports[`TextInputWithTokens renders tokens at the specified sizes 2`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={15} > @@ -14718,11 +14718,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -14769,11 +14769,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -14820,11 +14820,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -14871,11 +14871,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -14922,11 +14922,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -14973,11 +14973,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -15024,11 +15024,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > @@ -15075,11 +15075,11 @@ exports[`TextInputWithTokens renders with a loading indicator 1`] = ` "verticalAlign": "text-bottom", } } - viewBox="0 0 16 16" + viewBox="0 0 12 12" width={12} > From 3f03a12e405d41cd96cbbb7d7f0a3b287cb1768e Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Fri, 19 May 2023 09:35:23 -0400 Subject: [PATCH 24/35] If ActionList.Item content is rendered as a button, remove tabIndex from li and fix padding. --- src/ActionList/Item.tsx | 16 ++++++------- .../__snapshots__/NavList.test.tsx.snap | 23 +++++++------------ 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 51fe45e9c5d..3c155146153 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -72,12 +72,16 @@ export const Item = React.forwardRef( }, } + const isTopLevelInteractive = () => + // @ts-ignore props.as may be defined, may not. + props.as === 'button' || props.as === 'a' || menuContext.anchorId !== undefined || role?.match(/menuitem/) + const styles = { position: 'relative', display: 'flex', - paddingX: 2, + paddingX: isTopLevelInteractive() ? 2 : 0, fontSize: 1, - paddingY: '6px', // custom value off the scale + paddingY: isTopLevelInteractive() ? '6px' : 0, // custom value off the scale lineHeight: TEXT_ROW_HEIGHT, minHeight: 5, marginX: listVariant === 'inset' ? 2 : 0, @@ -134,7 +138,7 @@ export const Item = React.forwardRef( borderColor: 'var(--divider-color, transparent)', }, 'button[data-component="ActionList.Item--DividerContainer"]': { - padding: 0, + padding: '6px 8px', textAlign: 'left', }, // show between 2 items @@ -186,7 +190,7 @@ export const Item = React.forwardRef( onClick: clickHandler, onKeyPress: keyPressHandler, 'aria-disabled': disabled ? true : undefined, - tabIndex: disabled ? undefined : 0, + tabIndex: disabled || !isTopLevelInteractive() ? undefined : 0, 'aria-labelledby': `${labelId} ${ slots.description && slots.description.props.variant !== 'block' ? inlineDescriptionId : '' }`, @@ -199,10 +203,6 @@ export const Item = React.forwardRef( const wrapperProps = _PrivateItemWrapper ? menuItemProps : {} - const isTopLevelInteractive = () => - // @ts-ignore props.as may be defined, may not. - props.as === 'button' || props.as === 'a' || menuContext.anchorId !== undefined || role?.match(/menuitem/) - return ( (styles, sxProp)} {...containerProps} {...props}> diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index b8aae2a6c31..d110b8c839c 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -94,7 +94,7 @@ exports[`NavList renders a simple list 1`] = ` } .c3 button[data-component="ActionList.Item--DividerContainer"] { - padding: 0; + padding: 6px 8px; text-align: left; } @@ -181,7 +181,7 @@ exports[`NavList renders a simple list 1`] = ` } .c7 button[data-component="ActionList.Item--DividerContainer"] { - padding: 0; + padding: 6px 8px; text-align: left; } @@ -321,7 +321,6 @@ exports[`NavList renders a simple list 1`] = ` aria-labelledby="react-aria-2 " class="c4" href="/" - tabindex="0" > +
  • - +
  • - +
    • @@ -415,7 +417,6 @@ exports[`NavList renders with groups 1`] = ` -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: inherit; } .c2 { @@ -511,8 +512,8 @@ exports[`NavList renders with groups 1`] = ` } .c7 button[data-component="ActionList.Item--DividerContainer"] { - padding: 6px 8px; text-align: left; + padding: 0; } .c7:not(:first-of-type) { @@ -598,8 +599,8 @@ exports[`NavList renders with groups 1`] = ` } .c11 button[data-component="ActionList.Item--DividerContainer"] { - padding: 6px 8px; text-align: left; + padding: 0; } .c11:not(:first-of-type) { @@ -760,8 +761,9 @@ exports[`NavList renders with groups 1`] = ` aria-labelledby="react-aria-2 " class="c8" href="/getting-started" + tabindex="0" > - + @@ -805,8 +807,9 @@ exports[`NavList renders with groups 1`] = ` aria-labelledby="react-aria-5 " class="c8" href="/Avatar" + tabindex="0" > - + @@ -833,7 +836,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav padding-bottom: 8px; } -.c5 { +.c6 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -854,36 +857,50 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav line-height: 20px; } -.c7 { +.c8 { -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: inherit; } .c2 { list-style: none; } -.c6 { +.c5 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; display: flex; + padding-left: 0; + padding-right: 0; + padding-top: 0; + padding-bottom: 0; -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; } -.c10 { +.c7 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-box-flex: 1; + -webkit-flex-grow: 1; + -ms-flex-positive: 1; + flex-grow: 1; +} + +.c11 { padding: 0; margin: 0; display: block; } -.c8 { +.c9 { height: 20px; -webkit-flex-shrink: 0; -ms-flex-negative: 0; @@ -893,7 +910,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav font-weight: initial; } -.c9 { +.c10 { -webkit-transform: rotate(180deg); -ms-transform: rotate(180deg); transform: rotate(180deg); @@ -904,7 +921,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav padding-inline-start: 0; } -.c11 { +.c12 { position: relative; display: -webkit-box; display: -webkit-flex; @@ -938,15 +955,15 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav background-color: rgba(208,215,222,0.24); } -.c11[aria-disabled] { +.c12[aria-disabled] { cursor: not-allowed; } -.c11 [data-component="ActionList.Item--DividerContainer"] { +.c12 [data-component="ActionList.Item--DividerContainer"] { position: relative; } -.c11 [data-component="ActionList.Item--DividerContainer"]::before { +.c12 [data-component="ActionList.Item--DividerContainer"]::before { content: " "; display: block; position: absolute; @@ -957,32 +974,32 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav border-color: var(--divider-color,transparent); } -.c11 button[data-component="ActionList.Item--DividerContainer"] { - padding: 6px 8px; +.c12 button[data-component="ActionList.Item--DividerContainer"] { text-align: left; + padding: 0; } -.c11:not(:first-of-type) { +.c12:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } -[data-component="ActionList.Divider"] + .c11 { +[data-component="ActionList.Divider"] + .c12 { --divider-color: transparent !important; } -.c11:hover:not([aria-disabled]), -.c11:focus:not([aria-disabled]), -.c11[data-focus-visible-added]:not([aria-disabled]) { +.c12:hover:not([aria-disabled]), +.c12:focus:not([aria-disabled]), +.c12[data-focus-visible-added]:not([aria-disabled]) { --divider-color: transparent; } -.c11:hover:not([aria-disabled]) + .c3, -.c11:focus:not([aria-disabled]) + .c11, -.c11[data-focus-visible-added] + li { +.c12:hover:not([aria-disabled]) + .c3, +.c12:focus:not([aria-disabled]) + .c12, +.c12[data-focus-visible-added] + li { --divider-color: transparent; } -.c11::after { +.c12::after { position: absolute; top: calc(50% - 12px); left: -8px; @@ -993,12 +1010,6 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav border-radius: 6px; } -.c13:hover:not([aria-disabled]) + .c3, -.c13:focus:not([aria-disabled]) + .c13, -.c13[data-focus-visible-added] + li { - --divider-color: transparent; -} - .c14:hover:not([aria-disabled]) + .c3, .c14:focus:not([aria-disabled]) + .c14, .c14[data-focus-visible-added] + li { @@ -1035,6 +1046,12 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav --divider-color: transparent; } +.c20:hover:not([aria-disabled]) + .c3, +.c20:focus:not([aria-disabled]) + .c20, +.c20[data-focus-visible-added] + li { + --divider-color: transparent; +} + .c4 { position: relative; display: -webkit-box; @@ -1088,8 +1105,8 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav } .c4 button[data-component="ActionList.Item--DividerContainer"] { - padding: 6px 8px; text-align: left; + padding: 0; } .c4:not(:first-of-type) { @@ -1112,13 +1129,13 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav --divider-color: transparent; } -.c20:hover:not([aria-disabled]) + .c3, -.c20:focus:not([aria-disabled]) + .c20, -.c20[data-focus-visible-added] + li { +.c21:hover:not([aria-disabled]) + .c3, +.c21:focus:not([aria-disabled]) + .c21, +.c21[data-focus-visible-added] + li { --divider-color: transparent; } -.c12 { +.c13 { color: #0969da; -webkit-text-decoration: none; text-decoration: none; @@ -1140,12 +1157,12 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav font-weight: 400; } -.c12:hover { +.c13:hover { -webkit-text-decoration: underline; text-decoration: underline; } -.c12:is(button) { +.c13:is(button) { display: inline-block; padding: 0; font-size: inherit; @@ -1162,33 +1179,33 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav appearance: none; } -.c12:hover { +.c13:hover { color: inherit; -webkit-text-decoration: none; text-decoration: none; } @media (hover:hover) and (pointer:fine) { - .c11:hover:not([aria-disabled]) { + .c12:hover:not([aria-disabled]) { background-color: rgba(208,215,222,0.32); color: #1F2328; } - .c11:focus-visible, - .c11 > a:focus-visible { + .c12:focus-visible, + .c12 > a:focus-visible { outline: none; border: 2 solid; box-shadow: 0 0 0 2px #0969da; } - .c11:active:not([aria-disabled]) { + .c12:active:not([aria-disabled]) { background-color: rgba(208,215,222,0.48); color: #1F2328; } } @media (forced-colors:active) { - .c11:focus { + .c12:focus { outline: solid 1px transparent !important; } } @@ -1241,65 +1258,70 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav >
    - - Item - - - + - - - + + +
    @@ -1317,7 +1339,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t padding-bottom: 8px; } -.c5 { +.c6 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -1338,19 +1360,33 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t line-height: 20px; } -.c7 { +.c8 { -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; - font-weight: inherit; } .c2 { list-style: none; } -.c6 { +.c5 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + padding-left: 0; + padding-right: 0; + padding-top: 0; + padding-bottom: 0; + -webkit-box-flex: 1; + -webkit-flex-grow: 1; + -ms-flex-positive: 1; + flex-grow: 1; +} + +.c7 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -1361,13 +1397,13 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t flex-grow: 1; } -.c10 { +.c11 { padding: 0; margin: 0; display: none; } -.c8 { +.c9 { height: 20px; -webkit-flex-shrink: 0; -ms-flex-negative: 0; @@ -1377,7 +1413,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t font-weight: initial; } -.c9 { +.c10 { -webkit-transform: rotate(0deg); -ms-transform: rotate(0deg); transform: rotate(0deg); @@ -1388,7 +1424,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t padding-inline-start: 0; } -.c11 { +.c12 { position: relative; display: -webkit-box; display: -webkit-flex; @@ -1422,15 +1458,15 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t background-color: rgba(208,215,222,0.24); } -.c11[aria-disabled] { +.c12[aria-disabled] { cursor: not-allowed; } -.c11 [data-component="ActionList.Item--DividerContainer"] { +.c12 [data-component="ActionList.Item--DividerContainer"] { position: relative; } -.c11 [data-component="ActionList.Item--DividerContainer"]::before { +.c12 [data-component="ActionList.Item--DividerContainer"]::before { content: " "; display: block; position: absolute; @@ -1441,32 +1477,32 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t border-color: var(--divider-color,transparent); } -.c11 button[data-component="ActionList.Item--DividerContainer"] { - padding: 6px 8px; +.c12 button[data-component="ActionList.Item--DividerContainer"] { text-align: left; + padding: 0; } -.c11:not(:first-of-type) { +.c12:not(:first-of-type) { --divider-color: rgba(208,215,222,0.48); } -[data-component="ActionList.Divider"] + .c11 { +[data-component="ActionList.Divider"] + .c12 { --divider-color: transparent !important; } -.c11:hover:not([aria-disabled]), -.c11:focus:not([aria-disabled]), -.c11[data-focus-visible-added]:not([aria-disabled]) { +.c12:hover:not([aria-disabled]), +.c12:focus:not([aria-disabled]), +.c12[data-focus-visible-added]:not([aria-disabled]) { --divider-color: transparent; } -.c11:hover:not([aria-disabled]) + .c3, -.c11:focus:not([aria-disabled]) + .c11, -.c11[data-focus-visible-added] + li { +.c12:hover:not([aria-disabled]) + .c3, +.c12:focus:not([aria-disabled]) + .c12, +.c12[data-focus-visible-added] + li { --divider-color: transparent; } -.c11::after { +.c12::after { position: absolute; top: calc(50% - 12px); left: -8px; @@ -1477,12 +1513,6 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t border-radius: 6px; } -.c13:hover:not([aria-disabled]) + .c3, -.c13:focus:not([aria-disabled]) + .c13, -.c13[data-focus-visible-added] + li { - --divider-color: transparent; -} - .c14:hover:not([aria-disabled]) + .c3, .c14:focus:not([aria-disabled]) + .c14, .c14[data-focus-visible-added] + li { @@ -1525,6 +1555,12 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t --divider-color: transparent; } +.c21:hover:not([aria-disabled]) + .c3, +.c21:focus:not([aria-disabled]) + .c21, +.c21[data-focus-visible-added] + li { + --divider-color: transparent; +} + .c4 { position: relative; display: -webkit-box; @@ -1579,8 +1615,8 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t } .c4 button[data-component="ActionList.Item--DividerContainer"] { - padding: 6px 8px; text-align: left; + padding: 0; } .c4:not(:first-of-type) { @@ -1614,7 +1650,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t border-radius: 6px; } -.c12 { +.c13 { color: #0969da; -webkit-text-decoration: none; text-decoration: none; @@ -1636,12 +1672,12 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t font-weight: 400; } -.c12:hover { +.c13:hover { -webkit-text-decoration: underline; text-decoration: underline; } -.c12:is(button) { +.c13:is(button) { display: inline-block; padding: 0; font-size: inherit; @@ -1658,33 +1694,33 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t appearance: none; } -.c12:hover { +.c13:hover { color: inherit; -webkit-text-decoration: none; text-decoration: none; } @media (hover:hover) and (pointer:fine) { - .c11:hover:not([aria-disabled]) { + .c12:hover:not([aria-disabled]) { background-color: rgba(208,215,222,0.32); color: #1F2328; } - .c11:focus-visible, - .c11 > a:focus-visible { + .c12:focus-visible, + .c12 > a:focus-visible { outline: none; border: 2 solid; box-shadow: 0 0 0 2px #0969da; } - .c11:active:not([aria-disabled]) { + .c12:active:not([aria-disabled]) { background-color: rgba(208,215,222,0.48); color: #1F2328; } } @media (forced-colors:active) { - .c11:focus { + .c12:focus { outline: solid 1px transparent !important; } } @@ -1737,65 +1773,70 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t >
    - - Item - - - + - - - + + +
    From bf8602db21e12e2fcd2af9d7f0ba58296a5963fd Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Mon, 22 May 2023 16:22:02 -0400 Subject: [PATCH 28/35] Revert fontWeight config that differed from production docs. --- src/ActionList/Item.tsx | 1 + src/NavList/__snapshots__/NavList.test.tsx.snap | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 27e8ddbe46a..32a1a4835ae 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -252,6 +252,7 @@ export const Item = React.forwardRef( id={labelId} sx={{ flexGrow: slots.description && slots.description.props.variant !== 'block' ? 0 : 1, + fontWeight: slots.description && slots.description.props.variant !== 'block' ? 'bold' : 'normal', }} > {childrenWithoutSlots} diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index b2b76e5ee23..1ada63a5694 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -32,6 +32,7 @@ exports[`NavList renders a simple list 1`] = ` -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; + font-weight: 400; } .c1 { @@ -417,6 +418,7 @@ exports[`NavList renders with groups 1`] = ` -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; + font-weight: 400; } .c2 { @@ -862,6 +864,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; + font-weight: 400; } .c2 { @@ -1365,6 +1368,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t -webkit-flex-grow: 1; -ms-flex-positive: 1; flex-grow: 1; + font-weight: 400; } .c2 { From e12bc4047b528199a41a0ba4d84a1bf0beeb4136 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 23 May 2023 09:46:23 -0400 Subject: [PATCH 29/35] Pass selectionVariant prop illegally in UnderlineNav story to fix issue with Selection spans showing up. --- src/UnderlineNav2/UnderlineNav.tsx | 2 ++ src/UnderlineNav2/UnderlineNav2.features.stories.tsx | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/UnderlineNav2/UnderlineNav.tsx b/src/UnderlineNav2/UnderlineNav.tsx index e32dc57cedb..44bdf4c29d1 100644 --- a/src/UnderlineNav2/UnderlineNav.tsx +++ b/src/UnderlineNav2/UnderlineNav.tsx @@ -145,6 +145,7 @@ export const UnderlineNav = forwardRef( variant = 'default', loadingCounters = false, children, + ...props }: UnderlineNavProps, forwardedRef, ) => { @@ -369,6 +370,7 @@ export const UnderlineNav = forwardRef( ref={containerRef} id={disclosureWidgetId} sx={merge({display: isWidgetOpen ? 'block' : 'none'}, menuStyles)} + {...props} > {actions.map((action, index) => { const {children: actionElementChildren, ...actionElementProps} = action.props diff --git a/src/UnderlineNav2/UnderlineNav2.features.stories.tsx b/src/UnderlineNav2/UnderlineNav2.features.stories.tsx index 7c52532542a..d38796e3cdc 100644 --- a/src/UnderlineNav2/UnderlineNav2.features.stories.tsx +++ b/src/UnderlineNav2/UnderlineNav2.features.stories.tsx @@ -76,7 +76,11 @@ const items: {navigation: string; icon: React.FC; counter?: number | export const OverflowTemplate = ({initialSelectedIndex = 1}: {initialSelectedIndex?: number}) => { const [selectedIndex, setSelectedIndex] = React.useState(initialSelectedIndex) return ( - + {items.map((item, index) => ( Date: Wed, 31 May 2023 12:16:55 -0400 Subject: [PATCH 30/35] fix: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`'s `onSelect` handler by @gr2m (#3346) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`'s `onSelect` handler (#3163) * test: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`'s `onSelect` handler Failing test for https://github.com/primer/react/issues/3162 * fix: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`ߴs `onSelect` handler * add storybook example: Delayed Menu Close * update docs * docs: changeset * Update changelog --------- Co-authored-by: Siddharth Kshetrapal * Update generated/components.json --------- Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com> Co-authored-by: siddharthkp --- .changeset/brave-cherries-march.md | 5 +++ generated/components.json | 2 +- src/ActionList/ActionList.docs.json | 2 +- src/ActionList/Item.tsx | 26 ++++++++------ .../ActionMenu.examples.stories.tsx | 34 +++++++++++++++++++ src/ActionMenu/ActionMenu.test.tsx | 2 +- 6 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 .changeset/brave-cherries-march.md diff --git a/.changeset/brave-cherries-march.md b/.changeset/brave-cherries-march.md new file mode 100644 index 00000000000..d347b7a9a3b --- /dev/null +++ b/.changeset/brave-cherries-march.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +ActionMenu: Calling `event.preventDefault` inside `onSelect` of `ActionList.Item` will prevent the default behavior of closing the menu diff --git a/generated/components.json b/generated/components.json index 8964f1d7935..4df9e9e8dcc 100644 --- a/generated/components.json +++ b/generated/components.json @@ -163,7 +163,7 @@ "name": "onSelect", "type": "(event: React.MouseEvent | React.KeyboardEvent) => void", "defaultValue": "", - "description": "Callback that is called when the item is selected using either the mouse or keyboard." + "description": "Callback that is called when the item is selected using either the mouse or keyboard. `event.preventDefault()` will prevent a menu from closing when within an ``" }, { "name": "selected", diff --git a/src/ActionList/ActionList.docs.json b/src/ActionList/ActionList.docs.json index d74f3a0290a..18610f7f622 100644 --- a/src/ActionList/ActionList.docs.json +++ b/src/ActionList/ActionList.docs.json @@ -62,7 +62,7 @@ "name": "onSelect", "type": "(event: React.MouseEvent | React.KeyboardEvent) => void", "defaultValue": "", - "description": "Callback that is called when the item is selected using either the mouse or keyboard." + "description": "Callback that is called when the item is selected using either the mouse or keyboard. `event.preventDefault()` will prevent a menu from closing when within an ``" }, { "name": "selected", diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 32a1a4835ae..fa71cdfe36c 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -25,7 +25,7 @@ export const Item = React.forwardRef( disabled = false, selected = undefined, active = false, - onSelect, + onSelect: onSelectUser, sx: sxProp = defaultSxProp, id, role, @@ -46,6 +46,18 @@ export const Item = React.forwardRef( const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext) const selectionVariant = groupSelectionVariant ?? listSelectionVariant + const onSelect = React.useCallback( + ( + event: React.MouseEvent | React.KeyboardEvent, + // eslint-disable-next-line @typescript-eslint/ban-types + afterSelect?: Function, + ) => { + if (typeof onSelectUser === 'function') onSelectUser(event) + if (event.defaultPrevented) return + if (typeof afterSelect === 'function') afterSelect() + }, + [onSelectUser], + ) /** Infer item role based on the container */ let itemRole: ActionListItemProps['role'] @@ -163,11 +175,7 @@ export const Item = React.forwardRef( const clickHandler = React.useCallback( (event: React.MouseEvent) => { if (disabled) return - if (!event.defaultPrevented) { - if (typeof onSelect === 'function') onSelect(event) - // if this Item is inside a Menu, close the Menu - if (typeof afterSelect === 'function') afterSelect() - } + onSelect(event, afterSelect) }, [onSelect, disabled, afterSelect], ) @@ -175,10 +183,8 @@ export const Item = React.forwardRef( const keyPressHandler = React.useCallback( (event: React.KeyboardEvent) => { if (disabled) return - if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) { - if (typeof onSelect === 'function') onSelect(event) - // if this Item is inside a Menu, close the Menu - if (typeof afterSelect === 'function') afterSelect() + if ([' ', 'Enter'].includes(event.key)) { + onSelect(event, afterSelect) } }, [onSelect, disabled, afterSelect], diff --git a/src/ActionMenu/ActionMenu.examples.stories.tsx b/src/ActionMenu/ActionMenu.examples.stories.tsx index 88fec6fa0ec..308d2d2d26f 100644 --- a/src/ActionMenu/ActionMenu.examples.stories.tsx +++ b/src/ActionMenu/ActionMenu.examples.stories.tsx @@ -11,6 +11,8 @@ import { NumberIcon, CalendarIcon, XIcon, + CheckIcon, + CopyIcon, } from '@primer/octicons-react' export default { @@ -282,3 +284,35 @@ export const MultipleSections = () => { ) } + +export const DelayedMenuClose = () => { + const [open, setOpen] = React.useState(false) + const [copyLinkSuccess, setCopyLinkSuccess] = React.useState(false) + const onSelect = (event: React.MouseEvent | React.KeyboardEvent) => { + event.preventDefault() + + setCopyLinkSuccess(true) + setTimeout(() => { + setOpen(false) + setCopyLinkSuccess(false) + }, 700) + } + + return ( + <> +

    Delayed Menu Close

    + + + Anchor + + + + {copyLinkSuccess ? : } + {copyLinkSuccess ? 'Copied!' : 'Copy link'} + + + + + + ) +} diff --git a/src/ActionMenu/ActionMenu.test.tsx b/src/ActionMenu/ActionMenu.test.tsx index 99c1876e983..1cb488430bc 100644 --- a/src/ActionMenu/ActionMenu.test.tsx +++ b/src/ActionMenu/ActionMenu.test.tsx @@ -22,7 +22,7 @@ function Example(): JSX.Element { Copy link Edit file - event.preventDefault()}> + event.preventDefault()}> Delete file From afd0bec9b601cd0f4140b211ef41090c9839a62a Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 6 Jun 2023 09:32:34 -0400 Subject: [PATCH 31/35] Check for tabIndex value in isTopLevelInteractive. --- src/ActionList/Item.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index fa71cdfe36c..0d028f02cfc 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -30,6 +30,8 @@ export const Item = React.forwardRef( id, role, _PrivateItemWrapper, + // @ts-ignore tabIndex is sometimes passed as a prop in dotcom. + tabIndex, ...props }, forwardedRef, @@ -91,7 +93,8 @@ export const Item = React.forwardRef( // @ts-ignore props.as may be defined, may not. props.as === 'a' || menuContext.anchorId !== undefined || - role?.match(/menuitem/) + role?.match(/menuitem/) || + tabIndex !== undefined const styles = { position: 'relative', From 9903c8d9f2373c374eadf763ec99e3fb46c92ba6 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 6 Jun 2023 09:42:01 -0400 Subject: [PATCH 32/35] Reference styles in updated menuProps. --- src/ActionList/Item.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 0d028f02cfc..69bb4469320 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -220,8 +220,8 @@ export const Item = React.forwardRef( : { sx: { display: 'flex', - paddingX: isTopLevelInteractive() ? 0 : 2, - paddingY: isTopLevelInteractive() ? 0 : '6px', + paddingX: styles.paddingX, + paddingY: styles.paddingY, flexGrow: 1, }, } From fae7d6ce6288f506e98f3f02fca436063cb942c9 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 6 Jun 2023 10:12:05 -0400 Subject: [PATCH 33/35] Updated snapshots. --- src/NavList/__snapshots__/NavList.test.tsx.snap | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index e049249e370..455b8c80139 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -876,10 +876,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav display: -webkit-flex; display: -ms-flexbox; display: flex; - padding-left: 0; - padding-right: 0; - padding-top: 0; - padding-bottom: 0; + padding-left: 8px; + padding-right: 8px; + padding-top: 6px; + padding-bottom: 6px; -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1; @@ -1380,10 +1380,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t display: -webkit-flex; display: -ms-flexbox; display: flex; - padding-left: 0; - padding-right: 0; - padding-top: 0; - padding-bottom: 0; + padding-left: 8px; + padding-right: 8px; + padding-top: 6px; + padding-bottom: 6px; -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1; From 3f416be3eb61fceb5eb9c5170bfa203e7825f2b0 Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 6 Jun 2023 11:47:54 -0400 Subject: [PATCH 34/35] Fix padding setting instead of using values from styles, which are inverted. --- src/ActionList/Item.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 69bb4469320..c0b3eae74b8 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -220,8 +220,8 @@ export const Item = React.forwardRef( : { sx: { display: 'flex', - paddingX: styles.paddingX, - paddingY: styles.paddingY, + paddingX: isTopLevelInteractive() ? 0 : 2, + paddingY: isTopLevelInteractive() ? 0 : '6px', // custom value off the scale flexGrow: 1, }, } From 2725253bb84294c5d713bee2157247ff8deeed9b Mon Sep 17 00:00:00 2001 From: Jeremy Neal Date: Tue, 6 Jun 2023 13:03:14 -0400 Subject: [PATCH 35/35] Update snapshots. --- src/NavList/__snapshots__/NavList.test.tsx.snap | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index 455b8c80139..e049249e370 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -876,10 +876,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav display: -webkit-flex; display: -ms-flexbox; display: flex; - padding-left: 8px; - padding-right: 8px; - padding-top: 6px; - padding-bottom: 6px; + padding-left: 0; + padding-right: 0; + padding-top: 0; + padding-bottom: 0; -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1; @@ -1380,10 +1380,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t display: -webkit-flex; display: -ms-flexbox; display: flex; - padding-left: 8px; - padding-right: 8px; - padding-top: 6px; - padding-bottom: 6px; + padding-left: 0; + padding-right: 0; + padding-top: 0; + padding-bottom: 0; -webkit-box-flex: 1; -webkit-flex-grow: 1; -ms-flex-positive: 1;