Page MenuHomePhabricator

Checkbox: implement the error state
Closed, ResolvedPublic1 Estimated Story Points

Assigned To
Authored By
bmartinezcalvo
Jun 22 2023, 2:01 PM
Referenced Files
F37808860: checkbox-err.gif
Sep 27 2023, 9:45 AM
F37806672: image.png
Sep 26 2023, 9:43 PM
F37805549: image.png
Sep 26 2023, 4:02 PM
F37805541: image.png
Sep 26 2023, 4:02 PM
F37730348: ezgif.com-video-to-gif (2).gif
Sep 18 2023, 9:43 AM
F37730347: Grabación de pantalla 2023-09-18 a las 11.35.25.mov
Sep 18 2023, 9:41 AM
F37730345: ezgif.com-video-to-gif (1).gif
Sep 18 2023, 9:40 AM
F37730342: Grabación de pantalla 2023-09-18 a las 11.35.25.mov
Sep 18 2023, 9:39 AM

Description

Background

The error state in the Codex Checkbox component is missing. We need to implement the following missing states:

Unselected:

  • Error unselected
  • Error-hover unselected (new token border-color-error--hover is needed)
  • Error-active unselected (new token background-color-error--active is needed)
  • Error-focus unselected

Selected:

  • Error selected (new token background-color-error is needed)
  • Error-hover selected (new token background-color-error--hover is needed)
  • Error-active selected (new token background-color-error--active is needed)
  • Error-focus selected

Captura de pantalla 2023-09-08 a las 12.38.34.png (1×2 px, 125 KB)

The error state should be interactive like the non-error state. So we will need to create the following new color tokens to use in these error states:

  • background-color-error
  • background-color-error--hover
  • background-color-error--active
  • border-color-error--hover

Known use cases

Captura de pantalla 2023-06-22 a las 16.34.26.png (698×698 px, 73 KB)
Checkboxes in the T&S tools project
  • Here is a link to the Figma file where the Incident Reporting System is using check boxes in error state

User stories

  • As a user I need to use the Checkbox error state when checkbox selection is missing in a mandatory form field.

Previous implementations

  • Codex demo: Checkbox component in Codex
  • Design style guide: Checkbox guidelines in the DSG
  • OOUI: Checkbox component in OOUI
  • Vue: /-

Design spec

Open questions

Add here the questions to be answered in order to design and implement the component

Acceptance criteria (or Done)

Design

  • Design the Figma spec sheet and add it in this task
  • Add new color tokens as styles in the Figma library and use them to style the checkbox error states:
    • background-color-error
    • background-color-error--hover
    • border-color-error--hover
    • background-color-error--active
  • Update the component in the Figma library. This step will be done by a DST member.

Code

  • Create the new color tokens:
    • background-color-error
    • background-color-error--hover
    • border-color-error--hover
    • background-color-error--active
  • Update the component in Codex

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Hey @bmartinezcalvo, I'm going to add a mirror ticket to our sprint board and we will take a look at our next meeting to decide if/when we can get to it! I'll get back to you.

Hey @bmartinezcalvo, I'm going to add a mirror ticket to our sprint board and we will take a look at our next meeting to decide if/when we can get to it! I'll get back to you.

@JKieserman great, thank you!

@bmartinezcalvo @CCiufo-WMF is this something that DST has capacity to work on in the near term?

@bmartinezcalvo @CCiufo-WMF is this something that DST has capacity to work on in the near term?

The Figma spec sheet was completed so this task is ready to be implemented once the DST or other contributor has time to do that.

@bmartinezcalvo @CCiufo-WMF is this something that DST has capacity to work on in the near term?

If the Trust & Safety Tools team doesn't plan to contribute this change as part of the Incident Reporting System work (T340197), but still needs the error state, DST can probably make time to add it.

@bmartinezcalvo @CCiufo-WMF is this something that DST has capacity to work on in the near term?

If the Trust & Safety Tools team doesn't plan to contribute this change as part of the Incident Reporting System work (T340197), but still needs the error state, DST can probably make time to add it.

I am supportive of DST taking it on, given T&ST's reduced capacity at the moment. Thank you!

CCiufo-WMF triaged this task as Medium priority.Aug 23 2023, 2:16 PM

Revisiting this use case, should this not actually be a Codex Fieldset, in which case error states are already supported?

Revisiting this use case, should this not actually be a Codex Fieldset, in which case error states are already supported?

I was wondering about our (lack of use of) fieldsets in the application we're building. Let's try that. The documentation also says:

