Tropix126 / fluent-svelte

A faithful implementation of Microsoft's Fluent Design System in Svelte.
https://fluent-svelte.vercel.app
MIT License
606 stars 26 forks source link

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

Open kaizjen opened 2 years ago

kaizjen commented 2 years ago

Before you start...

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

Tropix126 commented 2 years ago

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.