[Tabs][a11y]: Fix a11y keyboard interaction (when focus overrides selected state) #3669

ginabak · 2020-11-30T23:40:12Z

WHY are these changes introduced?

Fixes #3638 (when selected Tab is focused (on keyboard), it has a grey underline instead the primary green)

WHAT is this pull request doing?

A11y related: when auditing online-store-ui's PanelCard component (see Initial Audit #2291) we noticed that during the manual keyboard testing, when the selected tab is focused, the ::before underline was grey instead of the primary green --p-action-primary colour.

BEFORE

AFTER

Please note the following:

the changes follow the pre new design language traits:
- focus state is a lighter hue from the selected state instead of grey (pre changes)
- the grey underline is only during a hover state

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Test new changes after pulling branch in this playground (post dev s)

🎩 checklist

Tested on mobile
- tested on Google Pixel 2 XL (with TalkBack on Android)✅
Tested on multiple browsers
- tested on Windows10 Edge ✅
- tested on FireFox ✅
- tested on Chrome ✅
- tested on Safari ✅
Tested for accessibility
- tested Voiceover on MacOS (Safari) ✅
- tested JAWs on Windows (Chrome) ✅
- tested Windows High Contrast (Edge) ❓ (going to post screenshot - not sure what the best is possibly a thicker border)
- tested Zoom on MacOS (FireFox) ❓(see comment below)
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide
For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

ghost · 2020-11-30T23:40:16Z

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

github-actions · 2020-11-30T23:42:31Z

🟢 This pull request modifies 2 files and might impact 1 other files.

Details:

All files potentially affected (total: 1)

/src/components/Tabs/Tabs.tsx

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/Tabs/Tabs.scss (total: 1)

Files potentially affected (total: 1)

/src/components/Tabs/Tabs.tsx

…e hover

ginabak · 2020-12-01T02:24:03Z

