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

feature: multiple choice checkbox in editor #2061

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnnyblasta
Copy link
Collaborator

Closes #1518
Adds a multiple choice checkbox to the editor with a possibility to have a free text choice.
image

Configurable like this:

{
        "name": "atgardsforslag",
        "title": "Åtgärdsförslag: ",
        "type": "checkbox",
	"options": [
		"Kronhöjning minst >4.6m",
		"Kronhöjning minst >3.2m",
		"Utrymme beskärning ",
		"Borttagnings av döda grenar",
		"Återhamling",
		"Hammling",
		"Leverans högstubbe",
		"Kronretireringbeskärning",
		"Formträdbeskärning",
		"Uppbyggnadsbeskärning",
		"Säkerhetsbeskärning",
		"Avlastbeskärning",
		"Borttagnings av vatten skott",
		"TRAQ 2",
		"TRAQ 3; Klätterandebesiktning",
		"TRAQ 3; Resistograph/tomograph",
		"TRAQ 3; Advencerad Rotbesikning",
		"Annat:textbox"
	]  
}

@steff-o
Copy link
Contributor

steff-o commented Nov 15, 2024

Even most of the stuff works as expected I'm not convinced.

I don't think that it should be called checkbox. It feels more like a new input type like "checkedlistbox" or "checkboxlist" or "checkboxgroup", with possibility to add extra values. Or a more user friendly dropdown/listbox with multi select. A checkbox with a list of options feels kind of awkward to me.

But my main concern is about the additional options that the user can add.

  1. The configuring syntax: it is a bit magic and limits the possible options values. Not that it is likely that someone will have an option that actually contains ":textbox", but still.
  2. While the configuration theoretically supports several "other" options, the implementation only fully supports one. It is possible to add several "other" values, but when re-read only the last is used for all "other" checkboxes.
  3. The configured "other" title ("Annat" in your example) does not enable localization the same way as other controls as it is hardcoded in the configuration.
  4. It is easy to trick the mechanism by using semicolon i an "other" value. If a semicolon is present in the options list it also breaks, but that can be avoided in configuration time, but in "other" fields it should be escaped.

My suggestion is to omit the possibility to configure the use of "others" in the options list and instead have a flag in a "config" object that says if it is allowed to add "other" values, and possibly one or many. If many "others" are allowed a new "other" row is created when the one visible is checked. On re-read, all options from database that are not in the options list are added as pre filled "others", or like the regular options.

It should also be possible to configure the separator, now hardcoded to semicolon.

Other than that, there is a bug when using this feature in related tables as the input fields are not getting unique ids. It is however a result of a bug in the existing code for checkbox that does not include layer name in the id.

@johnnyblasta
Copy link
Collaborator Author

I think I prefer checkboxgroup if it should be a new input type.
I can take a look at reworking it, so it's less hokus pokus in the implementation.

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.

Multiple values in editorform
2 participants