Skip to content

Commit

Permalink
fix: dynamically setting options sets undefined to select value
Browse files Browse the repository at this point in the history
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
  • Loading branch information
YAMAMOTO Yuji committed May 14, 2024
1 parent 03d3456 commit e231b5e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
5 changes: 4 additions & 1 deletion packages/list/src/List.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
let element: SvelteComponent;
let instance: MDCListFoundation;
let accessor: SMUIListAccessor;
let items: SMUIListItemAccessor[] = [];
let role = getContext<string | undefined>('SMUI:list:role');
let nav = getContext<boolean | undefined>('SMUI:list:nav');
Expand Down Expand Up @@ -232,7 +233,7 @@
},
});
const accessor: SMUIListAccessor = {
accessor = {
get element() {
return getElement();
},
Expand Down Expand Up @@ -282,6 +283,7 @@
selectedIndex = getListItemIndex(event.detail.element);
}
event.stopPropagation();
dispatch(getElement(), 'SMUIList:mountItem', accessor);
}
function handleItemUnmount(event: CustomEvent<SMUIListItemAccessor>) {
Expand All @@ -292,6 +294,7 @@
itemAccessorMap.delete(event.detail.element);
}
event.stopPropagation();
dispatch(getElement(), 'SMUIList:unmountItem', accessor);
}
function handleKeydown(event: KeyboardEvent) {
Expand Down
16 changes: 13 additions & 3 deletions packages/select/src/Select.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@
{wrapFocus}
bind:selectedIndex
on:SMUIList:mount={(event) => (list = event.detail)}
on:SMUIList:mountItem={handleItemMountUnmount}
on:SMUIList:unmountItem={handleItemMountUnmount}
{...prefixFilter($$restProps, 'list$')}><slot /></List
>
</Menu>
Expand Down Expand Up @@ -473,10 +475,13 @@
},
getSelectedIndex: () => selectedIndex,
setSelectedIndex: (index) => {
// Don't update the instance again.
previousSelectedIndex = index;
selectedIndex = index;
value = getMenuItemValues()[selectedIndex];
selectedIndex = index === -1 ? 0 : index;
const menuItems = getMenuItemValues();
// Avoid setting undefined to the value
if (selectedIndex < menuItems.length) {
value = menuItems[selectedIndex];
}
},
focusMenuItemAtIndex: (index) => {
list.focusItemAtIndex(index);
Expand Down Expand Up @@ -659,4 +664,9 @@
export function getElement() {
return element;
}
function handleItemMountUnmount(event: CustomEvent<SMUIListAccessor>): void {
list = event.detail;
instance && instance.layoutOptions();
}
</script>

0 comments on commit e231b5e

Please sign in to comment.