When building a Checkbox field, always set isFieldset to true, even for a single Checkbox

I was wondering about our (lack of use of) fieldsets in the application we're building. Let's try that. The documentation also says:

When building a Checkbox field, always set isFieldset to true, even for a single Checkbox

Exactly! That being said, I think we do still need to implement the error state on the checkbox itself (the red outline). DST discussed that the same is probably needed for Radio groups.

Also as part of this, we should evaluate whether an identical ticket is needed for radio groups.

Change 954339 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] Checkbox: implement the error state

https://gerrit.wikimedia.org/r/954339

Hey! I updated the documentation to include the Checkbox with error state. You can view it here.

Let me know your thoughts on the current implementation @bmartinezcalvo

Right now when the Checkbox is in an error state and checked:

  • if Checkbox is focused, the focus styles override the error styles
  • when Checkbox is unfocused, the error border styles are displayed on the checked Checkbox

checkbox-with-error.png (106×130 px, 5 KB)

lwatson changed the task status from Open to In Progress.Sep 2 2023, 12:17 AM

Hey! I updated the documentation to include the Checkbox with error state. You can view it here.

Let me know your thoughts on the current implementation @bmartinezcalvo

Right now when the Checkbox is in an error state and checked:

  • if Checkbox is focused, the focus styles override the error styles
  • when Checkbox is unfocused, the error border styles are displayed on the checked Checkbox

checkbox-with-error.png (106×130 px, 5 KB)

@lwatson as documented in the Figma spec, the error state is only valid on unselected checkboxes. This is because checkbox with error could only appear if the user doesn't check any of the checkbox items in the list when a checkbox group is mandatory in a field. So I don't view any need for the selected checkbox error.

Captura de pantalla 2023-09-04 a las 11.36.36.png (660×2 px, 140 KB)

Regarding the error-focus, it will be as in other form items where the base focus override the error state, so the error will be visible just when non focus.

Captura de pantalla 2023-09-04 a las 11.39.09.png (408×1 px, 36 KB)

Apart from this, could we include the error state in the configurable demo instead of creating a separate demo at the end of the page? As we do in TextInput where we display the prop status default or error. Also, we could include the prop disabled if possible so that we can check all the checkbox states in the same demo.

@bmartinezcalvo thank you for clarifying that the error state is needed for unselected checkboxes only. I'll update the styles and then add a configurable demo.

@bmartinezcalvo thank you for clarifying that the error state is needed for unselected checkboxes only. I'll update the styles and then add a configurable demo.

@lwatson great, thank you!

@bmartinezcalvo Hey - right now, the error styles are only applied to an unchecked & unfocused Checkbox (see Netlify link).

We (engineers) discussed implementing an error state for a checked Checkbox and the potential problems that can occur if we restrict this usage. I found an example of a checked Checkbox with error when making an invalid selection.

Is a checked Checkbox with an error state not recommended or strongly against UX guidelines?
If it's only recommended against using then we can implement error status for both checked and unchecked and add guidelines to the Checkbox.

@bmartinezcalvo Hey - right now, the error styles are only applied to an unchecked & unfocused Checkbox (see Netlify link).

We (engineers) discussed implementing an error state for a checked Checkbox and the potential problems that can occur if we restrict this usage. I found an example of a checked Checkbox with error when making an invalid selection.

Is a checked Checkbox with an error state not recommended or strongly against UX guidelines?
If it's only recommended against using then we can implement error status for both checked and unchecked and add guidelines to the Checkbox.

@lwatson as I commented before, the only use case I find for an error (unselected) checkbox is when selecting a checkbox is mandatory, for example:

  1. When you need to check at least one of the checkbox items within a checkbox list in a form
  2. When you need to accept the terms and conditions checkbox

But I can't identify a real scenario for an error-selected checkbox, as any option in the list would be acceptable, and displaying any of them as incorrect choices to the user wouldn't make sense.

@bmartinezcalvo we thought of a the case of a checkbox group where the user is required to check 2 or more boxes. When 1 box is checked and the form is submitted, the checkbox group should show an error state.

Of course, in this state, you could only show the error state for the unchecked checkboxes, but the current behavior of the Field component is to pass the error state of the Field down to all of its inputs, and we should try to make this behavior consistent for all input types.

We're also concerned about potential use cases in the future that we haven't thought of yet - maybe there's a case where a selected checkbox is invalid because of another condition in the form. Forms can also have hidden checkbox inputs, which could affect single checkboxes or checkbox groups in ways we don't expect. For this reason, we thought it would be better to enable users to set the error state on the checkbox any time they need to, not just when the box is unchecked.

