[GitHub] [superset] kgabryje commented on pull request #22830: fix: Handles disabled options on Select All

["kgabryje (via GitHub)" <gi...@apache.org>]

kgabryje commented on PR #22830:
URL: https://github.com/apache/superset/pull/22830#issuecomment-1419533486

   
   > @kgabryje Can you provide an example? I tested in the Storybook but was not able to reproduce the bug.
   
   I think there's a problem with `Select`'s `onChange` function. When selecting/deselecting, `onChange` provides an array of all currently selected item ids. So when there is a disabled option that is selected by default, un-clicking `Select all` should call `onChange` with an array containing the disabled option's id. Instead, it's passing an empty array.
   
   Mock piece of code that should allow to repro this example:
   
   ```
   const [selectValue, setSelectValue] = useState(1);
   const options = [{ value: 1, disabled: true }, { value: 2 }, { value: 3 }];
   const handleChange = (selected) => setSelectValue(selected); 
   return <Select options={options} value={selectValue} onChange={handleChange} />
   ```
   
   In this example, value `1` should be selected by default and be un-selectable. Un-selecting all will call `onChange` with an empty array and un-select value `1`.
   Of course, I could work around it by adding logic to `handleChange` to ensure that `1` stays selected, but I think it'd be better to have that logic in `Select` component.
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

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