fix: dynamically setting options sets undefined to select value#655
Open
igrep wants to merge 1 commit intohperrin:masterfrom
Open
fix: dynamically setting options sets undefined to select value#655igrep wants to merge 1 commit intohperrin:masterfrom
igrep wants to merge 1 commit intohperrin:masterfrom
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Approach
Without this change, the
Selectcomponent doesn't know theOptions(items in the internalListcomponent) afteronMount. So it can't tell whichOptions are added/removed.To fix the root cause, I created a new events to
List:SMUIList:mountItemandSMUIList:unmountItem, which tells the parent when some of its children are added or removed. Then, I implemented an event handler ofSelectfor the new events to update the internal list and calllayoutOptions.layoutOptionsshould 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 theList. But I didn't choose it becauseListseems to want to hide itsaccessoras the implementation details. I'll rewrite to try if you prefer.