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

Editor checkbox value #1533

Merged

Conversation

steff-o
Copy link
Contributor

@steff-o steff-o commented May 2, 2022

Closes #1519 and fixes #1520.

Adds the possibility to add which values a checkbox should use for checked and unchecked. As a bonus the checkbox is moved to the left of the label.

checkedValue defaults to 1 (backwards compatible)
uncheckedValue defaults to 0 (backwards compatible)

It is possible to only specify one of checkedValue and uncheckedValue and make it work, but I wouldn't recommend it as for backwards compatibility the logic interprets any truthy value as checked unless uncheckedValue is specified, but on save a specific value is set.

  {
          "name": "fritext",
          "title": "Check me out",
          "type": "checkbox",
          "config": {
            "uncheckedValue": "okryzz",
            "checkedValue": "kryzza lugnt"
          }
  }

Stefan Forsgren added 2 commits May 2, 2022 08:55
Also moved checkbox label to other side of checkbox.
@johnnyblasta
Copy link
Collaborator

Tested and having some issues:

  • When adding a new feature the checkboxes are checked, shouldn't they be unchecked.
  • Leaving out the config part results in a error Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'uncheckedValue'). Would expect to not configuring specific values give the default 0 and 1 values.

@steff-o
Copy link
Contributor Author

steff-o commented May 30, 2022

  • When adding a new feature the checkboxes are checked, shouldn't they be unchecked.

They should follow the default setting. If no default is specified they should behave as before (which probably was unchecked)

  • Leaving out the config part results in a error Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'uncheckedValue'). Would expect to not configuring specific values give the default 0 and 1 values.

Probably missed that. It defaults to 0 and 1 if uncheckedValue and checkedValue are missing, but apparently it can't handle if the entire config object is missing.

I will soon return with an up-to-expectation-implementation.

@steff-o
Copy link
Contributor Author

steff-o commented May 30, 2022

@johnnyblasta I can not reproduce the crash. Can you provide a configuration example?

@johnnyblasta
Copy link
Collaborator

@johnnyblasta I can not reproduce the crash. Can you provide a configuration example?

I did like this:

{
  	"name": "typ",
  	"title": "Grill",
  	"type": "checkbox"
}

Checbox

@steff-o
Copy link
Contributor Author

steff-o commented May 31, 2022

@johnnyblasta, still not able to reproduce. Access to uncheckedValue is guarded by this thing of beauty:
checked = (obj.config && obj.config.uncheckedValue ? obj.config.uncheckedValue !== val : val) ? ' checked' : '';, which for me would make it impossible to crash as the lazy evaluation of the && operator would not evaluate obj.config.uncheckedValue Are you running your own javascript specification???

@johnnyblasta
Copy link
Collaborator

@johnnyblasta, still not able to reproduce. Access to uncheckedValue is guarded by this thing of beauty: checked = (obj.config && obj.config.uncheckedValue ? obj.config.uncheckedValue !== val : val) ? ' checked' : '';, which for me would make it impossible to crash as the lazy evaluation of the && operator would not evaluate obj.config.uncheckedValue Are you running your own javascript specification???

Sorry I seemed to introduce the bug myself while debuging the thing with the checked boxes.

Copy link
Collaborator

@johnnyblasta johnnyblasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johnnyblasta johnnyblasta merged commit a46dce2 into origo-map:master Jun 1, 2022
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.

Editor - Checkbox alignment Editor - Checkbox set value
2 participants