Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {ActionListGroupProps, GroupContext} from './Group'
import {ActionListProps, ListContext} from './List'
import {Selection} from './Selection'
import {ActionListItemProps, Slots, TEXT_ROW_HEIGHT, getVariantStyles} from './shared'
import {defaultSxProp} from '../utils/defaultSxProp'

const LiBox = styled.li<SxProp>(sx)

Expand All @@ -21,7 +22,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
selected = undefined,
active = false,
onSelect,
sx: sxProp = {},
sx: sxProp = defaultSxProp,
id,
role,
_PrivateItemWrapper,
Expand Down
3 changes: 2 additions & 1 deletion src/ActionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from 'styled-components'
import sx, {SxProp, merge} from '../sx'
import {AriaRole} from '../utils/types'
import {ActionListContainerContext} from './ActionListContainerContext'
import {defaultSxProp} from '../utils/defaultSxProp'

export type ActionListProps = React.PropsWithChildren<{
/**
Expand Down Expand Up @@ -32,7 +33,7 @@ const ListBox = styled.ul<SxProp>(sx)

export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
(
{variant = 'inset', selectionVariant, showDividers = false, role, sx: sxProp = {}, ...props},
{variant = 'inset', selectionVariant, showDividers = false, role, sx: sxProp = defaultSxProp, ...props},
forwardedRef,
): JSX.Element => {
const styles = {
Expand Down
13 changes: 6 additions & 7 deletions src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {ActionListContainerContext} from './ActionList/ActionListContainerContex
import {Button, ButtonProps} from './Button'
import {useId} from './hooks/useId'
import {MandateProps} from './utils/types'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from './utils/polymorphic'

export type MenuContextProps = Pick<
AnchoredOverlayProps,
Expand Down Expand Up @@ -67,21 +68,19 @@ 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>(({...props}, anchorRef) => {
const MenuButton = React.forwardRef((props, anchorRef) => {
return (
<Anchor ref={anchorRef}>
<Button type="button" trailingAction={TriangleDownIcon} {...props} />
</Anchor>
)
})
}) as PolymorphicForwardRefComponent<'button', ActionMenuButtonProps>

type MenuOverlayProps = Partial<OverlayProps> &
Pick<AnchoredOverlayProps, 'align'> & {
Expand Down
7 changes: 2 additions & 5 deletions src/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ import {ButtonProps} from './types'
import {ButtonBase} from './ButtonBase'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'

const ButtonComponent: PolymorphicForwardRefComponent<'button', ButtonProps> = forwardRef<
HTMLButtonElement,
ButtonProps
>(({children, ...props}, forwardedRef): JSX.Element => {
const ButtonComponent = forwardRef(({children, ...props}, forwardedRef): JSX.Element => {
return (
<ButtonBase ref={forwardedRef} as="button" {...props}>
{children}
</ButtonBase>
)
})
}) as PolymorphicForwardRefComponent<'button', ButtonProps>

ButtonComponent.displayName = 'Button'

Expand Down
16 changes: 9 additions & 7 deletions src/Button/ButtonBase.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +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, getButtonStyles, getAlignContentSize} from './styles'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
declare let __DEV__: boolean
import {defaultSxProp} from '../utils/defaultSxProp'

const defaultSxProp = {}
const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
const ButtonBase = forwardRef(
({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
const {
leadingIcon: LeadingIcon,
Expand All @@ -22,15 +21,15 @@ const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
...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), getVariantStyles(variant, theme)])
}, [theme, variant])
const sxStyles = useMemo(() => {
return merge(baseStyles, sxProp as SxProp)
return merge<BetterSystemStyleObject>(baseStyles, sxProp)
}, [baseStyles, sxProp])
const iconWrapStyles = {
display: 'flex',
Expand All @@ -46,7 +45,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
3 changes: 2 additions & 1 deletion src/Button/ButtonCounter.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import React from 'react'
import {SxProp} from '../sx'
import CounterLabel from '../CounterLabel'
import {defaultSxProp} from '../utils/defaultSxProp'

export type CounterProps = {
children: number
} & SxProp

const Counter = ({children, sx: sxProp = {}, ...props}: CounterProps) => {
const Counter = ({children, sx: sxProp = defaultSxProp, ...props}: CounterProps) => {
return (
<CounterLabel data-component="ButtonCounter" sx={{ml: 2, ...sxProp}} {...props}>
{children}
Expand Down
10 changes: 6 additions & 4 deletions src/Button/IconButton.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React from 'react'
import React, {ComponentProps} from 'react'
import {EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon} from '@primer/octicons-react'
import {Story, Meta} from '@storybook/react'
import {IconButton} from '.'
import {OcticonArgType} from '../utils/story-helpers'

export default {
const meta: Meta<ComponentProps<typeof IconButton>> = {
title: 'Components/IconButton',
argTypes: {
size: {
Expand Down Expand Up @@ -33,6 +33,8 @@ export default {
'aria-label': 'Icon button description',
icon: XIcon,
},
} as Meta<typeof IconButton>
}

export const Playground: Story<typeof IconButton> = args => <IconButton {...args} />
export default meta

export const Playground: Story<ComponentProps<typeof IconButton>> = args => <IconButton {...args} />
21 changes: 15 additions & 6 deletions src/Button/IconButton.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import React, {forwardRef} from 'react'
import {merge, SxProp} from '../sx'
import {merge, BetterSystemStyleObject} from '../sx'
import {useTheme} from '../ThemeProvider'
import {IconButtonProps, StyledButton} from './types'
import {getBaseStyles, 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), getVariantStyles(variant, theme), sxProp as SxProp])
const sxStyles = merge.all<BetterSystemStyleObject>([getBaseStyles(theme), getVariantStyles(variant, theme), sxProp])
return (
<StyledButton data-component="IconButton" data-size={size} sx={sxStyles} {...rest} ref={forwardedRef}>
<StyledButton
data-component="IconButton"
data-size={size}
sx={sxStyles}
{...rest}
// @ts-expect-error StyledButton wants both Anchor and Button refs
ref={forwardedRef}
>
<Icon />
</StyledButton>
)
})
}) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>

export {IconButton}
19 changes: 13 additions & 6 deletions src/Button/LinkButton.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import React, {forwardRef} from 'react'
import {merge, SxProp} from '../sx'
import {BetterSystemStyleObject, merge} from '../sx'
import {LinkButtonProps} from './types'
import {ButtonBase, ButtonBaseProps} from './ButtonBase'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import {defaultSxProp} from '../utils/defaultSxProp'

type MyProps = LinkButtonProps & ButtonBaseProps

const LinkButton = forwardRef<HTMLElement, MyProps>(
({children, as: Component = 'a', sx = {}, ...props}, forwardedRef): JSX.Element => {
const sxStyle = merge.all([sx as SxProp])
const LinkButton = forwardRef(
({children, as: Component = 'a', sx = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
const sxStyle = merge.all<BetterSystemStyleObject>([sx])
return (
<ButtonBase as={Component} ref={forwardedRef} sx={sxStyle} {...props}>
<ButtonBase
as={Component}
// @ts-expect-error ButtonBase wants both Anchor and Button refs
ref={forwardedRef}
sx={sxStyle}
{...props}
>
{children}
</ButtonBase>
)
},
) as PolymorphicForwardRefComponent<'a', ButtonBaseProps>
) as PolymorphicForwardRefComponent<'a', MyProps>

export {LinkButton}
25 changes: 9 additions & 16 deletions src/Button/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
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'
import getGlobalFocusStyles from '../_getGlobalFocusStyles'

Expand All @@ -15,14 +14,9 @@ export type Size = 'small' | 'medium' | 'large'

export type AlignContent = 'start' | 'center'

/**
* 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}

export type ButtonBaseProps = {
/**
Expand All @@ -42,22 +36,21 @@ export type ButtonBaseProps = {
*/
block?: boolean
} & SxProp &
React.ButtonHTMLAttributes<HTMLButtonElement> &
StyledButtonProps
React.ButtonHTMLAttributes<HTMLButtonElement>

