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

Improve a11y of form error state v2 #2090

Merged
merged 19 commits into from
Jul 28, 2023

Conversation

MewenLeHo
Copy link
Contributor

@MewenLeHo MewenLeHo commented Jun 19, 2023

Related issues

Closes #1918

Description

The goal of this PR is to improve focus to allow sufficient contrast between error field focused and non focused.

Before this PR

Once a form is on error: fields on error will gain a red border and black one when focused
before

The main problem here is in a11y we can't convey an information by color alone because it can be not noticeable for certain types of colorblindness.
This references WCAG criterion 1.4.1 Use of Color (Level A).

For someone suffering from achromatopsia for instance, the same example will look like:
before_achromatopsia

All fields on error look almost identical, causing an a11y issue.

After this PR
The recent work on focus (see #1437) was applied to fields on error to add a visual cue so information does not rely exclusively on color anymore:
after

And for someone suffering from achromatopsia:
after_achromatopsia

Motivation & Context

A11y improvement for people suffering for any form of colorblindness.

Types of change

  • New feature (non-breaking change which adds functionality)

Live previews

Checklist

Contribution

Accessibility

  • My change follows accessibility good practices; I have at least run axe

Design

  • My change respects the design guidelines defined in Orange Design System
  • My change is compatible with responsive display

Development

  • My change follows the developer guide
  • (NA) I have added JavaScript unit tests to cover my changes
  • (NA) I have added SCSS unit tests to cover my changes

Documentation

  • My change introduces changes to the documentation and/or I have updated the documentation accordingly

Checklist (for Core Team only)

  • My change introduces changes to the migration guide
  • (NA) My new component is added in Storybook
  • (NA) My new component is compatible with RTL
  • Manually run BrowserStack tests
  • Manually test browser compatibility with BrowserStack (Chrome >= 60, Firefox >= 60 (+ ESR), Edge, Safari >= 12, iOS Safari, Chrome & Firefox on Android)
  • Code review
  • Design review
  • A11y review

After the merge

@MewenLeHo MewenLeHo added accessibility docs Improvements or additions to documentation labels Jun 19, 2023
@MewenLeHo MewenLeHo marked this pull request as draft June 19, 2023 09:14
@netlify
Copy link

netlify bot commented Jun 19, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 0eb7c22
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/64c3aaf6fbc03900089f8c59
😎 Deploy Preview https://deploy-preview-2090--boosted.netlify.app/docs/5.3/migration
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@MewenLeHo
Copy link
Contributor Author

Will be a fresh start for #1949
Needed to debug old code and find a solution for at least one Firefox issue.

@MewenLeHo MewenLeHo self-assigned this Jun 19, 2023
@@ -96,13 +96,13 @@ body {
@include focus-visible(); // 1
}

.js-focus-visible :focus:not([data-focus-visible-added]):not(.focus-ring),
.js-focus-visible .focus:not([data-focus-visible-added]):not(.focus-ring) { // 2
.js-focus-visible :focus:not([data-focus-visible-added]):not(.focus-ring):not(.form-select:invalid),
Copy link
Contributor Author

@MewenLeHo MewenLeHo Jun 20, 2023

Choose a reason for hiding this comment

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

Needed by Chrome and Firefox.

outline: 0 !important;
box-shadow: none;
}

:focus:not(:focus-visible):not(.focus-ring) { // 3
:focus:not(:focus-visible):not(.focus-ring):not(.form-select:invalid) { // 3
Copy link
Contributor Author

@MewenLeHo MewenLeHo Jun 20, 2023

Choose a reason for hiding this comment

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

Needed by Firefox.

@julien-deramond julien-deramond removed the docs Improvements or additions to documentation label Jun 20, 2023
@MewenLeHo MewenLeHo marked this pull request as ready for review June 20, 2023 13:44
@MewenLeHo MewenLeHo mentioned this pull request Jun 21, 2023
11 tasks
@louismaximepiton louismaximepiton self-requested a review June 23, 2023 12:17
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

LGTM! To me this solution seems more logical in terms of UX, so let's wait for the design review if they are fine with it.

scss/_reboot.scss Outdated Show resolved Hide resolved
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

👌

@Franco-Riccitelli
Copy link
Member

All looks good to me.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@julien-deramond
Copy link
Contributor

Screenshot 2023-07-27 at 09 22 27

Not 100% sure but I'm wondering if adding something in the migration guide could be beneficial to end-users at least to know that something (transparent) for them when they use our components has been changed. I'm thinking about when they create their own components; they should follow the same principle. Thoughts?

@julien-deramond
Copy link
Contributor

Form control file in https://deploy-preview-2090--boosted.netlify.app/docs/5.3/forms/validation/#supported-elements hasn't the outlines when focused with a mouse click.

Tried few things really quickly if it can help:

  1. Modify the mixin to add after the .form-control rule:
.form-control[type="file"] {
      @include form-validation-state-selector($state) {
        border-color: $border-color;

        &:focus {
          @include focus-visible();
          border-color: $border-color !important; // stylelint-disable-line declaration-no-important
        }
        // End mod
      }
    }
  1. Add the following 3 rules in reboot:
:not(.form-control[type="file"]:invalid)

Note: there's probably something overriding the .form-control rule because this use case should probably already be included and working 🤷

@MewenLeHo
Copy link
Contributor Author

Form control file in https://deploy-preview-2090--boosted.netlify.app/docs/5.3/forms/validation/#supported-elements hasn't the outlines when focused with a mouse click.

Tried few things really quickly if it can help:

  1. Modify the mixin to add after the .form-control rule:
.form-control[type="file"] {
      @include form-validation-state-selector($state) {
        border-color: $border-color;

        &:focus {
          @include focus-visible();
          border-color: $border-color !important; // stylelint-disable-line declaration-no-important
        }
        // End mod
      }
    }
  1. Add the following 3 rules in reboot:
:not(.form-control[type="file"]:invalid)

Note: there's probably something overriding the .form-control rule because this use case should probably already be included and working 🤷

Yes it's really strange.
It should work without the need to add the mixin specifically for .form-control[type="file"] because I already added it for all .form-control 🤔
I pushed the changes to make more tests.

@MewenLeHo
Copy link
Contributor Author

MewenLeHo commented Jul 28, 2023

@julien-deramond: as discussed in today's daily meeting, the existing rule for .form-control was not working for .form-control[type="file"] because data-focus-visible-added attribute is not added on click for input file 😮

@julien-deramond julien-deramond self-requested a review July 28, 2023 11:48
Copy link
Contributor

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Nice! LGTM! 🚀

@julien-deramond julien-deramond merged commit 17f6f4d into main Jul 28, 2023
@julien-deramond julien-deramond deleted the main-mlh-a11y-form-error-state branch July 28, 2023 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Insufficient contrast in form error state
5 participants