ItsJonQ / g2

✨ An experimental reimagining of WordPress components
http://g2-components.com/
MIT License
105 stars 12 forks source link

Possible Simplifciation of useSelectDropdown #203

Closed jorgefilipecosta closed 3 years ago

jorgefilipecosta commented 3 years ago

Currently, useSelectDropdown uses a substate to keep state, but it seems all the state is synchronized from other entities that also keep the same state. So I guess we may be able to remove the store.

The state required from the SelectDropdown component is:

The current value comes from the props. I think we should not have it as a local state. Currently, we have it as local state in two forms value, and commitValue. Why do we need a commit value can't we call onChange right away?

Regarding the dropdown open or not, and the current item to highlight state, it seems downshift provides a useSelect hook for this state, and we are using it, but then we synchronize it to our store. Can't we rely on downshift state as the source of truth?

I think the synchronization of state may be related to the issue https://github.com/ItsJonQ/g2/pull/201 and https://github.com/ItsJonQ/g2/issues/202.

ItsJonQ commented 3 years ago

Resolved in https://github.com/ItsJonQ/g2/issues/223