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

Checkbox content type field lists "values" property but only responds to "value" #22049

Closed
jdcmsd opened this issue Apr 19, 2022 · 5 comments · Fixed by #23478
Closed

Checkbox content type field lists "values" property but only responds to "value" #22049

jdcmsd opened this issue Apr 19, 2022 · 5 comments · Fixed by #23478

Comments

@jdcmsd
Copy link
Contributor

jdcmsd commented Apr 19, 2022

Describe the bug
When defining a content type, the Checkbox field has a values property that can only be accessed by calling .value instead of .values.

To Reproduce
Steps to reproduce the behavior:

  1. Build content of any content type that includes a Checkbox
  2. Include the following in its container, replacing checkboxVar where appropriate:
<ul>
    <li>Raw Output: $content.checkboxVar</li>
    <li>Options: $content.checkboxVar.options</li>
    <li>Values A: $content.checkboxVar.values</li>
    <li>Values B: $content.checkboxVar.value</li>
    <li>Selected Values:</li>
        <ul>
        #foreach($value in $content.checkboxVar.selectedValues) 
           $value#if($foreach.hasNext()), #end
        #end
        </ul>
</ul>
  1. The first line's output will look something like this (emphasis mine):

Raw Output: com.dotcms.rendering.velocity.viewtools.content.CheckboxMap@158b5751[options=[Option 1, Option 2, Option 3],values=[1, 2, 3],selectedValues=[1]]

  1. Despite this, the .values call will fail, while the .value call will succeed.

Expected behavior
Expected .values call to work and .value call to fail.

Desktop

  • OS: macOS Monterey 12.3.1
  • Browser: Chromium (Brave)
  • Version: 100.0.4896.127

Additional context
.values would better match the object's plural .options and .selectedValues properties, along with the data it prints when called directly. The documentation also listed .values up until today; I adjusted it to match the demonstrated behavior, but I will change it back if that behavior changes.

Could we check whether it was ever .values, perhaps changing some time recently? If I'm wrong and it's always been .value, then perhaps instead the output of $content.checkboxVar should be adjusted to reflect this, and to better maintain backwards compatibility.

E: Looks like it's been this way for years, at least. It'd be a breaking change, albeit a very small and easily-addressed one. Either:

  • A) End users will need to adjust checkbox calls from .value to .values
  • B) We can implement both .value and .values

Solution is trivial in all cases; just need to figure out our preference/principle.

@stale
Copy link

stale bot commented Dec 1, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 1, 2022
@jdcmsd
Copy link
Contributor Author

jdcmsd commented Dec 1, 2022

Thanks to the bot's reminder, I took a quick look and figured out a solution to this. PR will follow.

@bryanboza
Copy link
Member

We need internal QA here!

@dsilvam
Copy link
Contributor

dsilvam commented Feb 1, 2023

Passed Internal QA: Both .value and .values returning the expected values.

@bryanboza
Copy link
Member

Fixed, tested on release-23.02 // Docker // FF

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

Successfully merging a pull request may close this issue.

5 participants