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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[a11y]: Remove outline & border uses of ms-high-contrast #3962

Merged
merged 14 commits into from
Feb 23, 2021

Conversation

ginabak
Copy link
Contributor

@ginabak ginabak commented Feb 3, 2021

WHY are these changes introduced?

(partly) Fixes #3826 : it only "partly" fixes the issue since this PR only removes the outline and border uses of -ms-high-contrast.

Inspired by this interesting article, which states that

@media (forced-colors: active) {
  /* Styles here only apply to Forced Colors Mode */
}

basically does the same thing as -ms-high-contrast: active HOWEVER, 馃槶 it doesn鈥檛 work for firefox. See this article which talks about creating a custom media query called prefers-contrast, which is quite laborious. This is a temporary fix and will require thought as to how to account for firefox in high contrast mode.

There are still many instances of -ms-high-contrast in the Polaris repo.
See here: https://github.com/search?q=%22-ms-high-contrast%22+repo%3AShopify%2Fpolaris-react&type=code

According MDN Web Docs:

This feature is non-standard and is not on a standards track. Do not use it on production sites facing the Web: it will not work for every user. There may also be large incompatibilities between implementations and the behavior may change in the future

More on the limitations of -ms-high-contrast, here.

WHAT is this pull request doing?

Summary of your gif(s)

note: the LEFT are the changes, the RIGHT is current production, there should be no visual changes in these, keep scrolling down for visual changes

Avatar

avatar-high-contrast

Banner

banner-high-contrast

Contextual Save Bar

contextualSaveBar-high-contrast

ProgressBar

progressbar-ms-high-contrast

Toast

toast-high-contrast

Tag

tag-high-contrast

Before & Afters (minor visual changes)

SingleThumb

Before After
singlethumb-before singlethumb-AFTERFIX-high-contrast

DualThumb

(the changes are subtle, but before, there are 2 outlines, after there's only 1)

Before After
dual-thumb-BEFORE-high-contrast dual-thumb-AFTER-high-contrast

How to 馃帺

  1. Go to VM & set to High Contrast
  2. Compare side by side with http://polaris-react.herokuapp.com/?path=/story/playground-playground--playground
  3. There should be NO visual changes except with RangeSlider (Dual & Single noted in the before and after above)

馃帺 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @鈥奌YPD, @鈥妋irualves, @鈥妔arahill, or @鈥妑y5n to update the Polaris UI kit
    OUTLINE not border

@ginabak ginabak added the Accessibility Needs design, development, or content work relating to accessibility. label Feb 3, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2021

馃煛 This pull request modifies 16 files and might impact 69 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 69)
馃搫 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

馃帹 src/components/ActionList/ActionList.scss (total: 58)

Files potentially affected (total: 58)

馃帹 src/components/Avatar/Avatar.scss (total: 13)

Files potentially affected (total: 13)

馃帹 src/components/Banner/Banner.scss (total: 1)

Files potentially affected (total: 1)

馃帹 src/components/CheckableButton/CheckableButton.scss (total: 4)

Files potentially affected (total: 4)

馃帹 src/components/Frame/components/ContextualSaveBar/ContextualSaveBar.scss (total: 2)

Files potentially affected (total: 2)

馃帹 src/components/Frame/components/Toast/Toast.scss (total: 3)

Files potentially affected (total: 3)

馃帹 src/components/Modal/components/Dialog/Dialog.scss (total: 5)

Files potentially affected (total: 5)

馃帹 src/components/Navigation/Navigation.scss (total: 1)

Files potentially affected (total: 1)

馃帹 src/components/ProgressBar/ProgressBar.scss (total: 1)

Files potentially affected (total: 1)

馃帹 src/components/RadioButton/RadioButton.scss (total: 2)

Files potentially affected (total: 2)

馃帹 src/components/RangeSlider/components/DualThumb/DualThumb.scss (total: 2)

Files potentially affected (total: 2)

馃帹 src/components/RangeSlider/components/SingleThumb/SingleThumb.scss (total: 2)

Files potentially affected (total: 2)

馃帹 src/components/Tag/Tag.scss (total: 4)

Files potentially affected (total: 4)

馃帹 src/styles/foundation/_accessibility.scss (total: 0)

Files potentially affected (total: 0)

馃帹 src/styles/shared/_forms.scss (total: 0)

Files potentially affected (total: 0)

@ginabak ginabak changed the title [a11y]: Remove outline & border uses of ms-high-contrast [WIP][a11y]: Remove outline & border uses of ms-high-contrast Feb 4, 2021
@ginabak ginabak changed the title [WIP][a11y]: Remove outline & border uses of ms-high-contrast [a11y]: Remove outline & border uses of ms-high-contrast Feb 9, 2021
@ginabak ginabak marked this pull request as ready for review February 9, 2021 18:25
@ginabak ginabak requested a review from BPScott as a code owner February 9, 2021 18:25
Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

Looking good some quick comments!

Copy link
Contributor

@sarahill sarahill left a comment

Choose a reason for hiding this comment

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

Visually looks good 馃憤 Thanks for this update!

@ginabak ginabak requested review from alex-page and removed request for beefchimi February 11, 2021 21:39
@ginabak ginabak merged commit bd37c49 into main Feb 23, 2021
@ginabak ginabak deleted the gbak-remove-mshighcontrast branch February 23, 2021 17:38
sylvhama pushed a commit that referenced this pull request Mar 26, 2021
* Remove ms-high-contrast

* Replace & tophat all non-mixin border/outline ms-high-contrast

* Remove ActionList improvement for separate PR

* Add outline & unreleased

* Update UNRELEASED.md

* Revert ActionList Changes

* Remove border-top

* Remove border-color in dualThumb

* Add margin

* Remove margin, add padding to ensure no mouse flicker

* Border

* Only render border in high-contrast mode

* Temporary fix

* Fix Unreleased
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Needs design, development, or content work relating to accessibility.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[a11y]: remove all instances of -ms-high-contrast
3 participants