diff --git a/.changeset/actionmenu-remove-focus-trap.md b/.changeset/actionmenu-remove-focus-trap.md new file mode 100644 index 00000000000..92d765283b0 --- /dev/null +++ b/.changeset/actionmenu-remove-focus-trap.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +ActionMenu: Remove focus trap to enable Tab and Shift+Tab behavior and fix initial focus on click diff --git a/.changeset/fair-tips-travel.md b/.changeset/fair-tips-travel.md new file mode 100644 index 00000000000..1dd8f13d944 --- /dev/null +++ b/.changeset/fair-tips-travel.md @@ -0,0 +1,19 @@ +--- +"@primer/react": minor +--- + +* Updated the `position` prop in `PageLayout.Pane` to use the new responsive prop API introduced in https://github.com/primer/react/pull/2174. +* Deprecated the `positionWhenNarrow` prop in favor of the new responsive prop API + +**Before** + +``` +position="start" +positionWhenNarrow="end" +``` + +**After** + +``` +position={{regular: 'start', narrow: 'end'}} +``` diff --git a/.changeset/lucky-pianos-yell.md b/.changeset/lucky-pianos-yell.md new file mode 100644 index 00000000000..c4d16e77efb --- /dev/null +++ b/.changeset/lucky-pianos-yell.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Set ConfirmationDialog initial focus based on the confirmationButtonVariant. When `danger` autoFocus the cancel button, otherwise autoFocus the confirmation button diff --git a/.changeset/spotty-parents-cheat.md b/.changeset/spotty-parents-cheat.md new file mode 100644 index 00000000000..54544ed8632 --- /dev/null +++ b/.changeset/spotty-parents-cheat.md @@ -0,0 +1,19 @@ +--- +"@primer/react": minor +--- + +* Updated the `divider` prop in `PageLayout.Header`, `PageLayout.Pane`, and `PageLayout.Footer` to use the new responsive prop API introduced in https://github.com/primer/react/pull/2174. +* Deprecated the `dividerWhenNarrow` prop in favor of the new responsive prop API + +**Before** + +``` +divider="line" +dividerWhenNarrow="filled" +``` + +**After** + +``` +divider={{regular: 'line', narrow: 'filled'}} +``` diff --git a/docs/content/NavList.mdx b/docs/content/NavList.mdx index b016fa8108d..4cc7d78dc2b 100644 --- a/docs/content/NavList.mdx +++ b/docs/content/NavList.mdx @@ -210,7 +210,10 @@ import {NavList} from '@primer/react' function NavItem({href, children}) { const router = useRouter() - const isCurrent = router.asPath === href + const isCurrent = + typeof href === 'string' + ? router.asPath === href + : router.pathname === href.pathname; return ( {children} @@ -224,6 +227,14 @@ function App() { Dashboard Pull requests Issues + + Summary + ) } diff --git a/docs/content/PageLayout.mdx b/docs/content/PageLayout.mdx index 590ca1fd420..8511c8951ec 100644 --- a/docs/content/PageLayout.mdx +++ b/docs/content/PageLayout.mdx @@ -48,7 +48,8 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo - + {/* You can change the divider based on the viewport width */} + @@ -199,25 +200,39 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo @@ -248,10 +263,10 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo name="hidden" type={`| boolean | { - narrow?: boolean - regular?: boolean - wide?: boolean -}`} + narrow?: boolean + regular?: boolean + wide?: boolean + }`} defaultValue="false" description="Whether the content is hidden." /> @@ -264,15 +279,28 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo @@ -332,25 +374,39 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index 329207d046f..a88513c7e1e 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -3,17 +3,19 @@ import {useSSRSafeId} from '@react-aria/ssr' import {TriangleDownIcon} from '@primer/octicons-react' import {AnchoredOverlay, AnchoredOverlayProps} from './AnchoredOverlay' import {OverlayProps} from './Overlay' -import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuInitialFocus, useMnemonics} from './hooks' +import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuKeyboardNavigation} from './hooks' import {Divider} from './ActionList/Divider' import {ActionListContainerContext} from './ActionList/ActionListContainerContext' import {Button, ButtonProps} from './Button' import {MandateProps} from './utils/types' import {SxProp, merge} from './sx' -type MenuContextProps = Pick< +export type MenuContextProps = Pick< AnchoredOverlayProps, - 'anchorRef' | 'renderAnchor' | 'open' | 'onOpen' | 'onClose' | 'anchorId' -> + 'anchorRef' | 'renderAnchor' | 'open' | 'onOpen' | 'anchorId' +> & { + onClose?: (gesture: 'anchor-click' | 'click-outside' | 'escape' | 'tab') => void +} const MenuContext = React.createContext({renderAnchor: null, open: false}) export type ActionMenuProps = { @@ -111,8 +113,7 @@ const Overlay: React.FC> = ({children, > const containerRef = React.createRef() - const {openWithFocus} = useMenuInitialFocus(open, onOpen, containerRef) - useMnemonics(open, containerRef) + useMenuKeyboardNavigation(open, onClose, containerRef, anchorRef) return ( > = ({children, renderAnchor={renderAnchor} anchorId={anchorId} open={open} - onOpen={openWithFocus} + onOpen={onOpen} onClose={onClose} align={align} overlayProps={overlayProps} focusZoneSettings={{focusOutBehavior: 'wrap'}} + focusTrapSettings={{disabled: true}} >
{ onClose('confirm') }, [onClose]) + const isConfirmationDangerous = confirmButtonType === 'danger' const cancelButton: DialogButtonProps = { content: cancelButtonContent, - onClick: onCancelButtonClick + onClick: onCancelButtonClick, + autoFocus: isConfirmationDangerous } const confirmButton: DialogButtonProps = { content: confirmButtonContent, buttonType: confirmButtonType, onClick: onConfirmButtonClick, - autoFocus: true + autoFocus: !isConfirmationDangerous } const footerButtons = [cancelButton, confirmButton] return ( diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index f42f9778991..5825f0a0510 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -1,6 +1,6 @@ import React from 'react' import {Box} from '..' -import {ResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' +import {isResponsiveValue, ResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' import {BetterSystemStyleObject, merge, SxProp} from '../sx' const REGION_ORDER = { @@ -79,8 +79,7 @@ Root.displayName = 'PageLayout' // Divider (internal) type DividerProps = { - variant?: 'none' | 'line' - variantWhenNarrow?: 'inherit' | 'none' | 'line' | 'filled' + variant?: 'none' | 'line' | 'filled' | ResponsiveValue<'none' | 'line' | 'filled'> } & SxProp const horizontalDividerVariants = { @@ -111,12 +110,9 @@ function negateSpacingValue(value: number | null | Array) { return value === null ? null : -value } -const HorizontalDivider: React.FC> = ({ - variant = 'none', - variantWhenNarrow = 'inherit', - sx = {} -}) => { +const HorizontalDivider: React.FC> = ({variant = 'none', sx = {}}) => { const {padding} = React.useContext(PageLayoutContext) + const responsiveVariant = useResponsiveValue(variant, 'none') return ( > = ({ { // Stretch divider to viewport edges on narrow screens marginX: negateSpacingValue(SPACING_MAP[padding]), - ...horizontalDividerVariants[variantWhenNarrow === 'inherit' ? variant : variantWhenNarrow], + ...horizontalDividerVariants[responsiveVariant], [`@media screen and (min-width: ${theme.breakpoints[1]})`]: { - marginX: '0 !important', - ...horizontalDividerVariants[variant] + marginX: '0 !important' } }, sx @@ -157,26 +152,17 @@ const verticalDividerVariants = { } } -const VerticalDivider: React.FC> = ({ - variant = 'none', - variantWhenNarrow = 'inherit', - sx = {} -}) => { +const VerticalDivider: React.FC> = ({variant = 'none', sx = {}}) => { + const responsiveVariant = useResponsiveValue(variant, 'none') return ( - merge( - { - height: '100%', - ...verticalDividerVariants[variantWhenNarrow === 'inherit' ? variant : variantWhenNarrow], - [`@media screen and (min-width: ${theme.breakpoints[1]})`]: { - ...verticalDividerVariants[variant] - } - }, - sx - ) - } + sx={merge( + { + height: '100%', + ...verticalDividerVariants[responsiveVariant] + }, + sx + )} /> ) } @@ -186,7 +172,21 @@ const VerticalDivider: React.FC> = ({ export type PageLayoutHeaderProps = { padding?: keyof typeof SPACING_MAP - divider?: 'none' | 'line' + divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> + /** + * @deprecated Use the `divider` prop with a responsive value instead. + * + * Before: + * ``` + * divider="line" + * dividerWhenNarrow="filled" + * ``` + * + * After: + * ``` + * divider={{regular: 'line', narrow: 'filled'}} + * ``` + */ dividerWhenNarrow?: 'inherit' | 'none' | 'line' | 'filled' hidden?: boolean | ResponsiveValue } & SxProp @@ -199,6 +199,13 @@ const Header: React.FC> = ({ children, sx = {} }) => { + // Combine divider and dividerWhenNarrow for backwards compatibility + const dividerProp = + !isResponsiveValue(divider) && dividerWhenNarrow !== 'inherit' + ? {regular: divider, narrow: dividerWhenNarrow} + : divider + + const dividerVariant = useResponsiveValue(dividerProp, 'none') const isHidden = useResponsiveValue(hidden, false) const {rowGap} = React.useContext(PageLayoutContext) return ( @@ -215,11 +222,7 @@ const Header: React.FC> = ({ )} > {children} - + ) } @@ -282,11 +285,39 @@ Content.displayName = 'PageLayout.Content' // PageLayout.Pane export type PageLayoutPaneProps = { - position?: keyof typeof panePositions + position?: keyof typeof panePositions | ResponsiveValue + /** + * @deprecated Use the `position` prop with a responsive value instead. + * + * Before: + * ``` + * position="start" + * positionWhenNarrow="end" + * ``` + * + * After: + * ``` + * position={{regular: 'start', narrow: 'end'}} + * ``` + */ positionWhenNarrow?: 'inherit' | keyof typeof panePositions width?: keyof typeof paneWidths padding?: keyof typeof SPACING_MAP - divider?: 'none' | 'line' + divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> + /** + * @deprecated Use the `divider` prop with a responsive value instead. + * + * Before: + * ``` + * divider="line" + * dividerWhenNarrow="filled" + * ``` + * + * After: + * ``` + * divider={{regular: 'line', narrow: 'filled'}} + * ``` + */ dividerWhenNarrow?: 'inherit' | 'none' | 'line' | 'filled' hidden?: boolean | ResponsiveValue } & SxProp @@ -313,10 +344,23 @@ const Pane: React.FC> = ({ children, sx = {} }) => { + // Combine position and positionWhenNarrow for backwards compatibility + const positionProp = + !isResponsiveValue(position) && positionWhenNarrow !== 'inherit' + ? {regular: position, narrow: positionWhenNarrow} + : position + + const responsivePosition = useResponsiveValue(positionProp, 'end') + + // Combine divider and dividerWhenNarrow for backwards compatibility + const dividerProp = + !isResponsiveValue(divider) && dividerWhenNarrow !== 'inherit' + ? {regular: divider, narrow: dividerWhenNarrow} + : divider + + const dividerVariant = useResponsiveValue(dividerProp, 'none') const isHidden = useResponsiveValue(hidden, false) const {rowGap, columnGap} = React.useContext(PageLayoutContext) - const computedPositionWhenNarrow = positionWhenNarrow === 'inherit' ? position : positionWhenNarrow - const computedDividerWhenNarrow = dividerWhenNarrow === 'inherit' ? divider : dividerWhenNarrow return ( > = ({ sx={(theme: any) => merge( { - order: panePositions[computedPositionWhenNarrow], + order: panePositions[responsivePosition], display: isHidden ? 'none' : 'flex', - flexDirection: computedPositionWhenNarrow === 'end' ? 'column' : 'column-reverse', + flexDirection: responsivePosition === 'end' ? 'column' : 'column-reverse', width: '100%', marginX: 0, - [computedPositionWhenNarrow === 'end' ? 'marginTop' : 'marginBottom']: SPACING_MAP[rowGap], + [responsivePosition === 'end' ? 'marginTop' : 'marginBottom']: SPACING_MAP[rowGap], [`@media screen and (min-width: ${theme.breakpoints[1]})`]: { width: 'auto', - [position === 'end' ? 'marginLeft' : 'marginRight']: SPACING_MAP[columnGap], + [responsivePosition === 'end' ? 'marginLeft' : 'marginRight']: SPACING_MAP[columnGap], marginY: `0 !important`, - flexDirection: position === 'end' ? 'row' : 'row-reverse', - order: panePositions[position] + flexDirection: responsivePosition === 'end' ? 'row' : 'row-reverse', + order: panePositions[responsivePosition] } }, sx @@ -344,14 +388,12 @@ const Pane: React.FC> = ({ > {/* Show a horizontal divider when viewport is narrow. Otherwise, show a vertical divider. */} {children} @@ -366,7 +408,21 @@ Pane.displayName = 'PageLayout.Pane' export type PageLayoutFooterProps = { padding?: keyof typeof SPACING_MAP - divider?: 'none' | 'line' + divider?: 'none' | 'line' | ResponsiveValue<'none' | 'line', 'none' | 'line' | 'filled'> + /** + * @deprecated Use the `divider` prop with a responsive value instead. + * + * Before: + * ``` + * divider="line" + * dividerWhenNarrow="filled" + * ``` + * + * After: + * ``` + * divider={{regular: 'line', narrow: 'filled'}} + * ``` + */ dividerWhenNarrow?: 'inherit' | 'none' | 'line' | 'filled' hidden?: boolean | ResponsiveValue } & SxProp @@ -379,6 +435,13 @@ const Footer: React.FC> = ({ children, sx = {} }) => { + // Combine divider and dividerWhenNarrow for backwards compatibility + const dividerProp = + !isResponsiveValue(divider) && dividerWhenNarrow !== 'inherit' + ? {regular: divider, narrow: dividerWhenNarrow} + : divider + + const dividerVariant = useResponsiveValue(dividerProp, 'none') const isHidden = useResponsiveValue(hidden, false) const {rowGap} = React.useContext(PageLayoutContext) return ( @@ -394,11 +457,7 @@ const Footer: React.FC> = ({ sx )} > - + {children} ) diff --git a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap index 9f80e63af40..5f9e4c680bb 100644 --- a/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap +++ b/src/PageLayout/__snapshots__/PageLayout.test.tsx.snap @@ -121,7 +121,6 @@ exports[`PageLayout renders condensed layout 1`] = ` .c4 { margin-left: 0 !important; margin-right: 0 !important; - display: none; } } @@ -144,13 +143,6 @@ exports[`PageLayout renders condensed layout 1`] = ` .c8 { margin-left: 0 !important; margin-right: 0 !important; - display: none; - } -} - -@media screen and (min-width:768px) { - .c9 { - display: none; } } @@ -335,7 +327,6 @@ exports[`PageLayout renders default layout 1`] = ` .c4 { margin-left: 0 !important; margin-right: 0 !important; - display: none; } } @@ -372,7 +363,6 @@ exports[`PageLayout renders default layout 1`] = ` .c8 { margin-left: 0 !important; margin-right: 0 !important; - display: none; } } @@ -384,12 +374,6 @@ exports[`PageLayout renders default layout 1`] = ` } } -@media screen and (min-width:768px) { - .c9 { - display: none; - } -} - @media screen and (min-width:1012px) { .c9 { margin-right: 24px; @@ -536,25 +520,42 @@ exports[`PageLayout renders pane in different position when narrow 1`] = ` padding: 0; } -.c11 { +.c7 { + -webkit-order: 3; + -ms-flex-order: 3; + order: 3; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; + width: 100%; + margin-left: 0; + margin-right: 0; + margin-top: 16px; +} + +.c8 { margin-left: -16px; margin-right: -16px; display: none; margin-bottom: 16px; } -.c8 { +.c9 { height: 100%; display: none; margin-right: 16px; } -.c9 { +.c10 { width: 100%; padding: 0; } -.c10 { +.c11 { -webkit-order: 4; -ms-flex-order: 4; order: 4; @@ -562,23 +563,6 @@ exports[`PageLayout renders pane in different position when narrow 1`] = ` margin-top: 16px; } -.c7 { - -webkit-order: 1; - -ms-flex-order: 1; - order: 1; - display: -webkit-box; - display: -webkit-flex; - display: -ms-flexbox; - display: flex; - -webkit-flex-direction: column-reverse; - -ms-flex-direction: column-reverse; - flex-direction: column-reverse; - width: 100%; - margin-left: 0; - margin-right: 0; - margin-bottom: 16px; -} - @media screen and (min-width:1012px) { .c0 { padding: 24px; @@ -595,7 +579,6 @@ exports[`PageLayout renders pane in different position when narrow 1`] = ` .c4 { margin-left: 0 !important; margin-right: 0 !important; - display: none; } } @@ -608,69 +591,62 @@ exports[`PageLayout renders pane in different position when narrow 1`] = ` } @media screen and (min-width:768px) { - .c11 { - margin-left: 0 !important; - margin-right: 0 !important; - display: none; + .c7 { + width: auto; + margin-left: 16px; + margin-top: 0 !important; + margin-bottom: 0 !important; + -webkit-flex-direction: row; + -ms-flex-direction: row; + flex-direction: row; + -webkit-order: 3; + -ms-flex-order: 3; + order: 3; } } @media screen and (min-width:1012px) { - .c11 { - margin-left: -24px; - margin-right: -24px; - margin-bottom: 24px; + .c7 { + margin-top: 24px; } } @media screen and (min-width:768px) { .c8 { - display: none; + margin-left: 0 !important; + margin-right: 0 !important; } } @media screen and (min-width:1012px) { .c8 { - margin-right: 24px; - } -} - -@media screen and (min-width:768px) { - .c9 { - width: 256px; + margin-left: -24px; + margin-right: -24px; + margin-bottom: 24px; } } @media screen and (min-width:1012px) { .c9 { - width: 296px; + margin-right: 24px; } } -@media screen and (min-width:1012px) { +@media screen and (min-width:768px) { .c10 { - margin-top: 24px; + width: 256px; } } -@media screen and (min-width:768px) { - .c7 { - width: auto; - margin-left: 16px; - margin-top: 0 !important; - margin-bottom: 0 !important; - -webkit-flex-direction: row; - -ms-flex-direction: row; - flex-direction: row; - -webkit-order: 3; - -ms-flex-order: 3; - order: 3; +@media screen and (min-width:1012px) { + .c10 { + width: 296px; } } @media screen and (min-width:1012px) { - .c7 { - margin-bottom: 24px; + .c11 { + margin-top: 24px; } } @@ -705,23 +681,23 @@ exports[`PageLayout renders pane in different position when narrow 1`] = `