Display tooltips for buttons, especially those without text #4185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The best candidate is `aria-label`, followed by `description`. Most of the existing elements already have these defined, so try to route it as the tooltip instead of having to explicitly define tooltips everywhere through a redundant `title` or <Tooltip> tag. Minor side-effect: This will cancel off any effect from a parent <Tooltip>, i.e. might confuse future developers who are trying to do "<Tooltip><Button></Button></Tooltip>".
<MenuButton> is not from us, so the automatic tooltip from the previous commit does not apply here.
| @@ -164,6 +164,7 @@ const Header = (props: Props) => { | |||
| <Menu> | |||
| <MenuButton | |||
| aria-label={__('Publish a file, or create a channel')} | |||
| title={__('Publish a file, or create a channel')} | |||
There was a problem hiding this comment.
I think the tooltip needs to be shorter.
There was a problem hiding this comment.
Isn't there a possibility that it hides other text or gets cut?
There was a problem hiding this comment.
I think it would look better if it is made "Publish Your Content!" or "Publish Freedom" or something like that. Or just "Publish Content".
This is still my personal opinion. I don't like long tooltips, they look a bit messy to me.
There was a problem hiding this comment.
Nope, they sit on top of all the other text, so it won't get hidden.
|
Do you have a picture/gif of the tooltipw btw? |
|
Okay.
…On Wed, 13 May, 2020, 7:50 PM Sean Yesmunt, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In ui/component/header/view.jsx
<#4185 (comment)>:
> @@ -164,6 +164,7 @@ const Header = (props: Props) => {
<Menu>
<MenuButton
aria-label={__('Publish a file, or create a channel')}
+ title={__('Publish a file, or create a channel')}
Nope, they sit on top of all the other text, so it won't get hidden.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4185 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIJI5H2YNPU6HVZZSS5IMZLRRKUBHANCNFSM4M6XKESA>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check all that apply to this PR using "x":
PR Type
What kind of change does this PR introduce?
Fixes
Issue Number: Fixes #3465
What is the current behavior?
Some buttons (especially those without text) is lacking tooltips.
What is the new behavior?
Display tooltips for those elements.
Other information
The strings are already in place, so code was added to route the data to
titlefor it to appear as tooltip.Refer to commit message for full details.