Skip to content
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

fix: dynamically setting options sets undefined to select value #655

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

igrep
Copy link

@igrep igrep commented May 14, 2024

Approach

Without this change, the Select component doesn't know the Options (items in the internal List component) after onMount. So it can't tell which Options are added/removed.

To fix the root cause, I created a new events to List: SMUIList:mountItem and SMUIList:unmountItem, which tells the parent when some of its children are added or removed. Then, I implemented an event handler of Select for the new events to update the internal list and call layoutOptions. layoutOptions should always be called whenever the options are updated (Ref. https://github.com/material-components/material-components-web/blob/f80ac92b08dfa1b59cd9faf74f3d19a4b134993e/packages/mdc-select/foundation.ts#L235-L239.

Another Option

The change might be simpler adding bind:accessor={list} to the List. But I didn't choose it because List seems to want to hide its accessor as the implementation details. I'll rewrite to try if you prefer.

Approach
====

Without this change, the `Select` component doesn't know the `Options`
(items in the internal `List` component) after `onMount`. So it can't tell
which `Option`s are added/removed.

To fix the root cause, I created a new events to `List`: `SMUIList:mountItem`
and `SMUIList:unmountItem`, which tells the parent when some of its children
are added or removed. Then, I implemented an event handler of `Select` for the
new events to update the internal list and call `layoutOptions`. `layoutOptions`
should always be called whenever the options are updated

Another Option
====

The change might be simpler adding `bind:accessor={list}` to the `List`. But I
didn't choose it because `List` seems to want to hide its `accessor` as the
implementation details. I'll rewrite if you prefer.

Ref: hperrin#538.

Ref: https://discord.com/channels/833139170703704115/1228040959670091787
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

Successfully merging this pull request may close these issues.

Updating Select components value while also changing the Options causes the value to be undefined
1 participant