This is why we'd like to differentiate between whether showing an error state on a checked Checkbox is:

  • Something we don't expect to happen, or
  • Something that absolutely should never happen because it's bad UX

I think it's the first one, and in that case, it's probably better to go ahead and allow checked (and indeterminate) checkboxes to show an error state as well. Let me know what you think!

We're also concerned about potential use cases in the future that we haven't thought of yet - maybe there's a case where a selected checkbox is invalid because of another condition in the form. Forms can also have hidden checkbox inputs, which could affect single checkboxes or checkbox groups in ways we don't expect. For this reason, we thought it would be better to enable users to set the error state on the checkbox any time they need to, not just when the box is unchecked.

@AnneT @lwatson I agree that, from a design system perspective, it would be helpful to have both unselected and selected error states ready to be used in case they are needed in the future. So I've updated the Figma spec sheet to include all of them:

Unselected:

  • Error unselected
  • Error-hover unselected (new token border-color-error--hover is needed)
  • Error-focus unselected

Selected:

  • Error selected (new token background-color-error is needed)
  • Error-hover selected (new token background-color-error--hover is needed)
  • Error-focus selected

Captura de pantalla 2023-09-08 a las 12.38.34.png (1×2 px, 125 KB)

The error state should be interactive like the non-error state. So I have included hover and focus states for both statuses. We will need to create the following new color tokens to use in these error states:

  • background-color-error
  • background-color-error--hover
  • border-color-error--hover

@bmartinezcalvo we thought of a the case of a checkbox group where the user is required to check 2 or more boxes. When 1 box is checked and the form is submitted, the checkbox group should show an error state.

@AnneT although we finally create the selected checkbox error state, I think this is the right solution for this case:

Captura de pantalla 2023-09-08 a las 12.42.52.png (1×2 px, 244 KB)

When it is mandatory to select two options in a checkbox group, if the user selects only one option, the remaining items will be unselected, resulting in an error. The selected checkbox should remain non-error.

@AnneT thanks for highlighting cases where a checked Checkbox can result in an error state!

@bmartinezcalvo thanks for updating the spec sheet with these additional error states! I'll work on updating the component today.

@bmartinezcalvo Hey, what would you suggest for styling the unchecked Checkbox in an error, hover, and focus state? Do you prefer the hover to win over the focus styles or vice-versa?

This is the "error-hover-and-focused unselected" Checkbox in its current state:

Screenshot 2023-09-08 at 6.10.26 PM.png (357×1 px, 80 KB)

I updated the Checkbox with error, hover, & focus to show only the focus styles (Netlify link). This is similar to how it's handled in TextInput.

Before:

Screenshot 2023-09-08 at 5.43.19 PM.png (133×270 px, 8 KB)

After:

Screenshot 2023-09-11 at 9.22.30 AM.png (458×689 px, 30 KB)

I updated the Checkbox with error, hover, & focus to show only the focus styles (Netlify link). This is similar to how it's handled in TextInput.

Before:

Screenshot 2023-09-08 at 5.43.19 PM.png (133×270 px, 8 KB)

After:

Screenshot 2023-09-11 at 9.22.30 AM.png (458×689 px, 30 KB)

@lwatson this is great. Let's follow the same solution as in other system components.

@lwatson I've reviewed the last patch and all new error states look good.

@bmartinezcalvo The error styles in the latest patch include unchecked, checked, and indeterminate Checkbox. Lmk what you think!

Change 954339 merged by jenkins-bot:

[design/codex@main] Checkbox: Implement the error state

https://gerrit.wikimedia.org/r/954339

Change 956973 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.18.0 to v0.19.0

https://gerrit.wikimedia.org/r/956973

Change 956973 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.18.0 to v0.19.0

https://gerrit.wikimedia.org/r/956973

This has been included in the latest release, but I'm leaving the task open in case there are any design adjustments needed which would warrant a follow-up patch. @bmartinezcalvo feel free to close this task if you are happy with the results.

Design sign-off done. I've detected that we are not using the right styles for the error-active states. They should use a new background-color-error--active (color-red700) token to follow the same behavior of the non-error active where the checkbox is filled in active color when pressed. To review the behavior, please check the Figma prototype.

Captura de pantalla 2023-09-14 a las 13.45.52.png (616×2 px, 123 KB)

@lwatson moving the task back to In Progress to fix this.

Change 957839 had a related patch set uploaded (by LWatson; author: LWatson):

[design/codex@main] Checkbox: Add error-active state

