Skip to content

Commit

Permalink
Fix TextInput types (#1959)
Browse files Browse the repository at this point in the history
* Add failing TextInput type test

* Create loud-schools-own.md

* Update TextInput types

* Add another TextInput type test

* Update TextInput props type

* Update TextInputWithTokens types

* Fix incorrect uses of TextInputWithTokens

* Update SelectMenu types test

* Fix TextInput tests

* Update snapshot tests

* autocomplete -> autoComplete

* Remove theme prop SelectMenuFilter

* Fix TextInput in Overlay story

* Fix TextInputWithTokens stories
  • Loading branch information
colebemis committed Mar 16, 2022
1 parent 1b01485 commit 2025036
Show file tree
Hide file tree
Showing 16 changed files with 564 additions and 526 deletions.
5 changes: 5 additions & 0 deletions .changeset/loud-schools-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Fix `TextInput` types
2 changes: 1 addition & 1 deletion src/Autocomplete/AutocompleteInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ const AutocompleteInput = React.forwardRef(
aria-expanded={showMenu}
aria-haspopup="listbox"
aria-owns={`${id}-listbox`}
autocomplete="off"
autoComplete="off"
id={id}
{...props}
/>
Expand Down
28 changes: 18 additions & 10 deletions src/TextInput.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,34 @@
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '@radix-ui/react-polymorphic'
import classnames from 'classnames'
import React from 'react'
import {ComponentProps, Merge} from './utils/types'
import {Merge} from './utils/types'
import TextInputWrapper, {StyledWrapperProps} from './_TextInputWrapper'
import UnstyledTextInput from './_UnstyledTextInput'
import TextInputWrapper from './_TextInputWrapper'

type NonPassthroughProps = {
className?: string
/** @deprecated Use `leadingVisual` or `trailingVisual` prop instead */
icon?: React.ComponentType<{className?: string}>
leadingVisual?: string | React.ComponentType<{className?: string}>
trailingVisual?: string | React.ComponentType<{className?: string}>
} & Pick<
ComponentProps<typeof TextInputWrapper>,
'block' | 'contrast' | 'disabled' | 'monospace' | 'sx' | 'width' | 'maxWidth' | 'minWidth' | 'variant' | 'size'
StyledWrapperProps,
| 'block'
| 'contrast'
| 'disabled'
| 'monospace'
| 'sx'
| 'width'
| 'maxWidth'
| 'minWidth'
| 'variant'
| 'size'
| 'validationStatus'
>

// Note: using ComponentProps instead of ComponentPropsWithoutRef here would cause a type issue where `css` is a required prop.
type TextInputInternalProps = Merge<React.ComponentPropsWithoutRef<typeof UnstyledTextInput>, NonPassthroughProps>
export type TextInputProps = Merge<React.ComponentPropsWithoutRef<'input'>, NonPassthroughProps>

// using forwardRef is important so that other components (ex. SelectMenu) can autofocus the input
const TextInput = React.forwardRef<HTMLInputElement, TextInputInternalProps>(
const TextInput = React.forwardRef<HTMLInputElement, TextInputProps>(
(
{
icon: IconComponent,
Expand Down Expand Up @@ -78,13 +87,12 @@ const TextInput = React.forwardRef<HTMLInputElement, TextInputInternalProps>(
</TextInputWrapper>
)
}
)
) as PolymorphicForwardRefComponent<'input', TextInputProps>

TextInput.defaultProps = {
type: 'text'
}

TextInput.displayName = 'TextInput'

export type TextInputProps = ComponentProps<typeof TextInput>
export default TextInput
29 changes: 11 additions & 18 deletions src/TextInputWithTokens.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
import React, {FocusEventHandler, KeyboardEventHandler, MouseEventHandler, RefObject, useRef, useState} from 'react'
import {omit} from '@styled-system/props'
import {FocusKeys} from '@primer/behaviors'
import {isFocusable} from '@primer/behaviors/utils'
import {omit} from '@styled-system/props'
import React, {FocusEventHandler, KeyboardEventHandler, MouseEventHandler, RefObject, useRef, useState} from 'react'
import Box from './Box'
import {useProvidedRefOrCreate} from './hooks'
import {useCombinedRefs} from './hooks/useCombinedRefs'
import {useFocusZone} from './hooks/useFocusZone'
import {ComponentProps} from './utils/types'
import Text from './Text'
import {TextInputProps} from './TextInput'
import Token from './Token/Token'
import {TokenSizeKeys} from './Token/TokenBase'
import {TextInputProps} from './TextInput'
import {useProvidedRefOrCreate} from './hooks'
import UnstyledTextInput from './_UnstyledTextInput'
import TextInputWrapper, {textInputHorizPadding, TextInputSizes} from './_TextInputWrapper'
import Box from './Box'
import Text from './Text'
import {isFocusable} from '@primer/behaviors/utils'
import UnstyledTextInput from './_UnstyledTextInput'

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type AnyReactComponent = React.ComponentType<any>

// NOTE: if these props or their JSDoc comments are updated, be sure to also update
// the prop table in docs/content/TextInputTokens.mdx
type TextInputWithTokensInternalProps<TokenComponentType extends AnyReactComponent> = {
export type TextInputWithTokensProps<TokenComponentType extends AnyReactComponent = typeof Token> = {
/**
* The array of tokens to render
*/
Expand Down Expand Up @@ -53,7 +52,7 @@ type TextInputWithTokensInternalProps<TokenComponentType extends AnyReactCompone
* The number of tokens to display before truncating
*/
visibleTokenCount?: number
} & TextInputProps
} & Omit<TextInputProps, 'size'>

const overflowCountFontSizeMap: Record<TokenSizeKeys, number> = {
small: 0,
Expand All @@ -72,7 +71,6 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
className,
block,
disabled,
theme,
sx: sxProp,
tokens,
onTokenRemove,
Expand All @@ -88,10 +86,7 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
variant: variantProp, // deprecated. use `size` instead
visibleTokenCount,
...rest
}: TextInputWithTokensInternalProps<TokenComponentType> & {
selectedTokenIndex: number | undefined
setSelectedTokenIndex: React.Dispatch<React.SetStateAction<number | undefined>>
},
}: TextInputWithTokensProps<TokenComponentType>,
externalRef: React.ForwardedRef<HTMLInputElement>
) {
const {onBlur, onFocus, onKeyDown, ...inputPropsRest} = omit(rest)
Expand Down Expand Up @@ -252,7 +247,6 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
disabled={disabled}
hasLeadingVisual={Boolean(LeadingVisual)}
hasTrailingVisual={Boolean(TrailingVisual)}
theme={theme}
width={widthProp}
minWidth={minWidthProp}
maxWidth={maxWidthProp}
Expand Down Expand Up @@ -372,5 +366,4 @@ TextInputWithTokens.defaultProps = {

TextInputWithTokens.displayName = 'TextInputWithTokens'

export type TextInputWithTokensProps = ComponentProps<typeof TextInputWithTokens>
export default TextInputWithTokens
2 changes: 1 addition & 1 deletion src/__tests__/FormControl.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe('FormControl', () => {
{text: 'one', id: 1},
{text: 'two', id: 2}
]}
onRemove={onRemoveMock}
onTokenRemove={onRemoveMock}
/>
</FormControl>
</SSRProvider>
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/SelectMenu.types.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function shouldNotAcceptSystemProps() {
<SelectMenu.List backgroundColor="lightgreen" />
{/* @ts-expect-error system props should not be accepted */}
<SelectMenu.Divider backgroundColor="lightgrey" />
{/* This will not error for now, but once TextInputProps is fixed, a ts-expect-error can be added */}
{/* @ts-expect-error system props should not be accepted */}
<SelectMenu.Filter backgroundColor="lightpink" />
{/* @ts-expect-error system props should not be accepted */}
<SelectMenu.Footer backgroundColor="lightsalmon" />
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/TextInput.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ describe('TextInput', () => {
})

it('renders warning', () => {
expect(render(<TextInput name="zipcode" status="warning" />)).toMatchSnapshot()
expect(render(<TextInput name="zipcode" validationStatus="warning" />)).toMatchSnapshot()
})

it('renders error', () => {
expect(render(<TextInput name="zipcode" status="error" />)).toMatchSnapshot()
expect(render(<TextInput name="zipcode" validationStatus="error" />)).toMatchSnapshot()
})

it('renders contrast', () => {
Expand Down
12 changes: 12 additions & 0 deletions src/__tests__/TextInput.types.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import React from 'react'
import {TextInput} from '..'

export function shouldNotAcceptInvalidDomProps() {
// @ts-expect-error invalid DOM props should not be accepted
return <TextInput onKeyDown={true} />
}

export function shouldNotAcceptInvalidSize() {
// @ts-expect-error invalid size value should not be accepted
return <TextInput size="big" />
}
Loading

0 comments on commit 2025036

Please sign in to comment.