diff --git a/.changeset/gentle-badgers-confess.md b/.changeset/gentle-badgers-confess.md new file mode 100644 index 00000000000..ef01c618750 --- /dev/null +++ b/.changeset/gentle-badgers-confess.md @@ -0,0 +1,7 @@ +--- +'@primer/react': patch +--- + +Token: Update component type to be PolymorphicForwardRefComponent. + +this avoids types being swallowed by forwardRef (which isn't polymorphic) diff --git a/src/Token/AvatarToken.tsx b/src/Token/AvatarToken.tsx index 56be7184ec8..3378dcdd8b4 100644 --- a/src/Token/AvatarToken.tsx +++ b/src/Token/AvatarToken.tsx @@ -4,6 +4,7 @@ import {get} from '../constants' import {TokenBaseProps, defaultTokenSize, tokenSizes, TokenSizeKeys} from './TokenBase' import Token from './Token' import Avatar from '../Avatar' +import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' // TODO: update props to only accept 'large' and 'xlarge' on the next breaking change export interface AvatarTokenProps extends TokenBaseProps { @@ -20,7 +21,7 @@ const AvatarContainer = styled.span<{avatarSize: TokenSizeKeys}>` width: ${props => `calc(${tokenSizes[props.avatarSize]} - var(--spacing))`}; ` -const AvatarToken = forwardRef(({avatarSrc, id, size, ...rest}, forwardedRef) => { +const AvatarToken = forwardRef(({avatarSrc, id, size, ...rest}, forwardedRef) => { return ( ( @@ -44,7 +45,7 @@ const AvatarToken = forwardRef(({avatarSrc, id, s ref={forwardedRef} /> ) -}) +}) as PolymorphicForwardRefComponent<'span' | 'a' | 'button', AvatarTokenProps> AvatarToken.defaultProps = { size: defaultTokenSize, diff --git a/src/Token/IssueLabelToken.tsx b/src/Token/IssueLabelToken.tsx index d9556941531..f6eb899edf9 100644 --- a/src/Token/IssueLabelToken.tsx +++ b/src/Token/IssueLabelToken.tsx @@ -5,6 +5,7 @@ import RemoveTokenButton from './_RemoveTokenButton' import {parseToHsla, parseToRgba} from 'color2k' import {useTheme} from '../ThemeProvider' import TokenTextContainer from './_TokenTextContainer' +import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' export interface IssueLabelTokenProps extends TokenBaseProps { /** @@ -39,7 +40,7 @@ const darkModeStyles = { 'hsla(var(--label-h), calc(var(--label-s) * 1%),calc((var(--label-l) + var(--lighten-by)) * 1%),var(--border-alpha))', } -const IssueLabelToken = forwardRef((props, forwardedRef) => { +const IssueLabelToken = forwardRef((props, forwardedRef) => { const {as, fillColor = '#999', onRemove, id, isSelected, text, size, hideRemoveButton, href, onClick, ...rest} = props const interactiveTokenProps = { as, @@ -134,7 +135,7 @@ const IssueLabelToken = forwardRef((props, fo ) : null} ) -}) +}) as PolymorphicForwardRefComponent<'span' | 'a' | 'button', IssueLabelTokenProps> IssueLabelToken.defaultProps = { fillColor: '#999', diff --git a/src/Token/Token.tsx b/src/Token/Token.tsx index 6a4569ddc2a..d3f5e0f2ef6 100644 --- a/src/Token/Token.tsx +++ b/src/Token/Token.tsx @@ -1,14 +1,15 @@ import React, {forwardRef, MouseEventHandler} from 'react' import Box from '../Box' -import {merge, SxProp} from '../sx' +import {BetterSystemStyleObject, merge, SxProp} from '../sx' import {defaultSxProp} from '../utils/defaultSxProp' import TokenBase, {defaultTokenSize, isTokenInteractive, TokenBaseProps} from './TokenBase' import RemoveTokenButton from './_RemoveTokenButton' import TokenTextContainer from './_TokenTextContainer' +import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' // Omitting onResize and onResizeCapture because seems like React 18 types includes these menthod in the expansion but React 17 doesn't. // TODO: This is a temporary solution until we figure out why these methods are causing type errors. -export interface TokenProps extends Omit { +export interface TokenProps extends TokenBaseProps, SxProp { /** * A function that renders a component before the token text */ @@ -30,83 +31,81 @@ const LeadingVisualContainer: React.FC ) -const Token = forwardRef( - (props, forwardedRef) => { - const { - as, - onRemove, - id, - leadingVisual: LeadingVisual, - text, - size, - hideRemoveButton, - href, - onClick, - sx: sxProp = defaultSxProp, - ...rest - } = props - const hasMultipleActionTargets = isTokenInteractive(props) && Boolean(onRemove) && !hideRemoveButton - const onRemoveClick: MouseEventHandler = e => { - e.stopPropagation() - onRemove && onRemove() - } - const interactiveTokenProps = { - as, - href, - onClick, - } - const sx = merge( - { - backgroundColor: 'neutral.subtle', - borderColor: props.isSelected ? 'fg.default' : 'border.subtle', - borderStyle: 'solid', - borderWidth: `${tokenBorderWidthPx}px`, - color: props.isSelected ? 'fg.default' : 'fg.muted', - maxWidth: '100%', - paddingRight: !(hideRemoveButton || !onRemove) ? 0 : undefined, - ...(isTokenInteractive(props) - ? { - '&:hover': { - backgroundColor: 'neutral.muted', - boxShadow: 'shadow.medium', - color: 'fg.default', - }, - } - : {}), - }, - sxProp as SxProp, - ) +const Token = forwardRef((props, forwardedRef) => { + const { + as, + onRemove, + id, + leadingVisual: LeadingVisual, + text, + size, + hideRemoveButton, + href, + onClick, + sx: sxProp = defaultSxProp, + ...rest + } = props + const hasMultipleActionTargets = isTokenInteractive(props) && Boolean(onRemove) && !hideRemoveButton + const onRemoveClick: MouseEventHandler = e => { + e.stopPropagation() + onRemove && onRemove() + } + const interactiveTokenProps = { + as, + href, + onClick, + } + const sx = merge( + { + backgroundColor: 'neutral.subtle', + borderColor: props.isSelected ? 'fg.default' : 'border.subtle', + borderStyle: 'solid', + borderWidth: `${tokenBorderWidthPx}px`, + color: props.isSelected ? 'fg.default' : 'fg.muted', + maxWidth: '100%', + paddingRight: !(hideRemoveButton || !onRemove) ? 0 : undefined, + ...(isTokenInteractive(props) + ? { + '&:hover': { + backgroundColor: 'neutral.muted', + boxShadow: 'shadow.medium', + color: 'fg.default', + }, + } + : {}), + }, + sxProp, + ) - return ( - - {LeadingVisual ? ( - - - - ) : null} - {text} - {!hideRemoveButton && onRemove ? ( - - ) : null} - - ) - }, -) + return ( + + {LeadingVisual ? ( + + + + ) : null} + {text} + {!hideRemoveButton && onRemove ? ( + + ) : null} + + ) +}) as PolymorphicForwardRefComponent<'a' | 'button' | 'span', TokenProps> Token.displayName = 'Token' diff --git a/src/Token/TokenBase.tsx b/src/Token/TokenBase.tsx index 8f8b3369959..722f17b721f 100644 --- a/src/Token/TokenBase.tsx +++ b/src/Token/TokenBase.tsx @@ -1,8 +1,9 @@ -import React, {KeyboardEvent} from 'react' +import React, {ComponentProps, KeyboardEvent} from 'react' import styled from 'styled-components' import {variant} from 'styled-system' import {get} from '../constants' import sx, {SxProp} from '../sx' +import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' // TODO: remove invalid "extralarge" size name in next breaking change /** @deprecated 'extralarge' to be removed to align with size naming ADR https://github.com/github/primer/blob/main/adrs/2022-02-09-size-naming-guidelines.md **/ @@ -50,14 +51,12 @@ export interface TokenBaseProps size?: TokenSizeKeys } -type TokenElements = HTMLSpanElement | HTMLButtonElement | HTMLAnchorElement - export const isTokenInteractive = ({ as = 'span', onClick, onFocus, tabIndex = -1, -}: Pick) => +}: Pick, 'as' | 'onClick' | 'onFocus' | 'tabIndex'>) => Boolean(onFocus || onClick || tabIndex > -1 || ['a', 'button'].includes(as)) const xlargeVariantStyles = { @@ -133,25 +132,23 @@ const StyledTokenBase = styled.span` ${sx} ` -const TokenBase = React.forwardRef( - ({text, onRemove, onKeyDown, id, ...rest}, forwardedRef) => { - return ( - ) => { - onKeyDown && onKeyDown(event) +const TokenBase = React.forwardRef(({text, onRemove, onKeyDown, id, ...rest}, forwardedRef) => { + return ( + ) => { + onKeyDown && onKeyDown(event) - if ((event.key === 'Backspace' || event.key === 'Delete') && onRemove) { - onRemove() - } - }} - aria-label={onRemove ? `${text}, press backspace or delete to remove` : undefined} - id={id?.toString()} - {...rest} - ref={forwardedRef} - /> - ) - }, -) + if ((event.key === 'Backspace' || event.key === 'Delete') && onRemove) { + onRemove() + } + }} + aria-label={onRemove ? `${text}, press backspace or delete to remove` : undefined} + id={id?.toString()} + {...rest} + ref={forwardedRef} + /> + ) +}) as PolymorphicForwardRefComponent<'span' | 'a' | 'button', TokenBaseProps & SxProp> TokenBase.defaultProps = { as: 'span', diff --git a/src/__tests__/Token.types.test.tsx b/src/__tests__/Token.types.test.tsx new file mode 100644 index 00000000000..5034546bd6c --- /dev/null +++ b/src/__tests__/Token.types.test.tsx @@ -0,0 +1,135 @@ +import React from 'react' +import Token from '../Token' +import AvatarToken from '../Token/AvatarToken' +import IssueLabelToken from '../Token/IssueLabelToken' +import {CheckIcon} from '@primer/octicons-react' + +export function requiresAtLeastaTextProp() { + // @ts-expect-error text is required + return +} + +export function shouldPassWithOnlyTextProp() { + return +} + +export function shouldNotAcceptSystemProps() { + // @ts-expect-error system props should not be accepted + return +} + +export function acceptsAsProps() { + return ( + <> + + + + + ) +} + +export function acceptsOnRemoveProps() { + return {}} /> +} + +export function acceptsHideRemoveButtonProps() { + return +} + +export function acceptsIsSelectedProps() { + return +} + +export function acceptsIdProps() { + return +} + +export function acceptsSizeProps() { + return ( + <> + + + + + ) +} + +export function acceptsHrefProps() { + return +} + +export function acceptsLeadingVisualProps() { + return +} + +export function acceptsTrailingVisualProps() { + // @ts-expect-error unUsedPropsThatAreTotallyDefinitelyNotAllowed is not a valid prop + return +} + +export function allowsOnResizeProps() { + return {}} onResizeCapture={() => {}} /> +} + +export function allowsSxProp() { + return ( + + ) +} + +export function hasReasonableEventsForTargets() { + const handleButtonClick: React.MouseEventHandler = _event => {} + const handleAnchorClick: React.MouseEventHandler = _event => {} + const handleSpanClick: React.MouseEventHandler = _event => {} + return ( + <> + + + + + ) +} + +export function acceptsASubsetOfDomProps() { + return ( + <> + + + + + + + + ) +} + +export function specialTokenExtensionsAcceptTheirSpecificProps() { + return ( + <> + + + + ) +} + +export function specialTokenExtensionsDoNotAcceptOtherProps() { + return ( + <> + + + + ) +}