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

[bug] ListApi doesn't update consistently #547

Closed
braebo opened this issue May 3, 2023 · 10 comments
Closed

[bug] ListApi doesn't update consistently #547

braebo opened this issue May 3, 2023 · 10 comments
Labels

Comments

@braebo
Copy link

braebo commented May 3, 2023

Often times, making a selection on a list blade will result in the change event being called, but the text won't properly update to show the newly selected option's text value.

This code:

const songSelect = this.folder.addBlade({
	view: 'list',
	label: 'song',
	options: songOptions,
	value: defaultValue,
	index: 0,
}) as ListApi<SongData>

songSelect.on('change', (e) => {
	rack.loadSong(e.value)          // always fires ✅
	// songSelect.value = e.value   // no effect    🔴
	// songSelect.refresh()         // non-existent 🔴
})

The problem typically goes like this:

  • Select input shows default SongA
  • I select SongB
  • 🔴 SongB loads, but the input still shows SongA as selected
  • I select SongB again
  • 🟠 SongB doesn't load again, but the input does correctly update to show SongB
  • I select SongA
  • SongA loads, and the input updates correctly.
  • I select SongB
  • 🔴 SongB loads, but the input still shows SongA as selected

I've noticed this behavior a lot, and I'm wondering if it's a bug or if it's something I'm doing wrong?

@cocopon
Copy link
Owner

cocopon commented May 7, 2023

Can you share a minimum code or an environment to reproduce the issue? It looks a bit complicated. Simplifing a problem would be helpful to figure out a cause.

@braebo
Copy link
Author

braebo commented Jun 1, 2023

Sure! Here is a minimum reproduction: https://stackblitz.com/edit/vitejs-vite-zyepfu?file=src%2Fmain.ts

If you select "Option B", the list element will still say "Option A".

@arcasoy
Copy link

arcasoy commented Jun 5, 2023

Also experiencing this issue, but seeing better performance when the .on('changed') event listener is not added, and no updating when I add the event listener.

@DarkLight1337
Copy link

DarkLight1337 commented Jun 6, 2023

I am encountering the same issue. Here is a workaround I've been using:

// Without the bug, you only need this line
listBlade.value = newValue;

// Extra logic to manually update the HTML element
const [selectElem] = listBlade.element.getElementsByTagName('select');
selectElem.selectedIndex = listBlade.options.findIndex((e) => e.value === newValue);

@braebo
Copy link
Author

braebo commented Jun 7, 2023

Came here to share the fix that @DarkLight1337 mentioned. I've found I need to run that logic once after creation if the default value's index is anything other than 0. Additionally adding the logic to the change handler seems to do the trick for subsequent changes.

@braebo
Copy link
Author

braebo commented Jun 8, 2023

I wonder if this is caused by the fact that the HTMLSelectElement converts all option values to strings internally? If objects are used as option values, they're converted to the string literal "[Object object]".

Perhaps a potential solution could be to check the options array for values of type object, and substitute them for a unique id that can be used to store and retrieve them from an internal map? This way the values passed to the html can be the id's.

Though this doesn't explain why the ListApi sometimes shows a blank box even when a default value is set and the values are strings.

@cocopon
Copy link
Owner

cocopon commented Jun 8, 2023

Thank you for sharing details! Very helpful.

This seems to be caused by converting an object to a string as you said. Here is the line:

optionElem.value = String(item.value);

The current ListBladePlugin only supports primitive values (string, number, boolean) because of its implementation. I think additional option like formatter: (value: T) => string is needed to ensure uniqueness of the stringified option values.

@cocopon cocopon added the bug label Jun 8, 2023
@cocopon
Copy link
Owner

cocopon commented Jun 10, 2023

Perhaps a potential solution could be to check the options array for values of type object, and substitute them for a unique id that can be used to store and retrieve them from an internal map? This way the values passed to the html can be the id's.

Ah, I understand your idea and its direction looks good. Trying to fix the problem...

@cocopon
Copy link
Owner

cocopon commented Jun 15, 2023

Just published the latest version 3.1.10. Could you try this version?

@braebo
Copy link
Author

braebo commented Jun 18, 2023

The repro is fixed! https://stackblitz.com/edit/vitejs-vite-zyepfu?file=src%2Fmain.ts

Awesome work!! Thanks @cocopon 🙏🏽

I'll update my larger projects later today to try this out!

@braebo braebo closed this as completed Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants