Skip to content

Conversation

@forevermatt
Copy link
Contributor

Fixed

  • Only have the Select component dispatch populated events on initial load or changes to the list of options
    • This fixes a problem where, when the Select's value is changed (by the user), the change and populated events are both dispatched, even if the list of options is unchanged. Those subsequent dispatches of the populated event made it too easy for consumers of this component to accidentally set the value of the Select back to what it was an instant before, effectively preventing the user's choice from "sticking" sometimes.

This fixes a problem where, when the Select's value is changed (by the
user), the `change` and `populated` events are both dispatched, even if
the list of options is unchanged.

Those subsequent dispatches of the `populated` event make it too easy
for consumers of this component to accidentally set the value of the
Select back to what it was an instant before, effectively preventing
the user's choice from "sticking" sometimes.
@forevermatt forevermatt requested a review from a team October 24, 2023 20:02
let element = {}
let mdcSelect = {}
let previousOptionsIDsCSV = ''

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be passed in, or how are you setting this to previous option IDs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens on line 55 inside afterUpdate. The first time options is populated it will be empty, meaning optionsHaveChanged will return true. Is that correct @forevermatt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hobbitronics Yes, that's exactly it.

Copy link
Contributor

@hobbitronics hobbitronics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for fixing this.

let element = {}
let mdcSelect = {}
let previousOptionsIDsCSV = ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens on line 55 inside afterUpdate. The first time options is populated it will be empty, meaning optionsHaveChanged will return true. Is that correct @forevermatt?

@forevermatt forevermatt merged commit b184abd into develop Oct 25, 2023
@forevermatt forevermatt deleted the feature/fix-select-populated-event-dispatches branch October 25, 2023 13:28
@hobbitronics
Copy link
Contributor

🎉 This PR is included in version 10.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants