search

JosephusPaye / Keen-UI

A lightweight Vue.js UI library with a simple API, inspired by Google's Material Design.
https://josephuspaye.github.io/Keen-UI/
MIT License
4.1k stars 438 forks source link

Question about "looseIndexOf" in ui-select #451

Open macgyver opened 5 years ago

macgyver commented 5 years ago

Since there is already a key specified for the options, would it not make more sense to just compare the keys of the objects in the isOptionSelected method?

JosephusPaye commented 5 years ago

Hi @macgyver,

That's a good point, the component could use keys from options if it's available (not always, as keys is optional).

For cases where it's not available, we need to keep looseIndexOf, or something similar, since comparing objects directly by reference could have false negatives.

macgyver commented 5 years ago

It's optional as a prop, but it seems like the keys are always defined, with sensible defaults: https://github.com/JosephusPaye/Keen-UI/blob/master/src/UiSelect.vue#L232

JosephusPaye commented 5 years ago

That's true, but the problem with depending on that is that when the user sets the keys prop, it overrides the defaults, so if they don't specify all the keys, e.g. 

{ class: 'my-class', image: 'my_image_field' }

you end up with no id key.

This might be less of an issue in practice though - so we can check if keys.id is available and use that to compare, falling back to looseIndexOf if not.

macgyver commented 5 years ago

Ah, I didn't understand how the defaults was being used - I assumed incorrectly that any specified keys would be assigned over the defaults. At any rate I think you're right, checking for the presence of the value key specifically before using it for comparison should yield the desired effect.

If you're curious about my use-case, I'm storing the selected results of the ui-select as an object but weeks later when I go to re-populate the select the label of any of the options may have been edited (only the value is immutable). I could update my selected value object manually before sending it to the ui-select so that it matches up perfectly with one of the options, which is how I'm getting around this right now, but I think it will be slightly faster to do it in the UI framework, and would also help if somehow the key order got mixed up but the shape of the object was still the same.

JosephusPaye commented 5 years ago

Makes sense, will add.