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

optionValue and optionLabel functionality fixes for Multiselect and Dropdown #4340

Conversation

dkurucz
Copy link

@dkurucz dkurucz commented Aug 28, 2023

Summary

Fixes rendering, selection, and default value behavior in Dropdown and Multiselect, especially when providing optionLabel and optionValue against complex objects. Also fixes the documented behavior of label and value functioning as the default optionLabel and optionValue mapping when the properties themselves are unspecified

Defect Fixes

#4174

@vercel
Copy link

vercel bot commented Aug 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Visit Preview Aug 28, 2023 3:28pm

@dkurucz dkurucz changed the title Feature/dmk/dropdown multiselect option value option label fixes optionValue and optionLabel functionality fixes for Multiselect and Dropdown Aug 28, 2023
@mertsincan
Copy link
Member

Hi @dkurucz,

This is a documentation issue. Sorry for the confusion! We'll fix it asap. Users can set the values of the optionLabel and optionValue properties themselves.

Best Regards,

@mertsincan mertsincan closed this Sep 5, 2023
@dkurucz
Copy link
Author

dkurucz commented Sep 5, 2023

@mertsincan could you please provide an example of the expected behavior? The pull request here does in fact update the functionalty to work as it is currently documented. The documentation as-is describes sensible behavior, the functionality just seems to have been lost over time, based on the code I am seeing. Thanks in advance!

@dkurucz
Copy link
Author

dkurucz commented Sep 5, 2023

@mertsincan I would like to request that other maintainers weigh in here. I am happy to demonstrate the problem and describe how this fixes the problem and brings the functionality back up to par with the documented behavior. I can also summarize in a more detailed post if necessary. The code is an improvement to the codebase and I would appreciate a review or explanation. Currently it is not possible to set the values of optionLabel and optionValue as you say. The resulting behavior is a broken control. Please take a look at the before and after code and you will see this is much more than a documentation issue.

Thanks!

@mertsincan
Copy link
Member

mertsincan commented Sep 5, 2023

@dkurucz These documents have been ported from PrimeReact and are therefore mistakenly included in the PrimeVue documentation. Frankly, the label-value structure is included in our other Prime* libraries, and in order not to break backward compatibility, unfortunately, we cannot leave this task solely to the user, as in PrimeVue because the label-value structure caused us many problems in other libraries. Especially regarding what the value should return to the user. Your PR returns the selected object as the value. Some users, including me, expect the value that the 'value' key has. However, some users insist that it must be an object. So now we leave it 100% to the user. If you want to set up a label-value structure, you can create a custom PrimeVue Dropdown component and use it anywhere. This would be a really great solution.

As you know, PrimeVue Dropdown also supports :options="['Option1', 'Option2']" options structure. Users can use only <Dropdown :options="['Option1', 'Option2']" /> without optionLabel and optionValue.

Exp for label-value structure;
MyDropdown.vue

<template>
   <Dropdown optionLabel="label" optionValue="value" ...>
        ...
   </Dropdown>
</template>
<script>
    ...
</script>

Test.vue

<MyDropdown value="..." />

Best Regards,

@dkurucz
Copy link
Author

dkurucz commented Sep 11, 2023

@mertsincan I am only partially following the logic here. If the documentation is correct on PrimeReact, then it should also be correct on PrimeVue as well, as functionality should translate directly. It seems to me that the React side has the ability to work with complex objects out of the box, and behaves in a singular way when returning the selected object or value. It can't be both ways on the React side, it either does one or the other. My assumption is that it does in fact work as documented, instead of requiring a custom component to abstract away properties that would otherwise be broken. My proposal is to update this PR in one of two ways:

  • update the documentation to properly indicate that complex objects supplied as options to Dropdown/Multiselect/etc will be returned as objects. As strings, the value will be returned. This way, consumers of PrimeVue will be made aware of exactly what is happening. This is already an improvement over what exists now (the documentation is both absent and incorrect!)
    OR
  • update this PR to return the value directly instead of the complex object, and update the documentation accordingly.

I would strongly suggest deferring to standardization based on the source or origin implementation as it derives this library's implementation, functionality, and documentation. This will prevent forks in behavior and cut down on long-term maintenance (such as a one-off updates to the docs as you are taking on). The precedent for backwards compatibility with third party plugins should be that Prime libraries always behave the exact same way regardless of language or platform. As it stands, third party plugins are already operating on their own assumptions of functionality that isn't documented, and moving forward with fixes to out of the box functionality in this library to make things work better and/or in a standard way shouldn't be hindered by that. I would wager that all active developers maintaining a plugin to these controls would be happy with a final and documented decision on standard behavior.

Please let me know what I can do to make this better for the Prime ecosystem, I am happy to put the time and energy into this as I believe overall this library is currently one of the best out there!

@Fravadona
Copy link

OptionLabel and OptionValue don't work with the Select component...

I'm using PrimeVue from CDN.

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

Successfully merging this pull request may close these issues.

3 participants