Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cuddly-rules-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

update types for button extensions
48 changes: 23 additions & 25 deletions src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {ActionListContainerContext} from './ActionList/ActionListContainerContex
import {Button, ButtonProps} from './Button'
import {MandateProps} from './utils/types'
import {merge, BetterSystemStyleObject} from './sx'
import {defaultSxProp} from './utils/defaultSxProp'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from './utils/polymorphic'

export type MenuContextProps = Pick<
AnchoredOverlayProps,
Expand Down Expand Up @@ -68,34 +70,30 @@ const Menu: React.FC<React.PropsWithChildren<ActionMenuProps>> = ({
}

export type ActionMenuAnchorProps = {children: React.ReactElement}
const Anchor = React.forwardRef<AnchoredOverlayProps['anchorRef'], ActionMenuAnchorProps>(
({children, ...anchorProps}, anchorRef) => {
return React.cloneElement(children, {...anchorProps, ref: anchorRef})
},
)
const Anchor = React.forwardRef<HTMLElement, ActionMenuAnchorProps>(({children, ...anchorProps}, anchorRef) => {
return React.cloneElement(children, {...anchorProps, ref: anchorRef})
})

/** this component is syntactical sugar 🍭 */
export type ActionMenuButtonProps = ButtonProps
const MenuButton = React.forwardRef<AnchoredOverlayProps['anchorRef'], ButtonProps>(
({sx: sxProp = {}, ...props}, anchorRef) => {
return (
<Anchor ref={anchorRef}>
<Button
type="button"
trailingIcon={TriangleDownIcon}
sx={merge<BetterSystemStyleObject>(
{
// override the margin on caret for optical alignment
'[data-component=trailingIcon]': {marginX: -1},
},
sxProp,
)}
{...props}
/>
</Anchor>
)
},
)
const MenuButton = React.forwardRef(({sx: sxProp = defaultSxProp, ...props}, anchorRef) => {
return (
<Anchor ref={anchorRef}>
<Button
type="button"
trailingIcon={TriangleDownIcon}
sx={merge<BetterSystemStyleObject>(
{
// override the margin on caret for optical alignment
'[data-component=trailingIcon]': {marginX: -1},
},
sxProp,
)}
{...props}
/>
</Anchor>
)
}) as PolymorphicForwardRefComponent<'button', ActionMenuButtonProps>

type MenuOverlayProps = Partial<OverlayProps> &
Pick<AnchoredOverlayProps, 'align'> & {
Expand Down
17 changes: 10 additions & 7 deletions src/Button/ButtonBase.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import React, {ComponentPropsWithRef, forwardRef, useMemo} from 'react'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import Box from '../Box'
import {merge, SxProp} from '../sx'
import {BetterSystemStyleObject, merge} from '../sx'
import {useTheme} from '../ThemeProvider'
import {ButtonProps, StyledButton} from './types'
import {getVariantStyles, getSizeStyles, getButtonStyles} from './styles'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
import {defaultSxProp} from '../utils/defaultSxProp'
declare let __DEV__: boolean

const defaultSxProp = {}
const iconWrapStyles = {
display: 'inline-block',
}
Expand All @@ -17,18 +17,18 @@ const trailingIconStyles = {
ml: 2,
}

const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
const ButtonBase = forwardRef(
({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
const {leadingIcon: LeadingIcon, trailingIcon: TrailingIcon, variant = 'default', size = 'medium', ...rest} = props
const innerRef = React.useRef<HTMLElement>(null)
const innerRef = React.useRef<HTMLButtonElement>(null)
useRefObjectAsForwardedRef(forwardedRef, innerRef)

const {theme} = useTheme()
const baseStyles = useMemo(() => {
return merge.all([getButtonStyles(theme), getSizeStyles(size, variant, false), getVariantStyles(variant, theme)])
}, [theme, size, variant])
const sxStyles = useMemo(() => {
return merge(baseStyles, sxProp as SxProp)
const sxStyles: BetterSystemStyleObject = useMemo(() => {
return merge<BetterSystemStyleObject>(baseStyles, sxProp)
}, [baseStyles, sxProp])

if (__DEV__) {
Expand All @@ -40,7 +40,10 @@ const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
*/
// eslint-disable-next-line react-hooks/rules-of-hooks
React.useEffect(() => {
if (!(innerRef.current instanceof HTMLButtonElement) && !(innerRef.current instanceof HTMLAnchorElement)) {
if (
!(innerRef.current instanceof HTMLButtonElement) &&
!((innerRef.current as unknown) instanceof HTMLAnchorElement)
) {
// eslint-disable-next-line no-console
console.warn('This component should be an instanceof a semantic button or anchor')
}
Expand Down
8 changes: 5 additions & 3 deletions src/Button/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import {useTheme} from '../ThemeProvider'
import Box from '../Box'
import {IconButtonProps, StyledButton} from './types'
import {getBaseStyles, getSizeStyles, getVariantStyles} from './styles'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import {defaultSxProp} from '../utils/defaultSxProp'

const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>((props, forwardedRef): JSX.Element => {
const {variant = 'default', size = 'medium', sx: sxProp = {}, icon: Icon, ...rest} = props
const IconButton = forwardRef((props, forwardedRef): JSX.Element => {
const {variant = 'default', size = 'medium', sx: sxProp = defaultSxProp, icon: Icon, ...rest} = props
const {theme} = useTheme()
const sxStyles = merge.all([
getBaseStyles(theme),
Expand All @@ -21,6 +23,6 @@ const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>((props, forwar
</Box>
</StyledButton>
)
})
}) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>

export {IconButton}
16 changes: 5 additions & 11 deletions src/Button/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {ComponentPropsWithRef} from 'react'
import React from 'react'
import styled from 'styled-components'
import {IconProps} from '@primer/octicons-react'
import sx, {SxProp} from '../sx'
Expand All @@ -13,14 +13,9 @@ export type VariantType = 'default' | 'primary' | 'invisible' | 'danger' | 'outl

export type Size = 'small' | 'medium' | 'large'

/**
* Remove styled-components polymorphic as prop, which conflicts with radix's
*/
type StyledButtonProps = Omit<ComponentPropsWithRef<typeof StyledButton>, 'as'>

type ButtonA11yProps =
| {'aria-label': string; 'aria-labelledby'?: never}
| {'aria-label'?: never; 'aria-labelledby': string}
| {'aria-label': string; 'aria-labelledby'?: undefined}
| {'aria-label'?: undefined; 'aria-labelledby': string}
Comment thread
mattcosta7 marked this conversation as resolved.

export type ButtonBaseProps = {
/**
Expand All @@ -36,8 +31,7 @@ export type ButtonBaseProps = {
*/
disabled?: boolean
} & SxProp &
React.ButtonHTMLAttributes<HTMLButtonElement> &
StyledButtonProps
React.DetailedHTMLProps<React.ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>

export type ButtonProps = {
/**
Expand All @@ -53,7 +47,7 @@ export type ButtonProps = {

export type IconButtonProps = ButtonA11yProps & {
icon: React.FunctionComponent<React.PropsWithChildren<IconProps>>
} & ButtonBaseProps
} & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>
Comment thread
mattcosta7 marked this conversation as resolved.

// adopted from React.AnchorHTMLAttributes
export type LinkButtonProps = {
Expand Down
20 changes: 20 additions & 0 deletions src/__tests__/ActionMenu.types.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {ActionMenu} from '..'
import React from 'react'

export function actionButtonWithoutProps() {
//@ts-expect-error requires children
return <ActionMenu.Button />
}

export function actionButtonWithChildren() {
return <ActionMenu.Button>Click me!</ActionMenu.Button>
}

export function actionButtonWithOptionalProps() {
return <ActionMenu.Button size="small">Click me!</ActionMenu.Button>
}

export function actionButtonWithInvalidSize() {
//@ts-expect-error size must be one of the valid values
return <ActionMenu.Button size="some-unknownsize">Click me!</ActionMenu.Button>
}
39 changes: 37 additions & 2 deletions src/__tests__/Button.types.test.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {StopIcon} from '@primer/octicons-react'
import React, {useRef} from 'react'
import {Button} from '../Button'
import {Button, IconButton} from '../Button'

export function shouldAcceptOnlyAChildProp() {
return <Button>child</Button>
}

export function ShouldAcceptKnownButtonPropsAndDomProps() {
const buttonEl = useRef<HTMLButtonElement>()
const buttonEl = useRef<HTMLButtonElement | null>(null)
return (
<Button
ref={buttonEl}
Expand All @@ -33,3 +34,37 @@ export function shouldNotAcceptOutlandishProps() {
// @ts-expect-error system props should not be accepted
return <Button anOutlandshPropThatShouldNotBeAllowedOnA={'Button'} />
}

export function iconButtonRequiredProps() {
return (
<>
<IconButton icon={StopIcon} aria-label="Stop icon" />
<IconButton icon={StopIcon} aria-labelledby="Stop icon" />
</>
)
}

export function iconButtonShouldNotHaveLabelAndLabelledBy() {
// @ts-expect-error aria-label and aria-labelledby should not be allowed together
return <IconButton icon={StopIcon} aria-label="Stop icon" aria-labelledby="Stop icon" />
}

export function iconButtonRequiresAnIcon() {
// @ts-expect-error icon is required
return <IconButton aria-label="Stop icon" />
}

export function iconButtonOptionalProps() {
return (
<>
<IconButton icon={StopIcon} aria-label="Stop icon" size="small" />
<IconButton icon={StopIcon} aria-label="Stop icon" variant="danger" />
<IconButton icon={StopIcon} aria-label="Stop icon" sx={{m: 1}} />
</>
)
}

export function iconButtonShouldNotAcceptOutlandishProps() {
// @ts-expect-error system props should not be accepted
return <Button icon={StopIcon} aria-label="Stop icon" anOutlandshPropThatShouldNotBeAllowedOnA={'Button'} />
}
3 changes: 3 additions & 0 deletions src/utils/defaultSxProp.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import {BetterSystemStyleObject} from '../sx'

export const defaultSxProp: BetterSystemStyleObject = {}
Comment thread
mattcosta7 marked this conversation as resolved.
Outdated