https://gerrit.wikimedia.org/r/957839

@bmartinezcalvo Hey, I implemented the error active styles and added the new design token in this patch. Let me know if anything else is needed!

@bmartinezcalvo Hey, I implemented the error active styles and added the new design token in this patch. Let me know if anything else is needed!

@lwatson thank you! I'm reviewing the last patch, the new error-active token and the states have been implemented successfully. I only thing detect one thing to fix: when you press the error checkbox, a blue line appears around the error-active state.

ezgif.com-video-to-gif (2).gif (403×600 px, 321 KB)

@bmartinezcalvo should I remove the hover and focus border styles when the Checkbox is in an error-active state?

@bmartinezcalvo should I remove the hover and focus border styles when the Checkbox is in an error-active state?

@lwatson we should use the same solution as in other components. Not sure if these states are hidden when active, but they shouldn't visually affect to the active state.

@bmartinezcalvo Thanks! The active-error styles are consistent with the active-default styles in Checkbox. Therefore, hover and focus styles will not be visible when active.

@bmartinezcalvo Thanks! The active-error styles are consistent with the active-default styles in Checkbox. Therefore, hover and focus styles will not be visible when active.

@lwatson great, thank you. I'm checking that you already removed them in the patch. So all looks good to me now in case you want to solve this task and/or move to Pending Release.

@bmartinezcalvo thanks for reviewing and my apologies - the code hasn't been approved so it can remain in the code review column.

Change 957839 merged by jenkins-bot:

[design/codex@main] Checkbox: Add error-active state

https://gerrit.wikimedia.org/r/957839

Hiya, capturing some feedback/suggestions that were proposed in design review:
Firstly could the demo be improved by showing the error state with the error message, and also included in documentation.

Currently, no error text
image.png (762×1 px, 79 KB)
Proposed
image.png (812×1 px, 79 KB)

Secondly, it would be helpful to include use-cases in the documentation for when an error state is required.

Thirdly, a question is how the error text appears in cases when there is an error on an inline checkbox, esp. in the second or subsequent checkbox in the inline group?

@RHo I think including an error message in the configurable demo implies that the message is part of the checkbox component. We don't include an error message in the configurable demo for anything except for Field, where the validation message is implemented.

Given we need further explanation of when you might actually want validation on checkboxes, what if we add a new standalone demo to the Checkbox page to show when you might need an error state, with a working example that uses the Field component and has a validation message?

For your third question: we don't really support inline validation messages for checkboxes, although you could wire this up by wrapping each checkbox in its own Field component. However, the message would display below the checkbox, not inline.

@RHo I think including an error message in the configurable demo implies that the message is part of the checkbox component. We don't include an error message in the configurable demo for anything except for Field, where the validation message is implemented.
Given we need further explanation of when you might actually want validation on checkboxes, what if we add a new standalone demo to the Checkbox page to show when you might need an error state, with a working example that uses the Field component and has a validation message?

Hi @AnneT - I added this on the fly during the design review meeting and realise now that it may be best for @bmartinezcalvo to address in a separate follow up task but think what you suggest makes sense: to have a separate demo with validation message that also provides more details about usecases for error on checkboxes.

For your third question: we don't really support inline validation messages for checkboxes, although you could wire this up by wrapping each checkbox in its own Field component. However, the message would display below the checkbox, not inline.

I was wanting to check whether the expectation is for cases where there is an error on an inline checkbox which is not the first in a group, if the message would appear below the group (as in something like A in the image below), or would it be 'attached' to the specific checkbox (as in B):

image.png (424×1 px, 44 KB)

Alternatively wonder if we would in general not recommend/allow error states on checkboxes inline groups? One for @bmartinezcalvo to weigh in on

I always imagined that the primary use-case of a checkbox error state would be if an entire form field had error status, and that was propagated to individual checkbox components.

The classic example of this would be some kind of a sign-up form, where there is a checkbox that says something like I have read and agreed to the terms and conditions. The checkbox would not show the error state initially, but if the user attempted to submit the form without checking it, the checkbox error state could be used to draw their attention to the required element.

But the error message in this scenario would still live at the level of the entire Field. Having a stand-alone checkbox appear in error state with an inline error message seems strange to me.

In case it's interesting to the discussion, here is how we are implementing the error state in ReportIncident currently (patch), cc @Dreamy_Jazz

checkbox-err.gif (631×526 px, 705 KB)

Change 961452 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update Codex from v0.19.0 to v0.20.0

https://gerrit.wikimedia.org/r/961452