Skip to content

Should checkboxes support true/false? #114

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

Open
8 tasks
nkylstad opened this issue Jun 25, 2021 · 13 comments
Open
8 tasks

Should checkboxes support true/false? #114

nkylstad opened this issue Jun 25, 2021 · 13 comments
Labels
kind/user-story Used for issues that describes functionality for our users. ux Needs some love from a UX resource

Comments

@nkylstad
Copy link
Member

Description

We had an incident with an app with several checkbox-components, each with only 1 option to check. The data model fields were all booleans, so that checking the checkbox set the value to true. App: digdir/godkjenn-bruksvilkaar.

However, after this change (from january 2021) Altinn/altinn-studio@0bd246a#diff-df4b204405101ca09583792727c63870fcf2a486381a2575ca8eb334a33f2660, checkbox components no longer support toggling true/false. They only support toggling either options-value, or no value.

In the specific incident mentioned above, this caused problems because the underlying data model fields were not nullable booleans, and when the data model was initialized, they were automatically set to false. Checking the checkbox then resulted in a value false,true, since toggling between true/false is not supported. This results in a validation error, since false,true is not a boolean value.

We need to decide if toggling true/false is behaviour we would expect from a checkbox, and if so, implement support for it.

Screenshots

InkedFeil i bruksvilkår_app (002)_LI

Considerations

Is togglig between true/false behaviour we expect from a checkbox? Or is this better covered by f.ex. a radiobutton (Yes/No), where the user actively has to choose? Is togglig between value/no value as we do now sufficient for a checkbox?

Acceptance criteria

Describe criteria here (i.e. What is allowed/not allowed (negative tesing), validations, error messages and warnings etc.)

Specification tasks

  • Development tasks are defined

Development tasks

Add tasks here

Test

Add test cases here as checkboxes that are being tested as part of the changes.

Definition of done

Verify that this issue meets DoD (Only for project members) before closing.

  • Documentation is updated (if relevant)
    • Technical documentation (docs.altinn.studio)
    • User documentation (altinn.github.io/docs)
  • QA
  • Manual test is complete (if relevant)
  • Automated test is implemented (if relevant)
  • All tasks in this userstory are closed (i.e. remaining tasks are moved to other user stories or marked obsolete)
@nkylstad nkylstad added kind/user-story Used for issues that describes functionality for our users. ux Needs some love from a UX resource solution/app-frontend labels Jun 25, 2021
@mjulstein
Copy link
Contributor

Checkboxes have 3 states. How we render them is something else...
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#indeterminate

@altinnadmin
Copy link
Member

We need to decide if toggling true/false is behaviour we would expect from a checkbox

For simple checkboxes that is mapped to a boolean data element I guess it makes sense to support this kind of toggling. I would have expected us to handle this.

@nkylstad
Copy link
Member Author

nkylstad commented Jun 28, 2021

Checkboxes have 3 states. How we render them is something else...
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#indeterminate

@mjulstein Since we use Material-UI's checkbox components and don't implement the <input>-element directly, this is something that have to assume this is covered in the material-ui implementation.

The question here is whether unchecking a checkbox should be able to toggle a value (false). My initial thought was that no, this should not be something we support, but since this is something that we did (maybe inadvertently) support at one point, we need to do a consideration. Reading more in the docs you linked about the standard checkbox implementation, I still think that this is not functionality that should be covered by checkboxes, since the value that is submitted by a checkbox should only be included if the checkbox is checked.

For simple checkboxes that is mapped to a boolean data element I guess it makes sense to support this kind of toggling. I would have expected us to handle this.

@altinnadmin To support this we would have to implement custom handling of that specific case. If the user does not check the checkbox, then the value would never be set, and to get to false user would have to check, then uncheck the checkbox. (Unless the field in the datamodel, incl. any parent groups, is set as required in the data model, then data would be instantiated to false by default.) Wouldn't radio buttons with yes/no be a more logical choice in that case?

@altinnadmin
Copy link
Member

If the user does not check the checkbox, then the value would never be set, and to get to false user would have to check, then uncheck the checkbox.

