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

Question about "looseIndexOf" in ui-select #451

Open
macgyver opened this issue Jul 19, 2019 · 5 comments
Open

Question about "looseIndexOf" in ui-select #451

macgyver opened this issue Jul 19, 2019 · 5 comments

Comments

@macgyver
Copy link

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
Copy link
Owner

JosephusPaye commented Jul 20, 2019

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

macgyver commented Jul 31, 2019

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
Copy link
Owner

Makes sense, will add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants