Icon Tooltip update for WCAG 2.1 compliance #3115

elizabethsjudd · 2019-06-19T15:57:13Z

Closes #3094

Update to Icon Tooltip to pass AAT ruleset for WCAG 2.1

Changelog

Changed

Icon Tooltip trigger now supports using a span within the button instead of the aria-label attribute.
To support backwards compatibility and maintain a consistent animation, added bx--tooltip--a11y modifier to the trigger button

Testing / Reviewing

Should be tested in Carbon supported browsers (I believe it's Safari, Chrome, Firefox, Edge, and IE11)
Use AAT (DAP) to verify the violations no longer are thrown

Before:

After:

netlify · 2019-06-19T16:07:52Z

Deploy preview for the-carbon-components ready!

Built with commit 2d4ef90

https://deploy-preview-3115--the-carbon-components.netlify.com

netlify · 2019-06-19T16:08:49Z

Deploy preview for carbon-components-react ready!

Built with commit 2d4ef90

https://deploy-preview-3115--carbon-components-react.netlify.com

netlify · 2019-06-19T16:13:48Z

Deploy preview for carbon-elements failed.

Built with commit 2d4ef90

https://app.netlify.com/sites/carbon-elements/deploys/5d0a5b5c6187780008d79c33

netlify · 2019-06-19T16:17:33Z

Deploy preview for the-carbon-components ready!

Built with commit f4d6bf1

https://deploy-preview-3115--the-carbon-components.netlify.com

netlify · 2019-06-19T16:22:21Z

Deploy preview for carbon-components-react ready!

Built with commit f4d6bf1

https://deploy-preview-3115--carbon-components-react.netlify.com

netlify · 2019-06-19T16:26:22Z

Deploy preview for carbon-elements ready!

Built with commit f4d6bf1

https://deploy-preview-3115--carbon-elements.netlify.com

emyarod

my DAP results don't change from the WCAG 2.0 compliant tooltip to the WCAG 2.1 compliant tooltip, but I think it's because my DAP ruleset is out of date and I'm not sure how to upgrade

but from my other testing the WCAG 2.1 tooltip functions and appears like the WCAG 2.0 compliant tooltip

elizabethsjudd · 2019-06-19T19:11:15Z

@emyarod what version of DAP are you using? The latest is 1.8.0.0 and the rulesets should match the screen shot below (make sure you are using the Internal IBM version of DAP that you download through this URL: https://aat.w3ibm.mybluemix.net/tools/auth/tools.html). The external version that you can access through the Chrome store is out of date and doesn't have any of the WCAG 2.1 updates.

elizabethsjudd · 2019-06-19T20:29:57Z

FYI, my plan was to remove the legacy versions from the demo (so they don't make it on to the website) once the legacy version has been verified to still work with the changes.

joshblack · 2019-06-19T20:41:18Z

@elizabethsjudd just curious, what were the violations that this PR is hoping to address?

scottnath · 2019-06-19T20:41:32Z

packages/components/src/components/tooltip/tooltip--icon.hbs

+{{!-- @todo Had to add a modifier to the parent to correct the animation but in the next major release it could be removed in favor of the new HTML format --}}
+<button
+  class="{{@root.prefix}}--tooltip__trigger {{@root.prefix}}--tooltip--a11y {{@root.prefix}}--tooltip__trigger--icon {{@root.prefix}}--tooltip--left {{@root.prefix}}--tooltip--align-start">
+  {{!-- @todo Open to suggestions for this element's className; In WH we have a utility class (wh-u-a11yTitle) that can be applied to other components --}}


seems out-of-scope for this pr, but I'd vote for a global class for these a11y styles as they could be very useful in other situations. Then, {{@root.prefix}}--tooltip__content could bring those in for this use-case, but not have them locked to this component.

elizabethsjudd · 2019-06-19T20:44:23Z

@joshblack with WCAG 2.1 buttons that don't have text in them fail AAT because it reads it as missing a visible label. By moving the content from aria-label to the span the tests are able to recognize that as the label. WCAG 2.1 had a lot of updates where the use of aria-label is going to become a lot more restrictive. Visuals and behaviors with this PR should remain the same as the previous version.

dakahn

This looks good to me. Thanks a ton @elizabethsjudd 🏄🏽‍♂️

I'd like @snidersd to get eyes on this since there's a fair few (seemingly much needed) changes. 👍🏽

asudoh

Thanks @elizabethsjudd for jumping in!

@emyarod Would you have a chance to test it, especially considering long text (if you haven't done yet)? Thanks!

asudoh · 2019-06-20T00:48:50Z

packages/components/src/globals/scss/_tooltip.scss

+    overflow: hidden;
+    clip: rect(0, 0, 0, 0);
+    border: 0;
+    visibility: visible;


visibility:visible undoes visiblity:hidden in an ancestor node. I'd recommend visibility:inherit.

Also does this have similar purpose of .bx--assistive-text? https://github.com/carbon-design-system/carbon/blob/v10.3.0/packages/components/src/globals/scss/_css--helpers.scss#L17-L29

emyarod · 2019-06-20T13:33:11Z

@elizabethsjudd unfortunately I'm not able to view the page, I'm just taken to an "Access Denied" even after authenticating. would it be possible to just share the .crx? not sure how to gain access to the a11y tools page you linked

@asudoh yeah it seems fine with long text and functions as expected, but I am seeing the tooltip caret finish animating before the tooltip body though (tagging @shixiedesign for design review). my guess is we can apply the same opacity + animation rules to the caret?

elizabethsjudd · 2019-06-20T13:39:20Z

@emyarod what is the .crx? Also, try this link instead: http://ibm.biz/A11yToolsLandingPage (you have to be on the IBM VPN to access and use the internal DAP version)

joshblack · 2019-06-25T17:33:56Z

Thanks so much @elizabethsjudd! Really appreciate it 🙏

shixiedesign

Thank you! And especially thanks for double checking the motion part too 🙏

After finally getting to do the review, I noticed a tiny gap between the triangle of the tooltip & the tooltip body box. Vanilla only. @emyarod explained this is also magnification dependent so I'm not sure if there's a fix. If not I guess it's not a big deal, lemme know @elizabethsjudd Everything else looks great!

elizabethsjudd · 2019-06-25T19:42:54Z

@shixiedesign thanks for the review! I've noticed the gap between the body and caret as well but I saw it happen with the current implementation. As I was working on the definition tooltip, I noticed that the caret placement was actually quite finicky based on surrounding components/styles that were having odd side-effects with the component. I do feel like it's a valid issue to re-access the caret placement given all of the available demos, but that it's beyond the scope of this PR where I'm really focused on the a11y of the component while maintaining existing behaviors and styles.

asudoh · 2019-06-25T20:41:42Z

@elizabethsjudd Though we don't have a guideline for this topic, it'd be great if you can figure out a way to avoid :not() - The way I tend to avoid :not() is having some overriding style that negates original. Probably .bx--tooltip--a11y::after() { content: none; } (more specificity may be needed) for your case?

shixiedesign

👍 agreed! Thanks for the fix!

elizabethsjudd · 2019-06-26T12:39:25Z

@asudoh out of curiosity, why do you avoid the :not selectors (i've heard pros and cons both ways and was just curious for your reasoning)

…feature/icon-tooltip

emyarod

meant to comment in #3151

tw15egan · 2019-06-26T18:03:12Z

@asudoh @elizabethsjudd I'm also curious as to why we should avoid :not selectors. I thought the optimization of css selectors was pretty negligible.

asudoh · 2019-06-27T12:56:33Z

@elizabethsjudd @tw15egan Good question from you both! The event I lead to my inclination to avoid :not() has completely slipped off my mind, but a couple of things what I can recall... One is every occurrence of "I don't want this style for this feature" ends up with addition to :not(), like :not(.bx--foo):not(.bx--bar):not(.bx--baz). Also, the style rules of a feature set ends up with with multiple places, one is where :not() is, another is the feature-specific style implementation itself. I'm inclined to having .bx--foo { ... } work for :not() portion as well as for others.

elizabethsjudd · 2019-06-27T13:25:29Z

@asudoh I agree with the fact that :not should be used sparingly in the fact that yes, if you just use it as a fail safe you do end up with bloated selectors as you described. Another reason I have found the :not to be a pain is that it tends leads to higher selector weight making them harder to overwrite. However, in cases like this where we are trying to maintain backwards compatibility I do feel like they are a wash. We still end up with multiple feature specific declarations (the default backwards compatible and then overwriting for the new feature). Right now in tooltip the after pseudo element is actually still getting all of the styles applied to it even though it doesn't use them. which could actually lead to unexpected behaviors for users of Carbon that are trying to extend the styles. I'm good with keeping them removed but I guess I'm saying we shouldn't treat it as a blanket "never use it" but instead assess the situation and figure what is the appropriate solution. It was added to the CSS specification for a reason.

asudoh · 2019-06-27T13:35:33Z

Thank you for sharing your thoughts @elizabethsjudd - If I can focus on just one thing, you are right that ::after() gets all the styles even though they are not in use. As far as I see content:none eliminates all the adverse effects by that. And your latest code with content:none seems to have made the change look more straightforward.

tw15egan · 2019-06-27T16:00:45Z

@asudoh thanks for filling me in! Makes sense, and I agree in this situation. @elizabethsjudd hit it on the head here:

I guess I'm saying we shouldn't treat it as a blanket "never use it" but instead assess the situation and figure what is the appropriate solution

asudoh · 2019-06-27T22:44:28Z

@tw15egan Yes makes sense! - What I meant by my first comment wrt :not() is that good to think twice when we are tempted to use :not().

asudoh · 2019-06-27T22:48:26Z

@dakahn Do you think we are good to go? Or should we wait @snidersd? Thanks!

dakahn · 2019-06-27T22:56:57Z

@asudoh I'm happy! Let's do it 🏄

feat(icon-tooltip): wcag 2.1 compliance

2d4ef90

Merge branch 'master' into feature/icon-tooltip

c4869dd

elizabethsjudd mentioned this pull request Jun 19, 2019

Definition Tooltip: AVT 1 violation #3099

Closed

emyarod reviewed Jun 19, 2019

View reviewed changes

emyarod requested a review from shixiedesign June 19, 2019 18:43

emyarod added status: ready for review 👀 status: visual review 🔍 labels Jun 19, 2019

joshblack requested review from a team and removed request for a team June 19, 2019 20:36

ghost requested review from dakahn and removed request for a team June 19, 2019 20:37

scottnath reviewed Jun 19, 2019

View reviewed changes

dakahn requested a review from snidersd June 19, 2019 20:59

dakahn approved these changes Jun 19, 2019

View reviewed changes

asudoh reviewed Jun 20, 2019

View reviewed changes

feat(icon-tooltip): use global a11y class for span

160a31c

shixiedesign reviewed Jun 25, 2019

View reviewed changes

shixiedesign approved these changes Jun 25, 2019

View reviewed changes

Merge branch 'master' of github.com:carbon-design-system/carbon into …

647859b

…feature/icon-tooltip

emyarod reviewed Jun 26, 2019

View reviewed changes

feat(icon-tooltip): remove not selector

8739ca0

Merge branch 'master' into feature/icon-tooltip

9a677f9

Merge branch 'master' into feature/icon-tooltip

f4d6bf1

asudoh merged commit aea7c65 into carbon-design-system:master Jun 27, 2019

emyarod mentioned this pull request Jul 11, 2019

feat(Button): add icon-only button support #3368

Merged

emyarod mentioned this pull request Jul 23, 2019

fix(PasswordInput): use updated tooltip for password visibility toggle #3515

Merged

elizabethsjudd mentioned this pull request Jul 24, 2019

AVT 1 &AVT 2 - Default Tooltip has keyboard issue and TooltipDefinition and Icon have DAP violations #2736

Closed

elizabethsjudd mentioned this pull request Aug 2, 2019

Modal should use icon tooltip #3630

Closed

emyarod mentioned this pull request Aug 15, 2019

refactor(tooltip): avoid calc inside transform for IE support #3739

Merged

elizabethsjudd mentioned this pull request Aug 23, 2019

[React Icon Tooltip] carry over a11y enhancements from Vanilla #3811

Closed

This was referenced Aug 27, 2019

feat(TooltipIcon): port vanilla WCAG 2.1 fixes #3842

Merged

_tooltip.scss is exported into globals folder #3834

Closed

emyarod mentioned this pull request Sep 18, 2019

feat(tooltip): allow custom CSS classes for displaying tooltips #4016

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Icon Tooltip update for WCAG 2.1 compliance #3115

Icon Tooltip update for WCAG 2.1 compliance #3115

elizabethsjudd commented Jun 19, 2019

netlify bot commented Jun 19, 2019

netlify bot commented Jun 19, 2019

netlify bot commented Jun 19, 2019

netlify bot commented Jun 19, 2019 •

edited

Loading

netlify bot commented Jun 19, 2019 •

edited

Loading

netlify bot commented Jun 19, 2019 •

edited

Loading

emyarod left a comment

elizabethsjudd commented Jun 19, 2019

elizabethsjudd commented Jun 19, 2019

joshblack commented Jun 19, 2019

scottnath Jun 19, 2019

elizabethsjudd commented Jun 19, 2019

dakahn left a comment

asudoh left a comment

asudoh Jun 20, 2019

asudoh Jun 20, 2019

emyarod commented Jun 20, 2019 •

edited

Loading

elizabethsjudd commented Jun 20, 2019 •

edited

Loading

joshblack commented Jun 25, 2019

shixiedesign left a comment •

edited

Loading

elizabethsjudd commented Jun 25, 2019

asudoh commented Jun 25, 2019

shixiedesign left a comment •

edited

Loading

elizabethsjudd commented Jun 26, 2019

emyarod left a comment •

edited

Loading

tw15egan commented Jun 26, 2019

asudoh commented Jun 27, 2019 •

edited

Loading

elizabethsjudd commented Jun 27, 2019

asudoh commented Jun 27, 2019

tw15egan commented Jun 27, 2019

asudoh commented Jun 27, 2019

asudoh commented Jun 27, 2019

dakahn commented Jun 27, 2019

Icon Tooltip update for WCAG 2.1 compliance #3115

Icon Tooltip update for WCAG 2.1 compliance #3115

Conversation

elizabethsjudd commented Jun 19, 2019

Changelog

Testing / Reviewing

netlify bot commented Jun 19, 2019

netlify bot commented Jun 19, 2019

netlify bot commented Jun 19, 2019

netlify bot commented Jun 19, 2019 • edited Loading

netlify bot commented Jun 19, 2019 • edited Loading

netlify bot commented Jun 19, 2019 • edited Loading

emyarod left a comment

Choose a reason for hiding this comment

elizabethsjudd commented Jun 19, 2019

elizabethsjudd commented Jun 19, 2019

joshblack commented Jun 19, 2019

scottnath Jun 19, 2019

Choose a reason for hiding this comment

elizabethsjudd commented Jun 19, 2019

dakahn left a comment

Choose a reason for hiding this comment

asudoh left a comment

Choose a reason for hiding this comment

asudoh Jun 20, 2019

Choose a reason for hiding this comment

asudoh Jun 20, 2019

Choose a reason for hiding this comment

emyarod commented Jun 20, 2019 • edited Loading

elizabethsjudd commented Jun 20, 2019 • edited Loading

joshblack commented Jun 25, 2019

shixiedesign left a comment • edited Loading

Choose a reason for hiding this comment

elizabethsjudd commented Jun 25, 2019

asudoh commented Jun 25, 2019

shixiedesign left a comment • edited Loading

Choose a reason for hiding this comment

elizabethsjudd commented Jun 26, 2019

emyarod left a comment • edited Loading

Choose a reason for hiding this comment

tw15egan commented Jun 26, 2019

asudoh commented Jun 27, 2019 • edited Loading

elizabethsjudd commented Jun 27, 2019

asudoh commented Jun 27, 2019

tw15egan commented Jun 27, 2019

asudoh commented Jun 27, 2019

asudoh commented Jun 27, 2019

dakahn commented Jun 27, 2019

netlify bot commented Jun 19, 2019 •

edited

Loading

netlify bot commented Jun 19, 2019 •

edited

Loading

netlify bot commented Jun 19, 2019 •

edited

Loading

emyarod commented Jun 20, 2019 •

edited

Loading

elizabethsjudd commented Jun 20, 2019 •

edited

Loading

shixiedesign left a comment •

edited

Loading

shixiedesign left a comment •

edited

Loading

emyarod left a comment •

edited

Loading

asudoh commented Jun 27, 2019 •

edited

Loading