Skip to content

onChange prop isn't triggering for Multiselect #185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kimkvn opened this issue Aug 23, 2022 · 0 comments
Open

onChange prop isn't triggering for Multiselect #185

kimkvn opened this issue Aug 23, 2022 · 0 comments

Comments

@kimkvn
Copy link

kimkvn commented Aug 23, 2022

Hello!

I'm working with the MultiSelect component, but finding that the onChange prop isn't triggering. The rest of the event handlers seem to be functioning as intended.

The onChange prop for SingleSelect is still working, so I dug into the library and compared the onChangeBroadcasters. I copied over and am testing the example code in the demo: SingleSelect Basic, MultiSelect Basic

In singleSelectBroadcastChange when determining if prev and curr values are equal:

const shouldBroadcastChange = !isEqual(prevValue?.value, currValue?.value);

seems to check out because for example we compare object-key pairings

prevValue: 
{
  name: "carType1"
  text: "Subaru"
  value: "subaru"
}

currValue: 
{
  name: "carType1"
  text: "BMW"
  value: "bmw"
}

However in multiSelectBroadCastChange when looking at the equivalent boolean:

const shouldBroadcastChange = !prevOptions || !isEqual(prevOptions.values, currOptions.values);

We're comparing arrays, and not an object-key pairing

prevOptions: [{
    name: "make6"
    text: "Fiat"
    value: "fiat"
  },
  {
    name: "make6"
    text: "Suzuki"
    value: "suzuki"
  }]

currOptions:[{
    name: "make6"
    text: "Fiat"
    value: "fiat"
  },
  {
    name: "make6"
    text: "Suzuki"
    value: "suzuki"
  },
  {
    name: "make6"
    text: "Subaru"
    value: "subaru"
  }]

and so running isEqual(prevOptions.values, currOptions.values) seems to always evaluate to true, which then makes shouldBroadcastChange eval to false, which causes the onChange callback to not fire. Should we be evaluating isEqual(prevOptions, currOptions) instead? Wondering if this is a bug, or if I'm misinterpreting the logic here.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant