diff --git a/.changeset/busy-planes-leave.md b/.changeset/busy-planes-leave.md new file mode 100644 index 00000000000..cb85913ad4f --- /dev/null +++ b/.changeset/busy-planes-leave.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Removes sx prop from UnderlineNav components diff --git a/packages/react/src/UnderlineNav/UnderlineNav.docs.json b/packages/react/src/UnderlineNav/UnderlineNav.docs.json index 65b5a0da13f..aed639ccff1 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.docs.json +++ b/packages/react/src/UnderlineNav/UnderlineNav.docs.json @@ -55,11 +55,6 @@ "type": "'inset' | 'flush'", "defaultValue": "'inset'", "description": "`inset` children are offset horizontally from container edges. `flush` children are flush horizontally with container edges" - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ], "subcomponents": [ @@ -100,11 +95,6 @@ "name": "as", "type": "React.ElementType", "defaultValue": "\"a\"" - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ], "passthrough": { diff --git a/packages/react/src/UnderlineNav/UnderlineNav.module.css b/packages/react/src/UnderlineNav/UnderlineNav.module.css new file mode 100644 index 00000000000..79145f723f1 --- /dev/null +++ b/packages/react/src/UnderlineNav/UnderlineNav.module.css @@ -0,0 +1,77 @@ +.Divider { + display: inline-block; + border-left: var(--borderWidth-thin) solid var(--borderColor-muted); + width: var(--borderWidth-thin); + margin-right: var(--base-size-4); + height: 24px; /* The height of the divider - reference from Figma */ +} + +.MoreButton { + /* Set margin 0 here because safari puts extra margin around the button, + rest is to reset style to make it look like a list element */ + margin: 0; + border: 0; + font-weight: var(--base-text-weight-normal); + box-shadow: none; + padding-block: var(--base-size-4); + padding-inline: var(--base-size-8); +} + +.MoreButton, +.MoreButton:hover, +.MoreButton[aria-expanded='true'] { + background: transparent; +} + +.MoreButton[data-component='trailingVisual'] { + margin-left: 0; +} + +.MoreMenuListItem { + display: flex; + align-items: center; + /* Hard-coded height is needed to make sure we don't have a layout shift when the more button is the only item in the nav. */ + height: 45px; +} + +.MenuContainer { + position: absolute; + z-index: 1; + top: 90%; + box-shadow: + /* TODO: replace custom shadow with Primer Primitives variable */ + /* stylelint-disable-next-line primer/box-shadow */ + rgba(0, 0, 0, 0.12) 0 1px 3px, + rgba(0, 0, 0, 0.24) 0 1px 2px; + border-radius: var(--borderRadius-large); + background-color: var(--bgColor-default); + list-style: none; + min-width: 192px; /* baseMenuMinWidth */ + max-width: 640px; + right: 0; + display: block; +} + +.MenuContainer[data-is-widget-open='false'] { + display: none; +} + +.MenuItem { + text-decoration: none; +} + +.MenuItem > span:first-child { + display: none; +} + +.MenuItemContent { + display: flex; + align-items: center; + justify-content: space-between; +} + +.NavListItem { + display: flex; + flex-direction: column; + align-items: center; +} diff --git a/packages/react/src/UnderlineNav/UnderlineNav.test.tsx b/packages/react/src/UnderlineNav/UnderlineNav.test.tsx index e205e043c99..b4ad1deb033 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.test.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.test.tsx @@ -14,7 +14,7 @@ import { } from '@primer/octicons-react' import {UnderlineNav} from '.' -import {baseMenuMinWidth, menuStyles} from './styles' +import {BASE_MENU_MIN_WIDTH, getLeftAnchoredPosition} from './UnderlineNav' const ResponsiveUnderlineNav = ({ selectedItemText = 'Code', @@ -166,20 +166,20 @@ describe('UnderlineNav', () => { spy.mockRestore() }) - it(`menuStyles should set the menu position, if the container size is below ${baseMenuMinWidth} px`, () => { + it(`should set the menu position using getLeftAnchoredPosition, if the container size is below ${BASE_MENU_MIN_WIDTH} px`, () => { // GIVEN // Mock the refs. const containerRef = document.createElement('div') const listRef = document.createElement('div') // Set the clientWidth on the mock element - Object.defineProperty(listRef, 'clientWidth', {value: baseMenuMinWidth - 1}) + Object.defineProperty(listRef, 'clientWidth', {value: BASE_MENU_MIN_WIDTH - 1}) // WHEN - const results = menuStyles(containerRef, listRef) + const results = getLeftAnchoredPosition(containerRef, listRef) // THEN - // We are expecting a left value back, that way we know the `getAnchoredPosition` ran. - expect(results).toEqual(expect.objectContaining({left: 0})) + // We are expecting a left value back, that way we know the `getLeftAnchoredPosition` ran. + expect(results).toEqual(0) }) it('should support icons passed in as an element', () => { diff --git a/packages/react/src/UnderlineNav/UnderlineNav.tsx b/packages/react/src/UnderlineNav/UnderlineNav.tsx index c0f44fe244c..8b64fc8eea1 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.tsx @@ -1,33 +1,28 @@ import type {MutableRefObject, RefObject} from 'react' import React, {useRef, forwardRef, useCallback, useState, useEffect} from 'react' -import Box from '../Box' -import type {SxProp} from '../sx' -import sx from '../sx' +import {getAnchoredPosition} from '@primer/behaviors' import {UnderlineNavContext} from './UnderlineNavContext' import type {ResizeObserverEntry} from '../hooks/useResizeObserver' import {useResizeObserver} from '../hooks/useResizeObserver' import {useTheme} from '../ThemeProvider' import type {ChildWidthArray, ResponsiveProps, ChildSize} from './types' import VisuallyHidden from '../_VisuallyHidden' -import {moreBtnStyles, getDividerStyle, menuStyles, menuItemStyles, baseMenuStyles, baseMenuMinWidth} from './styles' import {UnderlineItemList, UnderlineWrapper, LoadingCounter, GAP} from '../internal/components/UnderlineTabbedInterface' -import styled from 'styled-components' import {Button} from '../Button' import {TriangleDownIcon} from '@primer/octicons-react' import {useOnEscapePress} from '../hooks/useOnEscapePress' import {useOnOutsideClick} from '../hooks/useOnOutsideClick' import {useId} from '../hooks/useId' import {ActionList} from '../ActionList' -import {defaultSxProp} from '../utils/defaultSxProp' import CounterLabel from '../CounterLabel' import {invariant} from '../utils/invariant' +import classes from './UnderlineNav.module.css' export type UnderlineNavProps = { children: React.ReactNode 'aria-label'?: React.AriaAttributes['aria-label'] as?: React.ElementType className?: string - sx?: SxProp['sx'] /** * loading state for all counters. It displays loading animation for individual counters (UnderlineNav.Item) until all are resolved. It is needed to prevent multiple layout shift. */ @@ -42,19 +37,7 @@ export type UnderlineNavProps = { // When page is loaded, we don't have ref for the more button as it is not on the DOM yet. // However, we need to calculate number of possible items when the more button present as well. So using the width of the more button as a constant. export const MORE_BTN_WIDTH = 86 -// The height is needed to make sure we don't have a layout shift when the more button is the only item in the nav. -const MORE_BTN_HEIGHT = 45 - -// Needed this because passing a ref using HTMLULListElement to `Box` causes a type error -export const NavigationList = styled.ul` - ${sx}; -` - -export const MoreMenuListItem = styled.li` - display: flex; - align-items: center; - height: ${MORE_BTN_HEIGHT}px; -` +export const BASE_MENU_MIN_WIDTH = 192 const overflowEffect = ( navWidth: number, @@ -140,12 +123,25 @@ const calculatePossibleItems = (childWidthArray: ChildWidthArray, navWidth: numb return breakpoint } +/** + * + * @param containerRef The Menu List Container Reference. + * @param listRef The Underline Nav Container Reference. + * @description This calculates the position of the menu + * + * Exported because it's used in unit tests. + */ +export const getLeftAnchoredPosition = (containerRef: Element | null, listRef: Element | null): number => { + return containerRef && listRef + ? getAnchoredPosition(containerRef, listRef, {align: 'start', side: 'outside-bottom'}).left + : 0 +} + export const UnderlineNav = forwardRef( ( { as = 'nav', 'aria-label': ariaLabel, - sx: sxProp = defaultSxProp, loadingCounters = false, variant = 'inset', className, @@ -316,28 +312,21 @@ export const UnderlineNav = forwardRef( }} > {ariaLabel && {`${ariaLabel} navigation`}} - + {listItems} {menuItems.length > 0 && ( - - {!onlyMenuVisible && } +
  • + {!onlyMenuVisible &&
    } = baseMenuMinWidth - ? baseMenuStyles - : menuStyles(containerRef.current, listRef.current) - } - style={{display: isWidgetOpen ? 'block' : 'none'}} + className={classes.MenuContainer} + data-is-widget-open={isWidgetOpen} + style={{ + left: + listRef.current?.clientWidth && listRef.current.clientWidth >= BASE_MENU_MIN_WIDTH + ? undefined + : getLeftAnchoredPosition(containerRef.current, listRef.current), + }} > {menuItems.map((menuItem, index) => { const { @@ -385,7 +376,7 @@ export const UnderlineNav = forwardRef( return ( | React.KeyboardEvent, ) => { @@ -398,23 +389,23 @@ export const UnderlineNav = forwardRef( }} {...menuItemProps} > - + {menuItemChildren} {loadingCounters ? ( ) : ( counter !== undefined && ( - + {counter} - + ) )} - + ) })} - +
    • )}     diff --git a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx index f63c38c270b..40da1b652e7 100644 --- a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx @@ -1,13 +1,11 @@ import type {MutableRefObject, RefObject} from 'react' -import React, {forwardRef, useRef, useContext} from 'react' -import Box from '../Box' -import type {SxProp} from '../sx' +import React, {useRef, useContext} from 'react' import type {IconProps} from '@primer/octicons-react' -import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' +import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic' import {UnderlineNavContext} from './UnderlineNavContext' import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' -import {defaultSxProp} from '../utils/defaultSxProp' import {UnderlineItem} from '../internal/components/UnderlineTabbedInterface' +import classes from './UnderlineNav.module.css' // adopted from React.AnchorHTMLAttributes export type LinkProps = { @@ -22,115 +20,115 @@ export type LinkProps = { referrerPolicy?: React.AnchorHTMLAttributes['referrerPolicy'] } -export type UnderlineNavItemProps = { - /** - * Primary content for an UnderlineNav - */ - children?: React.ReactNode - /** - * Callback that will trigger both on click selection and keyboard selection. - */ - onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void - /** - * Is `UnderlineNav.Item` current page? - */ - 'aria-current'?: 'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false' | boolean - /** - * Icon before the text - */ - icon?: React.FunctionComponent | React.ReactElement - /** - * Renders `UnderlineNav.Item` as given component i.e. react-router's Link - **/ - as?: React.ElementType | 'a' - /** - * Counter - */ - counter?: number | string -} & SxProp & - LinkProps +export type UnderlineNavItemProps = PolymorphicProps< + As, + 'a', + { + /** + * Primary content for an UnderlineNav + */ + children?: React.ReactNode + /** + * Callback that will trigger both on click selection and keyboard selection. + */ + onSelect?: (event: React.MouseEvent | React.KeyboardEvent) => void + /** + * Is `UnderlineNav.Item` current page? + */ + 'aria-current'?: 'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false' | boolean + /** + * Icon before the text + */ + icon?: React.FunctionComponent | React.ReactElement + /** + * Counter + */ + counter?: number | string + } & LinkProps +> -export const UnderlineNavItem = forwardRef( - ( - { - sx: sxProp = defaultSxProp, - as: Component = 'a', - href = '#', - children, - counter, - onSelect, - 'aria-current': ariaCurrent, - icon: Icon, - ...props - }, - forwardedRef, - ) => { - const backupRef = useRef(null) - const ref = (forwardedRef ?? backupRef) as RefObject - const {setChildrenWidth, setNoIconChildrenWidth, loadingCounters, iconsVisible} = useContext(UnderlineNavContext) +function UnwrappedUnderlineNavItem( + { + as, + href = '#', + children, + counter, + onSelect, + 'aria-current': ariaCurrent, + icon: Icon, + ...props + }: UnderlineNavItemProps, + forwardedRef: React.ForwardedRef, +) { + const Component = as || 'a' + const backupRef = useRef(null) + const ref = (forwardedRef ?? backupRef) as RefObject + const {setChildrenWidth, setNoIconChildrenWidth, loadingCounters, iconsVisible} = useContext(UnderlineNavContext) + + useLayoutEffect(() => { + if (ref.current) { + const domRect = (ref as MutableRefObject).current.getBoundingClientRect() - useLayoutEffect(() => { - if (ref.current) { - const domRect = (ref as MutableRefObject).current.getBoundingClientRect() + const icon = Array.from((ref as MutableRefObject).current.children).find( + child => child.getAttribute('data-component') === 'icon', + ) - const icon = Array.from((ref as MutableRefObject).current.children).find( - child => child.getAttribute('data-component') === 'icon', - ) + const content = Array.from((ref as MutableRefObject).current.children).find( + child => child.getAttribute('data-component') === 'text', + ) as HTMLElement + const text = content.textContent as string - const content = Array.from((ref as MutableRefObject).current.children).find( - child => child.getAttribute('data-component') === 'text', - ) as HTMLElement - const text = content.textContent as string + const iconWidthWithMargin = icon + ? icon.getBoundingClientRect().width + + Number(getComputedStyle(icon).marginRight.slice(0, -2)) + + Number(getComputedStyle(icon).marginLeft.slice(0, -2)) + : 0 - const iconWidthWithMargin = icon - ? icon.getBoundingClientRect().width + - Number(getComputedStyle(icon).marginRight.slice(0, -2)) + - Number(getComputedStyle(icon).marginLeft.slice(0, -2)) - : 0 + setChildrenWidth({text, width: domRect.width}) + setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin}) + } + }, [ref, setChildrenWidth, setNoIconChildrenWidth]) - setChildrenWidth({text, width: domRect.width}) - setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin}) + const keyDownHandler = React.useCallback( + (event: React.KeyboardEvent) => { + if ((event.key === ' ' || event.key === 'Enter') && !event.defaultPrevented && typeof onSelect === 'function') { + onSelect(event) + } + }, + [onSelect], + ) + const clickHandler = React.useCallback( + (event: React.MouseEvent) => { + if (!event.defaultPrevented && typeof onSelect === 'function') { + onSelect(event) } - }, [ref, setChildrenWidth, setNoIconChildrenWidth]) + }, + [onSelect], + ) - const keyDownHandler = React.useCallback( - (event: React.KeyboardEvent) => { - if ((event.key === ' ' || event.key === 'Enter') && !event.defaultPrevented && typeof onSelect === 'function') { - onSelect(event) - } - }, - [onSelect], - ) - const clickHandler = React.useCallback( - (event: React.MouseEvent) => { - if (!event.defaultPrevented && typeof onSelect === 'function') { - onSelect(event) - } - }, - [onSelect], - ) + return ( +
  • + + {children} + +
    • + ) +} - return ( - - - {children} - - - ) - }, -) as PolymorphicForwardRefComponent<'a', UnderlineNavItemProps> +export const UnderlineNavItem = fixedForwardRef(UnwrappedUnderlineNavItem) -UnderlineNavItem.displayName = 'UnderlineNavItem' +Object.assign(UnderlineNavItem, {displayName: 'UnderlineNavItem'}) diff --git a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx index a2887dd13ad..b4b47563f33 100644 --- a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx +++ b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx @@ -7,6 +7,7 @@ import type {IconProps} from '@primer/octicons-react' import CounterLabel from '../../CounterLabel' import {type SxProp} from '../../sx' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../../utils/polymorphic' +import {type PolymorphicProps, fixedForwardRef} from '../../utils/modern-polymorphic' import classes from './UnderlineTabbedInterface.module.css' import {clsx} from 'clsx' @@ -44,51 +45,47 @@ export const LoadingCounter = () => { return } -export type UnderlineItemProps = { - as?: React.ElementType | 'a' | 'button' - className?: string - iconsVisible?: boolean - loadingCounters?: boolean - counter?: number | string - icon?: FC | React.ReactElement - id?: string - ref?: React.Ref -} & SxProp +export type UnderlineItemProps = PolymorphicProps< + As, + 'a', + { + className?: string + iconsVisible?: boolean + loadingCounters?: boolean + counter?: number | string + icon?: FC | React.ReactElement + id?: string + children?: React.ReactNode + } +> & + SxProp -export const UnderlineItem = forwardRef( - ( - { - as = 'a', - children, - counter, - icon: Icon, - iconsVisible, - loadingCounters, - className, - ...rest - }: PropsWithChildren, - forwardedRef, - ) => { - return ( - - {iconsVisible && Icon && {isElement(Icon) ? Icon : }} - {children && ( - - {children} +const UnwrappedUnderlineItem = ( + props: UnderlineItemProps, + forwardedRef: React.ForwardedRef, +) => { + const {as = 'a', children, counter, icon: Icon, iconsVisible, loadingCounters, className, ...rest} = props + return ( + + {iconsVisible && Icon && {isElement(Icon) ? Icon : }} + {children && ( + + {children} + + )} + {counter !== undefined ? ( + loadingCounters ? ( + + - )} - {counter !== undefined ? ( - loadingCounters ? ( - - - - ) : ( - - {counter} - - ) - ) : null} - - ) - }, -) as PolymorphicForwardRefComponent<'a', UnderlineItemProps> + ) : ( + + {counter} + + ) + ) : null} + + ) +} + +export const UnderlineItem = fixedForwardRef(UnwrappedUnderlineItem) diff --git a/packages/react/src/utils/modern-polymorphic.ts b/packages/react/src/utils/modern-polymorphic.ts new file mode 100644 index 00000000000..5f83844ac10 --- /dev/null +++ b/packages/react/src/utils/modern-polymorphic.ts @@ -0,0 +1,35 @@ +// Mostly taken from https://github.com/total-typescript/react-typescript-tutorial/blob/main/src/08-advanced-patterns/72-as-prop-with-forward-ref.solution.tsx + +import {forwardRef} from 'react' +import type {ComponentPropsWithRef, ElementType} from 'react' + +/** + * Distributive Omit utility type that works correctly with union types + */ +type DistributiveOmit = T extends unknown ? Omit : never + +/** + * Fixed version of forwardRef that provides better type inference for polymorphic components + */ +// TODO: figure out how to change this type so we can set displayName +// like this: `ComponentName.displayName = 'DisplayName' instead of using workarounds +type FixedForwardRef = = Record>( + render: (props: P, ref: React.Ref) => React.ReactNode, +) => (props: P & React.RefAttributes) => React.ReactNode + +/** + * Cast forwardRef to the fixed version with better type inference + */ +export const fixedForwardRef = forwardRef as FixedForwardRef + +/** + * Simplified polymorphic props type that handles the common pattern of + * `DistributiveOmit, 'as'>` + */ +export type PolymorphicProps< + TAs extends ElementType, + TDefaultElement extends ElementType = 'div', + Props extends Record = Record, +> = DistributiveOmit & Props, 'as'> & { + as?: TAs +} diff --git a/packages/styled-react/src/components/UnderlineNav.tsx b/packages/styled-react/src/components/UnderlineNav.tsx new file mode 100644 index 00000000000..f0495c34d03 --- /dev/null +++ b/packages/styled-react/src/components/UnderlineNav.tsx @@ -0,0 +1,26 @@ +import React, {type PropsWithChildren, type ComponentProps} from 'react' +import {UnderlineNav as PrimerUnderlineNav, Box} from '@primer/react' +import type { + UnderlineNavProps as PrimerUnderlineNavProps, + UnderlineNavItemProps as PrimerUnderlineNavItemProps, +} from '@primer/react' +import type {SxProp} from '../sx' + +type UnderlineNavProps = PropsWithChildren & SxProp + +const UnderlineNavImpl = React.forwardRef((props, ref) => { + return +}) + +type UnderlineNavItemProps = PropsWithChildren & SxProp + +const UnderlineNavItem = React.forwardRef((props, ref) => { + return +}) + +const UnderlineNav = Object.assign(UnderlineNavImpl, { + Item: UnderlineNavItem as React.FC> & SxProp>, +}) + +export {UnderlineNav} +export type {UnderlineNavProps, UnderlineNavItemProps} diff --git a/packages/styled-react/src/index.tsx b/packages/styled-react/src/index.tsx index 66763c5389a..fabd725d792 100644 --- a/packages/styled-react/src/index.tsx +++ b/packages/styled-react/src/index.tsx @@ -28,6 +28,7 @@ import type { SpaceProps, TypographyProps, } from 'styled-system' +import {UnderlineNav} from './components/UnderlineNav' type StyledProps = SxProp & SpaceProps & @@ -90,7 +91,7 @@ const ToggleSwitch = forwardRef(function T return }) -export {SegmentedControl, StateLabel, SubNav, ToggleSwitch} +export {SegmentedControl, StateLabel, SubNav, ToggleSwitch, UnderlineNav} export { ActionList, @@ -129,7 +130,6 @@ export { Token, Tooltip, Truncate, - UnderlineNav, // styled-components components or types Box, diff --git a/packages/styled-react/src/sx.ts b/packages/styled-react/src/sx.ts new file mode 100644 index 00000000000..e3ff0277f10 --- /dev/null +++ b/packages/styled-react/src/sx.ts @@ -0,0 +1,8 @@ +import css from '@styled-system/css' +import type {SxProp} from '@primer/react' + +export const sx = (props: SxProp) => { + return css(props.sx) +} + +export type {SxProp}