Yes, if a default value has not been set. If we look at this from a "data point of view", it makes more sense to be able to toggle a boolean field between true/false than true/empty. Checkboxes still are "the original true/false-togglers" in UI :)

But I see your point, just fearing this will be perceived as a lack of flexibility, that we're "forcing" the use of radio buttons on devs.

@Hannott
Copy link

Hannott commented Jul 27, 2022

In my mind this is asking if a thing should do what it was made to do. While it technically can have three states, in practice it's either true or false, and null should be treated as false, or you should be using a different component.

The component as it is now should be renamed "multiple choice checkboxes", and the standard checkbox component should work like a standard checkbox, true/false

@olemartinorg
Copy link
Contributor

Highly relevant issue here, and possibly the reason booleans are not supported properly in app-frontend:

Fixing this properly is most likely a (very) breaking change, so this should be considered in #339

@FinnurO
Copy link

FinnurO commented Nov 17, 2022

It seems clear that, according to the HTML specification https://html.spec.whatwg.org/#checkbox-state-(type=checkbox), we should only support true/false and not a third state.

So the tasks should be to follow the standard and implement boolean support as commented.

@mjulstein
Copy link
Contributor

@FinnurO It is true that the spec only has two states, but the indeterminate IDL attribute, should possibly used when the user has to make a decision and has not yet done so?

@mjulstein
Copy link
Contributor

I think 2 states for the checkbox. A toggle is a radiobitton.
There is an initial undefined state that can be in store though, if the user is either answering false or has not answered (undefined) the check can be unchecked. If the answer is true it is checked.
The html component has only 2 states, but the form in redux can have as many as we want.

@SimenRokaas
Copy link
Contributor

I currently got a change request to replace radio buttons for a yes/no boolean field with a checkbox, and to my big surprise I find that it is not possible?!

I totally agree with @Hannott above. Not supporting a simple yes/no boolean field as a checkbox seems very strange! Please do implement this 🙏

@olemartinorg
Copy link
Contributor

olemartinorg commented Jan 6, 2023

I also agree, that's not the problem. Currently, app-frontend-react does not support any other data types than strings in the data model, so before we can have booleans anywhere we need to solve #205. As that's a breaking change, we would need to release a new major version (because manually testing your app is needed, and possibly adjusting custom code/backend in the process). Work has started, but it's currently not progressing quite as much as we'd like, as there are other tasks we need to complete in order to make that change manageable to implement (such as the one I'm working on right now, #377). In the end, when we get there, the plan is to implement the use-case for a single boolean-bound checkbox in #681.

EDIT: And just to make it clear why I think this issue is still relevant, when a simple yes/no checkbox is meant to be implemented in #681; it could be possible/wanted/needed to bind multiple checkboxes to booleans as well, but then only as an object supporting boolean values, such as:

{
  "alternative1": true,
  "alternative2": false,
}

That would be a data structure suitable for binding the Checkboxes component to booleans (as that component can have more than one checkbox in them).

@altinnadmin
Copy link
Member

@olemartinorg Is there a reason why we can't get non-breaking #681 out of the door quickly, to solve most simple cases out there and take away "heat" from this issue?

@olemartinorg
Copy link
Contributor

I guess we could try, but it depends on how the backend/platform handles our broken serialization. Right now, everything we send from app-frontend to the backend (app-lib-dotnet) is strings inside JSON. When the backend converts that to XML in some way, I guess the JSON types don't really matter - but as long as the strongly typed C# backend types a boolean as an actual boolean (not a stringy 'true' | 'false'), I suspect more and more things will subtly break on the backend if we put the horse before the cart on this one.

Right now, the progress on our app-frontend v4 (that, among other things, aims to introduce support for multiple types) is moving slower than I'd like. It's the same story as always; more urgent matters get prioritized, and it's difficult to see the long-term benefits of such structural changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/user-story Used for issues that describes functionality for our users. ux Needs some love from a UX resource
Projects
Status: No status
Development

No branches or pull requests

8 participants