UNRELEASED.md

 - Added a `alwaysRenderCustomProperties` to `ThemeProvider` for elements that render outside of the DOM tree to their parent context ([#3652](https://github.com/Shopify/polaris-react/pull/3652))
 - Increased precision of hue, saturation, lightness, and alpha in HSBLA `color-transformers` (https://github.com/Shopify/polaris-react/pull/3640)
+- Fixed keyboard interactions for the `Tab` component ([#3650](https://github.com/Shopify/polaris-react/pull/3650))
+- Fixed keyboard interaction when selected Tab was focused and rendering the wrong `::before` colour ([#3669](https://github.com/Shopify/polaris-react/pull/3669))


Line 31 was my unreleased portion. I just moved the other ones in what I thought was the appropriate "sub heading" and changed the verb tenses to be past tense as per the Polaris Documentation Guideline

ginabak · 2020-12-01T03:25:25Z

src/components/Tabs/Tabs.scss

  }

  .Tab-selected {
+    outline: 3px solid transparent;


the reason I added a transparent outline was for Windows High Contrast Mode A11Y reasons:

This was BEFORE (it's hard to tell which one is ACTUALLY selected):

This is what it looks like AFTER(this is what it looks like now - it's very obvious):

NOT SURE if this is the best route would LOVE to know THOUGHTSSSS!

I would just wrap this in a media query so it only applies to high contrast mode. We can also be explicit about the color we use here by including our high contrast color mixin

Suggested change

outline: 3px solid transparent;

@media (-ms-high-contrast: active) {

outline: 3px solid ms-high-contrast-color('text');

}

ms-high-contrast-color('text') this will return the highest contrast value according to whatever the background color is set to. If the background is black, text will always be white etc... so we use this for borders / outlines and such.

oooh, I like this 👀 😄

We seem to be using this approach, media query + HC mixin, in a lot of places so I am all in favour of using it for consistency.

That said, it comes with some tradeoffs. The media query only works in Microsoft browsers so a user using WHCM in Chrome would still see the before experience.

The issue with the transparent outline is that even though the CSS property visually has no impact on the user, it is still rendered by the browser even when its not needed.

ginabak · 2020-12-01T03:41:35Z

🎩 on Zoom on MacOS (FireFox):

In the GIF above, I'm using arrow keys but when I get to the last one it goes back to the first tab ... 🤭
... not sure how one accesses the three dots?! 🤯 OR ... if that's okay? I have a feeling it's not.

kyledurand

This looks awesome! I just left a comment around the high contrast stuff. Let me know if you have any questions about it 😄

Thanks Gina!

kyledurand · 2020-12-01T15:35:58Z

src/components/Tabs/Tabs.scss

  }

  .Tab-selected {
+    outline: 3px solid transparent;


I would just wrap this in a media query so it only applies to high contrast mode. We can also be explicit about the color we use here by including our high contrast color mixin

Suggested change

outline: 3px solid transparent;

@media (-ms-high-contrast: active) {

outline: 3px solid ms-high-contrast-color('text');

}

ms-high-contrast-color('text') this will return the highest contrast value according to whatever the background color is set to. If the background is black, text will always be white etc... so we use this for borders / outlines and such.

arapehl

Apart from Kyle's media query suggestion, everything else LGTM.

ginabak · 2020-12-01T15:51:16Z

With the high contrast @media , this is what it looks like and looks much more clean in my opinion! 👀
YAY!

beefchimi

Quick comments.

beefchimi · 2020-12-01T16:01:29Z

src/components/Tabs/Tabs.scss


+    &:active .Title::before {
+      background: var(--p-action-primary-pressed);
+    }


Nit: Space between blocks

src/components/Tabs/Tabs.scss

beefchimi · 2020-12-01T16:02:29Z

src/components/Tabs/Tabs.scss

      }

      @media (-ms-high-contrast: active) {
+        outline: 3px solid ms-high-contrast-color('text');


Where does the 3px come from? Is this informed by somewhere else in Polaris? If so, is there a way to share that code? If not, you should at least wrap 3pm in the rem() function.

this was not informed by Polaris, I just did a quick search and I see on ActionList.scss that they use 1px:

polaris-react/src/components/ActionList/ActionList.scss

Line 209 in f1e8dcb

outline: 1px solid ms-high-contrast-color('text');

ginabak · 2020-12-01T17:00:28Z

src/components/Tabs/Tabs.scss

      }

      @media (-ms-high-contrast: active) {
+        outline: rem(3px) solid ms-high-contrast-color('text');


@beefchimi 👀 how's this?

beefchimi

Have not been able to tophat, but code looks good 👍

ghost · 2020-12-01T18:16:09Z

🎉 Thanks for your contribution to Polaris React!

DanGodfrey

🎉 awesome work 🙌

emma-boardman · 2020-12-02T09:58:31Z

In the GIF above, I'm using arrow keys but when I get to the last one it goes back to the first tab ... 🤭
... not sure how one accesses the three dots?! 🤯 OR ... if that's okay? I have a feeling it's not.

I think this is ok - I would expect the three dots to be accessible via the tab key, rather than an arrow key. The arrow keys in this pattern is for moving specifically between the tab titles (the elements with role=tab). The three dots are button, rather than a tab title.

If I'm tabbing through the element in your gif as a whole, my first tab would focus the first role=tab element (which I can then navigate within using the arrow keys), and my second tab would focus the three-dot button (which I can then open using enter or space).

…ected state) (#3669) * Experiment Tabs.scss * Fix issue when hover overwrites selected state - on keyboard and mouse hover * Add unreleased.md * Add transparent border for Windows High Contrast Mode * Change focus to grey * Change transparent border to wrap in media query * Address Curtis comments -space and rem

Experiment Tabs.scss

6921e29

Fix issue when hover overwrites selected state - on keyboard and mous…

2c74506

…e hover

ginabak changed the title ~~[WIP][a11][Tabs]: Fix a11y hover states~~ [Tabs][a11y]: Fix a11y issue (when focus overrides selected state) Nov 30, 2020

Add unreleased.md

61f406a

ginabak commented Dec 1, 2020

View reviewed changes

ginabak requested a review from kyledurand December 1, 2020 02:26

ginabak marked this pull request as ready for review December 1, 2020 02:27

ginabak changed the title ~~[Tabs][a11y]: Fix a11y issue (when focus overrides selected state)~~ [Tabs][a11y]: Fix a11y keyboard interaction (when focus overrides selected state) Dec 1, 2020

Add transparent border for Windows High Contrast Mode

04fe3e7

ginabak commented Dec 1, 2020

View reviewed changes

Change focus to grey

39f4ccb

ginabak requested review from DanGodfrey, emma-boardman, beefchimi and arapehl December 1, 2020 15:21

kyledurand approved these changes Dec 1, 2020

View reviewed changes

arapehl approved these changes Dec 1, 2020

View reviewed changes

Change transparent border to wrap in media query

b8c1a8c

beefchimi reviewed Dec 1, 2020

View reviewed changes

ginabak and others added 2 commits December 1, 2020 11:04

Merge branch 'master' into gbak-tabs

a490fdb

Address Curtis comments -space and rem

ff98d62

ginabak commented Dec 1, 2020

View reviewed changes

beefchimi approved these changes Dec 1, 2020

View reviewed changes

ginabak merged commit 270ef9d into master Dec 1, 2020

ginabak deleted the gbak-tabs branch December 1, 2020 18:16

DanGodfrey reviewed Dec 1, 2020

View reviewed changes

[Tabs][a11y]: Fix a11y keyboard interaction (when focus overrides selected state) #3669

[Tabs][a11y]: Fix a11y keyboard interaction (when focus overrides selected state) #3669

Uh oh!

Conversation

ginabak commented Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

BEFORE

AFTER

How to 🎩

🎩 checklist

Uh oh!

ghost commented Nov 30, 2020

Uh oh!

github-actions bot commented Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Uh oh!

ginabak Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ginabak Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ginabak commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kyledurand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arapehl left a comment

Choose a reason for hiding this comment

Uh oh!

ginabak commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beefchimi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beefchimi left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Dec 1, 2020

Uh oh!

DanGodfrey left a comment

Choose a reason for hiding this comment

Uh oh!

emma-boardman commented Dec 2, 2020

Uh oh!

Uh oh!

ginabak commented Nov 30, 2020 •

edited

Loading

github-actions bot commented Nov 30, 2020 •

edited

Loading

ginabak Dec 1, 2020 •

edited

Loading

ginabak Dec 1, 2020 •

edited

Loading

ginabak commented Dec 1, 2020 •

edited

Loading

ginabak commented Dec 1, 2020 •

edited

Loading