Add tooltips to View Pull Request buttons #827
Conversation
|
Hovering over title: Hovering over # link: |
| @@ -57,7 +57,7 @@ | |||
| TextTrimming="CharacterEllipsis" | |||
| Margin="0,-3,5,0" | |||
| Content="{Binding Title}" | |||
| ToolTip="{Binding Title}" | |||
| ToolTip="View Pull Request details" | |||
There was a problem hiding this comment.
We normally put strings like this in the resources file for when we start adding in translations.
There was a problem hiding this comment.
I thought that might be the case! I'll fix it. 👍
There was a problem hiding this comment.
Each project has its own resx file, I forget which project this code belongs to 😆 but it should be there.
There was a problem hiding this comment.
Thanks @paladique for the links!
https://github.com/github/VisualStudio/blob/396baff13f4cecb0a035f6a7569a23a929652900/src/GitHub.VisualStudio.UI/Resources.resx
Here's an example of how we do it within this project
There was a problem hiding this comment.
@paladique There doesn't appear to be a Resources.resx file in the GitHub.VisualStudio project, which is where the PullRequestListItem.xaml file lives. I'm not sure where the command description really belongs. Should it be in the same assembly as the ViewModels (which controls other text that appears on the view). Maybe @grokys has some opinions?
There was a problem hiding this comment.
@jcansdale ah yes! It lives in GitHub.VisualStudio.UI
There was a problem hiding this comment.
I see now, 👍
xmlns:prop="clr-namespace:GitHub.VisualStudio.UI;assembly=GitHub.VisualStudio.UI"
|
This link should also say 'View Pull Request on GitHub'. Dow! Do we have a description associated with commands? I'll have a poke around... |
Probably not? It might be worth doing the tooltip for that in a separate PR, not sure if it's going to involve more code than this one needs. |
| @@ -110,6 +110,7 @@ | |||
| Command="{Binding OpenPROnGitHub, RelativeSource={RelativeSource FindAncestor, AncestorType={x:Type local:PullRequestListView}}}" | |||
| CommandParameter="{Binding Number}" | |||
| Content="{Binding Number}" | |||
| ToolTip="View Pull Request on GitHub" | |||
There was a problem hiding this comment.
What about GitHub Enterprise instances? Usually we'd do something like:
- For GitHub.com: `View Pull Request on GitHub"
- For Enterprise:
View Pull Request on xyz.comwherexyz.comis the domain of the enterprise instance.
There was a problem hiding this comment.
Well, that would make the PR a little more interesting. 😉
I was thinking GitHub the application as opposed to github.com the site. Can I claim president for this interpretation?
(the ghe.io search also says "Search GitHub")
My instinct would be to KISS. What do you think?
There was a problem hiding this comment.
We already do the distinction between "GitHub" and "explicit server name" in the TE home section title and the Connect section title, and there's already code readily available to do this, if that's what's worrying you 😄
|
Updated with @paladique's suggestion to use text from |
jcansdale
force-pushed
the
fixes/824-button-tooltips
branch
from
28e5025
to
0424699
Compare
|
To be honest, I'm not sure about replacing the tooltip that gives the title of the PR. It's a common pattern that when you have possibly truncated text to use the tooltip to display the non-truncated text, and that's what we were doing here. I personally find this behavior useful as my PR list is frequently not wide enough to display the PR title:
|
This makes sense. What about the other links. Do you think it makes sense to let the user know they're about to open a browser? |
|
Yeah, the "View Pull Request on GitHub" message on the # link definitely makes sense. |
jcansdale
force-pushed
the
fixes/824-button-tooltips
branch
from
df56b1f
to
e762477
Compare
|
Reverted tooltip that gives the title of the PR. This PR makes me feel like a heavyweight! 😉 @grokys If you're happy, could you approve and merge? |
This is a fix for #824.
The heading link now says: "View Pull Request details".
The # link now says: "View Pull Request details".