[GitHub] [superset] geido commented on pull request #15121: feature: Select component

[GitBox <gi...@apache.org>]

geido commented on pull request #15121:
URL: https://github.com/apache/superset/pull/15121#issuecomment-860916782


   > @geido First thoughts:
   > 
   > * You can rename `AntdSelect` and `AntSelect.stories` to `Select` and `Select.stories`  using the `index` to resolve any name conflicts.
   
   This is just to simplify the review and to avoid that the new implementation would appear as changes made on the original file. Once this PR is merged, we can just rename it.
   
   > * I think it would be nice to simulate a service API for the storybook. We can support search, pagination, and also throw errors to test the error handling.
   
   I have enhanced the storybook to provide an Async Select dedicated story. However, I am looking to do more improvements there as soon as I am done with the rest of the implementation.
   
   > * You can use React `useReducer` to manipulate multiple state values if the number of states increases.
   
   I am definitely going to if the number of changes to the state should increase.
   
   > * Replace `handleOnDeselect = (option: any)` with `handleOnDeselect = (value: string | number | AntdLabeledValue)`
   > * Replace `handleOnSelect = (option: any)` with `handleOnSelect = (value: string | number | AntdLabeledValue)`
   
   I tried, however, typescript still complaints. I am avoiding dedicating too much time to this and leaving it for when the main implementation is completed.
   
   > * It would be nice to add one Select on each corner of the viewport on the storybook page. Then we can test the drop-down behavior for each corner and when scrolling.
   
   This is now done.
   
   Thank you!
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org