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

Components: Add Tooltip component, display button aria-label tooltips #2293

Merged
merged 7 commits into from
Aug 11, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Aug 8, 2017

Supersedes: #839

This pull request seeks to...

  • Add a new Tooltip component
  • Use the new Tooltip component to render IconButton tooltips by aria-label if no other children are specified (i.e. no explanatory text)

Tooltips

Open questions:

  • Do we want all aria-label to show as tooltips? Previous iteration of this pull request applied labels to all Button. Now we only apply to IconButton with no children (explanatory text)

Testing instructions:

Verify that tooltips show, particularly:

  • Both by mouseover and focus events
  • Both for disabled and enabled buttons

@aduth aduth added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). General Interface Parts of the UI which don't fall neatly under other labels. [Feature] UI Components Impacts or related to the UI component system labels Aug 8, 2017
@aduth
Copy link
Member Author

aduth commented Aug 9, 2017

I suppose I should add role="tooltip" as well. (4b8b5f1)

@codecov
Copy link

codecov bot commented Aug 11, 2017

Codecov Report

Merging #2293 into master will increase coverage by 0.05%.
The diff coverage is 82.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2293      +/-   ##
==========================================
+ Coverage   24.95%   25.01%   +0.05%     
==========================================
  Files         153      154       +1     
  Lines        4780     4946     +166     
  Branches      803      858      +55     
==========================================
+ Hits         1193     1237      +44     
- Misses       3032     3107      +75     
- Partials      555      602      +47
Impacted Files Coverage Δ
components/button/index.js 90.9% <ø> (ø) ⬆️
components/icon-button/index.js 100% <100%> (ø) ⬆️
components/popover/index.js 94.73% <100%> (-0.27%) ⬇️
components/panel/body.js 100% <100%> (ø) ⬆️
components/tooltip/index.js 77.27% <77.27%> (ø)
...ditor/modes/visual-editor/invalid-block-warning.js 0% <0%> (ø) ⬆️
blocks/editable/index.js 7.35% <0%> (+6.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0714e2...9710cc0. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). General Interface Parts of the UI which don't fall neatly under other labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant