Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: CHI-696 select component accepts an inputRef prop #233

Merged
merged 3 commits into from
Nov 7, 2019

Conversation

rtalvarez
Copy link
Member

@rtalvarez rtalvarez commented Nov 4, 2019

What?

The Select component now accepts an optional inputRef of type RefObject<HTMLInputElement> | React.Ref<HTMLInputElement>

Usage:

The prop accepts three usages:

  • inputRef={undefined}
  • inputRef={React.createRef<HTMLInputElement>()}
  • inputRef={(input: HTMLInputElement) => void}

@rtalvarez rtalvarez requested a review from a team November 4, 2019 21:01
@jorgemoya
Copy link
Contributor

Need to add new prop to the docs. 🙏

@deini
Copy link
Member

deini commented Nov 5, 2019

Can we use forwardRef 🙏

@rtalvarez
Copy link
Member Author

👷 🔨

@rtalvarez
Copy link
Member Author

Offline: this prop name will remain as inputRef as opposed to ref (by using forwardRef) because it is not possible to pass generic type arguments to React.forwardRef as of right now, so the type T for the select options would be lost by doing this:

export const Select = React.forwardRef<HTMLInputElement, SelectProps<any>>( ... );

Various workarounds for this did not yield acceptable results

@rtalvarez rtalvarez changed the title feat: select component accepts an inputRef prop feat: CHI-696 select component accepts an inputRef prop Nov 7, 2019
@rtalvarez rtalvarez merged commit 847e8ef into bigcommerce:master Nov 7, 2019
@rtalvarez rtalvarez deleted the select-ref branch November 7, 2019 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants