oruga-ui / oruga

🐛 Oruga is a lightweight library of UI components without any CSS framework dependency
https://oruga-ui.com
MIT License
1.13k stars 172 forks source link

Options props can be mutated - options behaviour comment in v0.9.1 #1126

Open dauriata opened 5 days ago

dauriata commented 5 days ago

Oruga version: 0.9.1

Thanks for all the work done with v0.9 ! I have some questions / comments regarding the new options api in v0.9.1

I would have expected to be able to pass any object as options, and maybe tell what should be the "value" and "label" field, and/or use slot for label. With version 0.9.1 it seems to be very specific, object are required to have a value property, if not the props are mutated by adding a "hidden" property which seems not good as it can have side effect on the rest of the app. It looks like it should also not have a key property or it else it would be overwritten.

I am also concerned how this would scale for large datasets ( maybe not for tabs or steps that would not go over 10 entries but certainly for autocomplete where it could be common to search across thousand entries ) I see that all options are rendered in the DOM even when hided, this may not be optimal. And maybe I don't get all the implication but I also think that using crypto.randomUUID is overkill, I think a simple sequence counter would do if at all needed. Why not use the index of the returning options array as key ?

holtolee commented 5 days ago

For info crypto.randomUUID break the app in Nuxt (crypto.randomUUID is not a function), no issue with 0.9.0-pre.2

mlmoravek commented 4 days ago

Hey and thanks for your feedback!!

And maybe I don't get all the implication but I also think that using crypto.randomUUID is overkill, I think a simple sequence counter would do if at all needed. Why not use the index of the returning options array as key ?

At first I used the useId native Vue composable. But the composables have the restriction of being called in the component setup context. So when a reactive hook is triggered due to an external update of the options prop, the related composable property is recalculated outside the setup context. Therfore I needed an alternative solution to useId. The first and easiest way for me was to use a native browser implementation (like crypto.randomUUID). However, I confirm your problems with nuxt here... and I see your concerns this to be an overkill. We can try to change it to a simpler sequence counter. Feel free to open a PR :)

mlmoravek commented 4 days ago

Regarding the options prop interface: I needed a way to add some internal information to the options object. However, I don't want to mutate the real options object as it was done before. So I adapted the options implementation of the FormKit library (you can see some example usecases here: https://formkit.com/inputs/select

Short explanation of the options prop type: Most components which use the options prop, the options prop can be defined as follows:

And a more detailed explanation will surely follow in the docs (see #1079).

mlmoravek commented 4 days ago

I see that all options are rendered in the DOM even when hided, this may not be optimal.

For example, with the autocomplete component, having the options already rendered and only hiding them using v-show when the string to autocomplete does not match the option is much more performant than removing them from the DOM.

pflirae commented 4 days ago

For info crypto.randomUUID break the app in Nuxt (crypto.randomUUID is not a function), no issue with 0.9.0-pre.2

related to #1105