feat(CommandPalette/InputMenu/Select/SelectMenu): unite object matching options

[J-Michalek]

🔗 Linked issue

❓ Type of change

• [ ] 📖 Documentation (updates to the documentation or readme)
• [ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
• [ ] 👌 Enhancement (improving an existing functionality)
• [x] ✨ New feature (a non-breaking change that adds functionality)
• [ ] 🧹 Chore (updates to the build process or auxiliary tools and libraries)
• [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

I found it very useful to be able to compare objects by a certain key or by custom logic as comparison by reference is not always easily achievable.

📝 Checklist

• [ ] I have linked an issue or discussion.
• [ ] I have updated the documentation accordingly.

[pkg-pr-new[bot]]

npm i https://pkg.pr.new/@nuxt/ui@5158

commit: c97ece3

[benjamincanac]

@J-Michalek This prop should also be implemented in Select, SelectMenu and InputMenu no? 🤔

[J-Michalek]

@benjamincanac Hmm there may be more to this than I initially though. The Select, SelectMenu and InputMenu components already have valueKey prop which helps with the matching of objects by certain key, we can add the by prop to them as another option for object matching.

We could also add the valueKey to the CommandPalette for the sake of consistency, what do you think?

[benjamincanac]

Indeed, adding a value-key would be better for consistency!

[J-Michalek]

@benjamincanac I've added the by prop to Select, SelectMenu and InputMenu. I also added the valueKey prop to CommandPalette, but I got stuck on a TS issue where it seems that the modelValue/defaultValue somehow conflicts with groups[number]['items'] type, but I could not figure out what is going on. When I tried to manually input the types into CommandPaletteProps the type looked correct, perhaps I am missing something regarding the generic types in a SFC?

[sandros94]

Will happily take a look tomorrow, but in general I'm not a super fun of by naming. My first reaction before reading the description and comments on this PR was like "by what? is this an alias for as?". What about a more descriptive prop like compareBy?

[benjamincanac]

The by props are overridden here to handle our types but it's inherited from Reka UI so I think keeping the same naming is the right move: https://github.com/unovue/reka-ui/blob/v2/packages/core/src/Listbox/ListboxRoot.vue#L22

[sandros94]

Pushed a commit mainly focused on ordering generics with their sequential use, but I'm noticing a strange never for items which breaks various things starting from modelValue

[benjamincanac]

@sandros94 @J-Michalek What's left to do on this?

[sandros94]

@sandros94 @J-Michalek What's left to do on this?

On my end a type improvement for the model value (which gets typed as never in some cases)

[benjamincanac]

Maybe we should split the work of CommandPalette (value-key + generics) and by prop in two different PRs? 🤔

[benjamincanac]

I've merged all the conflicts but @sandros94 I think the generics are wrong, it should be generic="T extends CommandPaletteItem[]" right? We should also be using DynamicSlots I believe 🤔

[sandros94]

I've merged all the conflicts but @sandros94 I think the generics are wrong, it should be generic="T extends CommandPaletteItem[]" right? We should also be using DynamicSlots I believe 🤔

Yes, but actually no 🥲. But yes, generics are wrong

T should actually be a union of all the groups, each with its own CommandPaletteItem[]. Other components only have nested arrays, which in typescript is far easier to handle, but in this case we have the nested array as part of a key inside an object that is part of an array. The issue I'm having (other than lack of time and work) is that I still need to objectively understand: which came first, the chicken or the egg? (aka CommandPaletteItem or CommandPaletteGroup)