sci-components icon indicating copy to clipboard operation
sci-components copied to clipboard

Bug: Dropdown controlled state mismatch

Open tihuan opened this issue 1 year ago • 0 comments

Context:

Currently Dropdown setValueAndCallOnChange() function will always setValue(). However, recently SC has a use case where after selecting a new Dropdown item, we pop a confirm modal to ask if the user is sure they want to commit to the new selection. If they cancel it, we want to keep the old value

SC feature PR: https://github.com/chanzuckerberg/single-cell-data-portal/pull/6075

However, with setValueAndCallOnChange() always setting value, we ran into a mismatch where the parent is on the old value, but the Dropdown component's value state is the new value (which manifests when a user opens the Dropdown and see the new value is selected, even though DropdownInput shows the old value is selected)

Fix:

  1. I think we just need to add a condition in setValueAndCallOnChange() to check if the component is controlled. If so, don't setValue and wait for the parent to change and pass the new value down as props.value, so useEffect() will catch it and update the value!

E.g.,

// Old
function setValueAndCallOnChange(
    newValue: Value<DefaultDropdownMenuOption, Multiple>
) {
    setValue(newValue);
    onChange(newValue);
 }

// New
function setValueAndCallOnChange(
    newValue: Value<DefaultDropdownMenuOption, Multiple>
) {
  if (!isControlled) {
    setValue(newValue);
  }    
  
  onChange(newValue);
}

However, I haven't tested the code, so not sure if I'm missing any edge cases 💡

Thank you!

tihuan avatar Oct 25 '23 20:10 tihuan