export type ButtonProps = {
/**
* The leading icon comes before button content
*/
leadingIcon?: React.FunctionComponent<React.PropsWithChildren<IconProps>>
leadingIcon?: React.ComponentType | null | undefined
/**
* The trailing icon comes after button content
*/
trailingIcon?: React.FunctionComponent<React.PropsWithChildren<IconProps>>
trailingIcon?: React.ComponentType | null | undefined
/**
* Trailing action appears to the right of the trailing visual and is always locked to the end
*/
trailingAction?: React.FunctionComponent<React.PropsWithChildren<IconProps>>
trailingAction?: React.ComponentType | null | undefined
children: React.ReactNode
/**
* Content alignment for when visuals are present
Expand All @@ -66,8 +59,8 @@ export type ButtonProps = {
} & ButtonBaseProps

export type IconButtonProps = ButtonA11yProps & {
icon: React.FunctionComponent<React.PropsWithChildren<IconProps>>
} & ButtonBaseProps
icon: React.ComponentType
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

broadens these to they work with arbitrary components, and not only primer octicons

} & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>

// adopted from React.AnchorHTMLAttributes
export type LinkButtonProps = {
Expand Down
7 changes: 4 additions & 3 deletions src/NavList/NavList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import Box from '../Box'
import StyledOcticon from '../StyledOcticon'
import sx, {merge, SxProp} from '../sx'
import {defaultSxProp} from '../utils/defaultSxProp'
import {useId} from '../hooks/useId'

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -43,7 +44,7 @@ export type NavListItemProps = {
} & SxProp

const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
({'aria-current': ariaCurrent, children, sx: sxProp = {}, ...props}, ref) => {
({'aria-current': ariaCurrent, children, sx: sxProp = defaultSxProp, ...props}, ref) => {
const {depth} = React.useContext(SubNavContext)

// Get SubNav from children
Expand Down Expand Up @@ -102,7 +103,7 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s

// TODO: ref prop
// TODO: Animate open/close transition
function ItemWithSubNav({children, subNav, sx: sxProp = {}}: ItemWithSubNavProps) {
function ItemWithSubNav({children, subNav, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) {
const buttonId = useId()
const subNavId = useId()
const [isOpen, setIsOpen] = React.useState(false)
Expand Down Expand Up @@ -167,7 +168,7 @@ const SubNavContext = React.createContext<{depth: number}>({depth: 0})

// TODO: ref prop
// NOTE: SubNav must be a direct child of an Item
const SubNav = ({children, sx: sxProp = {}}: NavListSubNavProps) => {
const SubNav = ({children, sx: sxProp = defaultSxProp}: NavListSubNavProps) => {
const {buttonId, subNavId, isOpen} = React.useContext(ItemWithSubNavContext)
const {depth} = React.useContext(SubNavContext)

Expand Down
4 changes: 2 additions & 2 deletions src/PageHeader/PageHeader.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ const Template: Story = args => (
}}
>
<PageHeader.LeadingAction hidden={!args.hasLeadingAction}>
<IconButton icon={SidebarExpandIcon} variant="invisible" />{' '}
<IconButton aria-label="Expand" icon={SidebarExpandIcon} variant="invisible" />{' '}
</PageHeader.LeadingAction>
<PageHeader.LeadingVisual hidden={!args.hasLeadingVisual}>{<args.LeadingVisual />}</PageHeader.LeadingVisual>
<PageHeader.Title as={args['Title.as']} hidden={!args.hasTitle}>
Expand All @@ -227,7 +227,7 @@ const Template: Story = args => (
<Label>Beta</Label>
</PageHeader.TrailingVisual>
<PageHeader.TrailingAction hidden={!args.hasTrailingAction}>
<IconButton icon={PencilIcon} variant="invisible" />
<IconButton aria-label="Edit" icon={PencilIcon} variant="invisible" />
</PageHeader.TrailingAction>
<PageHeader.Actions hidden={!args.hasActions}>
<Hidden on={['narrow']}>
Expand Down
8 changes: 4 additions & 4 deletions src/PageHeader/PageHeader.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ describe('PageHeader', () => {
<PageHeader.ContextArea>ContextArea</PageHeader.ContextArea>
<PageHeader.TitleArea>
<PageHeader.LeadingAction>
<IconButton data-testid="LeadingAction" icon={SidebarExpandIcon} variant="invisible" />
<IconButton aria-label="Expand" data-testid="LeadingAction" icon={SidebarExpandIcon} variant="invisible" />
</PageHeader.LeadingAction>
<PageHeader.Title>Title</PageHeader.Title>
<PageHeader.TrailingAction>
<IconButton data-testid="TrailingAction" icon={PencilIcon} variant="invisible" />
<IconButton aria-label="edit" data-testid="TrailingAction" icon={PencilIcon} variant="invisible" />
</PageHeader.TrailingAction>
</PageHeader.TitleArea>
<PageHeader.Description></PageHeader.Description>
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('PageHeader', () => {
<PageHeader.TitleArea variant="large">
<PageHeader.LeadingAction>
Leading Action
<IconButton icon={SidebarExpandIcon} variant="invisible" />
<IconButton aria-label="expand" icon={SidebarExpandIcon} variant="invisible" />
</PageHeader.LeadingAction>
<PageHeader.LeadingVisual>
Leading Visual
Expand All @@ -138,7 +138,7 @@ describe('PageHeader', () => {
<PageHeader.Title>Title</PageHeader.Title>
<PageHeader.TrailingAction>
Trailing Action
<IconButton icon={PencilIcon} variant="invisible" />
<IconButton aria-label="edit" icon={PencilIcon} variant="invisible" />
</PageHeader.TrailingAction>
<PageHeader.TrailingVisual>
Trailing Visual
Expand Down
Loading