fluent-svelte icon indicating copy to clipboard operation
fluent-svelte copied to clipboard

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

Open whzard opened this issue 3 years ago • 1 comments

Before you start...

  • [X] Have you updated your dependencies? You might be using an old version of the library.
  • [X] 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

whzard avatar Apr 26 '22 21:04 whzard

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.

tropicaaal avatar May 02 '22 00:05 tropicaaal