Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(docs): add usage example with Tooltip for the actionable elements #1636

Merged
merged 20 commits into from
Jul 16, 2019

Conversation

mnajdova
Copy link
Contributor

This PR is adding usage example for the Button, MenuItem and ToolbarItem components for using Tooltip

Open questions:
Should we add tooltip for the disabled elements? - there is an example of this in the Toolbar example
Should we add tooltip examples of any other components, like Slider, or Popup and Dialog - as they have usually Button as a trigger

@codecov
Copy link

codecov bot commented Jul 15, 2019

Codecov Report

Merging #1636 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1636   +/-   ##
=======================================
  Coverage   71.28%   71.28%           
=======================================
  Files         852      852           
  Lines        7045     7045           
  Branches     2029     2008   -21     
=======================================
  Hits         5022     5022           
  Misses       2017     2017           
  Partials        6        6
Impacted Files Coverage Δ
packages/react/src/components/Tooltip/Tooltip.tsx 61.97% <ø> (ø) ⬆️

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 775742d...5e2b332. Read the comment docs.

@sophieH29
Copy link
Contributor

sophieH29 commented Jul 15, 2019

Should we add tooltip examples of any other components, like Slider, or Popup and Dialog - as they have usually Button as a trigger

I think Popup is good to have. I know there is one for the toolbar with Popup. But would be a plus to see a separate example of combining that nested logic with Popup's trigger and Tooltip's trigger

@mnajdova
Copy link
Contributor Author

@sophieH29 I tried adding example for the Popup with tooltip, but it seems that accessibility is broken in this case:
image

With escape, the tooltip is only closed, we cannot close the Popup with escape... Not sure if we should add this example until we have working solution. Here is the code I tried:
https://codesandbox.io/s/stardust-ui-example-7ho97

@mnajdova
Copy link
Contributor Author

As oppose to the prev comment for the issues of the Popup with tooltip, the toolbar item containing popup and tooltip on escape is closing only the popup, the tooltip can never be closed with this action. Should I remove this example for now and we can address these issues in a separate PR and add the examples together with the fix? @miroslavstastny I know you were investigating this, maybe you have some different proposal. @sophieH29 FYI

Copy link
Member

@miroslavstastny miroslavstastny left a comment

Choose a reason for hiding this comment

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

Let's solve Popup and Tooltip attached to the same trigger in a separate issue, discussing desired behavior first. As you described, accessibility is broken and there are some other problems I have found so far:

  • Popup and Tooltip are not spreading props to trigger
  • both have z-index: 1000, my gut feeling is Tooltip should have higher z-index than Popup

-updated changelog
@mnajdova
Copy link
Contributor Author

Thanks everyone, will add steps for the new screens and will merge it

@vercel vercel bot requested a deployment to staging July 16, 2019 10:38 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 12:31 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 12:37 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 12:39 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 13:01 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 13:10 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 13:12 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 13:38 Abandoned
@vercel vercel bot requested a deployment to staging July 16, 2019 13:39 Abandoned
@mnajdova mnajdova merged commit 470ad24 into master Jul 16, 2019
@delete-merged-branch delete-merged-branch bot deleted the docs/add-actionable-examples branch July 16, 2019 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants