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

fix: change getToggleButtonProps typing to support div #1486

Closed

Conversation

drewbrend
Copy link
Contributor

@drewbrend drewbrend commented Mar 15, 2023

What:

getToggleButtonProps should support HTMLButtonElement or HTMLDivElement props

Why:

In the downshift docs, useSelect recommends applying getToggleButtonProps to a div element but getToggleButtonProps specifies its options are specific to a button. This works fine, unless you want to provide options to getToggleButtonProps that are unique to a div element, in my case I ran into this when providing a ref to my HTMLDivElement - this conflicted because Ref<HTMLDivElement> is not applicable to Ref<HTMLButtonElement>

How:

Simply added | HTMLDivElement to its type.

Checklist:

  • Documentation
  • Tests
  • TypeScript Types
  • Flow Types
  • Ready to be merged

@silviuaavram
Copy link
Collaborator

Thanks @drewbrend for the fix! I will make sure to address it in #1482. The reason is that only the UseSelectToggleButtonPropsOptions should have this change, which is going to most probably be only HTMLElement, since someone may want to use anything else other than a div. We are using a div, aria uses a div in their example, but that's not really a hard requirement. It most probably should not be a button, since button with a role of combobox is not really correct, but it might not be a div anyway.

And it's going to be a breaking change, so expect this to arrive in v8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants