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

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

Closed
snidersd opened this issue May 15, 2019 · 14 comments · Fixed by #4073 or #4110
Assignees
Labels
package: react carbon-components-react severity: 2 https://ibm.biz/carbon-severity status: blocked 🙅‍♀️ type: a11y ♿

Comments

@snidersd
Copy link

This issue is related to #2458
Retested after the latest fix #2656 and there is still a DAP failure.

The original fix for #2458 was to remove aria-labelledby="_carbon-tooltip-trigger_qonq7h6yqf", but the aria-label="tooltip" was also removed which is causing the new violation. (see screenshot):
Screen Shot 2019-05-15 at 9 33 42 AM
Please add aria-label="tooltip" back in to fix.

@emyarod
Copy link
Member

emyarod commented May 15, 2019

hi @snidersd! #2656 only removes the aria-labelledby attribute. after doing a bit of digging it seems this violation is instead caused by 6190488 (this commit was originally contained in carbon-design-system/carbon-components-react#2245 but the PR links were broken during the monorepo move).

if I understand correctly, we are getting conflicting information (or have misinterpreted the feedback) about the necessity of aria-label="tooltip" in https://github.com/carbon-design-system/carbon-components-react/issues/2193. can we get additional clarification before proceeding with any changes?

@snidersd
Copy link
Author

@emyarod It looks like the mix up is in the use of aria-label vs aria-labelledby. In the original issue #2458 both the aria-labelledby and an aria-label were on the svg which effectively meant there were 2 different labels on the same element. As mentioned in #2193 an aria-label is typically used if there is no visible label to reference or aria-labelledby is used to reference a visible label. The DAP error is because neither one exist in the updated React Tooltip. If you add aria-labelledby to the

and reference the tooltip label that will also fix the issue. Note: The Vanilla Tooltip Component has no Violations using aria-labelledby in the
.

@dakahn dakahn added priority: high severity: 1 https://ibm.biz/carbon-severity labels May 20, 2019
@snidersd
Copy link
Author

@dakahn Please add the react label to this issue. Thx!

@dakahn dakahn added the package: react carbon-components-react label May 21, 2019
@snidersd snidersd changed the title AVT 1 - - Tooltip DAP violation AVT 1 - Default Tooltip DAP violation May 24, 2019
@snidersd
Copy link
Author

@dakahn Please add this issue to the IBM Carbon Copy Milestone. Thx!

@dakahn dakahn added this to the IBMa Carbon Copy 🧁 milestone May 28, 2019
@dakahn dakahn self-assigned this Jun 13, 2019
@snidersd
Copy link
Author

I'm only seeing the DAP issue with the TooltipDefintion and the TooltipIcon. However, the Tooltip, which is actually a Modal does not restrict tab focus to the modal.

@snidersd snidersd changed the title AVT 1 - Default Tooltip DAP violation AVT 1 &AVT 2 - Default Tooltip has keyboard issue and TooltipDefinition and Icon have DAP violations Jun 13, 2019
@dakahn
Copy link
Contributor

dakahn commented Jul 23, 2019

@snidersd Circling back on this. Looks like the DAP errors are cleared 🥂.

As for the AVT2 (keyboard a11y) violation what we're talking about is making our tooltip if it's open? Or only tooltips that have links/buttons in them? I'm not a huge fan of using tooltips in that way anyway (hiding options etc) -- my assumption is that this isn't suggested use, but we're just showing off what you could do.

If we changed our example would a focus trap still be necessary?

@elizabethsjudd
Copy link
Contributor

@snidersd @dakahn this is a related PR that i'm working on to resolve AVT 2 and 3 in vanilla #3476

@snidersd
Copy link
Author

@dakahn IMO we should keep React and Vanilla in sync. I think we should look at updating React when the related PR Liz has open #3476 is complete.

@elizabethsjudd
Copy link
Contributor

@snidersd @dakahn I've had PRs merged that corrected the DAP violations in Definition and Icon tooltips for Vanilla: #3151 and #3115

@dakahn dakahn added status: blocked 🙅‍♀️ severity: 2 https://ibm.biz/carbon-severity and removed severity: 1 https://ibm.biz/carbon-severity labels Jul 24, 2019
@dakahn
Copy link
Contributor

dakahn commented Jul 24, 2019

Cool! I'll wait for @elizabethsjudd's PR to drop and then port changes over 👍

@snidersd
Copy link
Author

After retesting this issue, the Default Tooltip still has a DAP violation. See screenshot below:
Screen Shot 2019-09-25 at 9 54 30 AM
In addition, the Learn More Link and Create button cannot be activated using only a keyboard.
Note: The DAP issue is only present when the button is not pressed.

@snidersd snidersd reopened this Sep 25, 2019
@dakahn
Copy link
Contributor

dakahn commented Sep 26, 2019

We should open a separate ticket for the inaccessible tooltip elements. My hunch is that we actually don't intend our tooltips to have confirmation buttons etc...

@dakahn
Copy link
Contributor

dakahn commented Sep 26, 2019

@snidersd Couldn't we classify this DAP error as a React app false positive? If the tooltip has focus -- it's open. And if it's open, that aria-describedby id is valid. It's only in the situation where we're scanning the DOM using AAT that this is a problem unless there's something I'm missing.

@elizabethsjudd
Copy link
Contributor

@dakahn the DAP violation @snidersd was getting was because the ID started with an underscore.
IDs must start with a letter in order to be valid.

@dakahn dakahn modified the milestones: IBMa Carbon Copy, ♿Carbon WCAG Compliance♿ Nov 21, 2019
@mattrosno mattrosno removed this from the ♿Carbon WCAG Compliance♿ milestone Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: react carbon-components-react severity: 2 https://ibm.biz/carbon-severity status: blocked 🙅‍♀️ type: a11y ♿
Projects
None yet
5 participants