Skip to content

Commit

Permalink
Refine 'as' prop type in button component and fix name of aria type (#…
Browse files Browse the repository at this point in the history
…2659)

* Refine as prop in button component and fix aria type

* whoops, remove type change

* fix lint bugs

* changeset

* use local mergeRef

* undo package-lock changes

* Update src/Button/ButtonBase.tsx

* Update src/Button/ButtonBase.tsx

Co-authored-by: Josh Black <joshblack@github.com>

* fix eslint

* define dev

Co-authored-by: Josh Black <joshblack@github.com>
  • Loading branch information
kendallgassner and joshblack committed Dec 7, 2022
1 parent 5551fdd commit 84d2997
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-horses-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Add a console warning if the Button and IconButton as property is used incorrectly
16 changes: 15 additions & 1 deletion src/Button/ButtonBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {merge, SxProp} from '../sx'
import {useTheme} from '../ThemeProvider'
import {ButtonProps, StyledButton} from './types'
import {getVariantStyles, getSizeStyles, getButtonStyles} from './styles'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
declare let __DEV__: boolean

const defaultSxProp = {}
const iconWrapStyles = {
Expand All @@ -18,6 +20,9 @@ const trailingIconStyles = {
const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
({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)
useRefObjectAsForwardedRef(forwardedRef, innerRef)

const {theme} = useTheme()
const baseStyles = useMemo(() => {
return merge.all([getButtonStyles(theme), getSizeStyles(size, variant, false), getVariantStyles(variant, theme)])
Expand All @@ -26,8 +31,17 @@ const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
return merge(baseStyles, sxProp as SxProp)
}, [baseStyles, sxProp])

React.useEffect(() => {
if (!(innerRef.current instanceof HTMLButtonElement) && !(innerRef.current instanceof HTMLAnchorElement)) {
if (__DEV__) {
// eslint-disable-next-line no-console
console.warn('This component should be an instanceof a semantic button or anchor')
}
}
}, [innerRef])

return (
<StyledButton as={Component} sx={sxStyles} {...rest} ref={forwardedRef}>
<StyledButton as={Component} sx={sxStyles} {...rest} ref={innerRef}>
{LeadingIcon && (
<Box as="span" data-component="leadingIcon" sx={iconWrapStyles}>
<LeadingIcon />
Expand Down
4 changes: 3 additions & 1 deletion src/Button/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export type Size = 'small' | 'medium' | 'large'
*/
type StyledButtonProps = Omit<ComponentPropsWithRef<typeof StyledButton>, 'as'>

type ButtonA11yProps = {'aria-label': string; 'aria-labelby'?: never} | {'aria-label'?: never; 'aria-labelby': string}
type ButtonA11yProps =
| {'aria-label': string; 'aria-labelledby'?: never}
| {'aria-label'?: never; 'aria-labelledby': string}

export type ButtonBaseProps = {
/**
Expand Down

0 comments on commit 84d2997

Please sign in to comment.