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

<ComboBox> fires select event every time an object in items is changed #44

Open
2 tasks done
kaizjen opened this issue Apr 26, 2022 · 1 comment
Open
2 tasks done
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@kaizjen
Copy link
Contributor

kaizjen commented Apr 26, 2022

Before you start...

  • Have you updated your dependencies? You might be using an old version of the library.
  • Have you checked for an existing issue on this topic? If there is one, post a comment on it instead.

What browsers are you seeing the problem on?

Other - list in description

Description

<ComboBox> will fire its select event if the items prop is updated in the way that replaces a previous object with a new one, as opposed to just mutating the old object.

Consider the following code:

<script>
	import { ComboBox, Button } from 'fluent-svelte'

	let items = [
		{ name: 'first', value: 0 },
		{ name: 'second', value: 1 },
		{ name: 'minute', value: 2 },
	]
</script>

<ComboBox on:select={console.log} value={0} />
<Button on:click={() => { items[2] = { name: 'minute', value: 2 }; items = items; }}>Change 'minute' to 'third'</Button>

The console.log actually fires when you press the button!

P.S. I tested it only in Opera, but I'm sure this bug is not browser-specific

Steps To Reproduce

No response

Expected behavior

This behavior is very counter-intuitive, because the select event should not fire with the same value of item (nothing new has been selected). I'm guessing the code checks aboutToBeSelectedItem === previousItem but what should matter here is only the value of item! The way items array should be filled (by users of the library) is questionable by itself: #25

Also, I think that it would be easier if the select event fired only when the user selected an item. That would make more sense, because all the changes of the value prop can be handled by the bind: directive

Relevant Assets

No response

@kaizjen kaizjen added the bug Something isn't working label Apr 26, 2022
@Tropix126
Copy link
Owner

Yeah I think this is because i'm using a reactive statement here. This might affect other components as well, will look into it for the next release.

@Tropix126 Tropix126 added the good first issue Good for newcomers label May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants