downshift icon indicating copy to clipboard operation
downshift copied to clipboard

Clean hooks props

Open silviuaavram opened this issue 3 years ago • 3 comments

Problem description:

Some props can be removed from the hooks:

  • defaultSelectedItem
  • defaultInputValue
  • change handlers can be replaced with onChange, onSelect (useSelect, useCombobox), onInputValueChange (useCombobox), onStateChange.

Suggested solution: Replace the props in v8.

silviuaavram avatar Jan 09 '23 08:01 silviuaavram

Hi @silviuaavram. Thanks for all your work on Downshift. It is a key dependency for Zendesk Garden. Can you clarify two things here for the v8 plan:

  • why would the (uncontrolled) default... props be removed?
  • which change handlers do you see getting removed? (or am I not reading that right?)

jzempel avatar Jan 27 '23 16:01 jzempel

The plan is to align with the original Downshift API, and there are also practical reasons. This will not affect Downshift component, but the useSelect and useCombobox hooks.

  • removing defaultSelectedItem and defaultInputValue makes sense as default values will be applied either at first render (like the initial props) and also at selection. Consequently, when you make a selection, it makes sense to reset the state for isOpen and highlightedIndex (keep the menu open with first item highlighted after selection, for example), but not with inputValue and selectedItem (since you already selected an item).
  • the onStateChange prop should be enough to execute side effects as certain elements of state are changed, and onSelect should be enough to execute side effect on selection, maybe onInputValueChange should also be kept on useCombobox. This aligns with Downshift's original API.

Sorry that I did not make it clear in the beginning that it does not affect Downshift, buy the hooks instead, which are the way going forward, since they allow for ARIA 1.2 combobox/select pattern implementations.

Let me know if I can provide any more info. Thanks!

silviuaavram avatar Jan 31 '23 07:01 silviuaavram

Thanks for the context and clarification. We are in the process of refactoring to a hooks-based API by encapsulating Downshift within our own useCombobox that smooths out behaviors specific to our domain. (You might hear more from me on this later 😉 .) For now, we want to avoid exposing parts of Downshift that will be removed in the next release.

jzempel avatar Jan 31 '23 17:01 jzempel

Will close this for now, for lack of value.

silviuaavram avatar Mar 09 '24 12:03 silviuaavram