NavigationViewItem: Empty tooltip no longer shown #3430

Felix-Dev · 2020-10-15T14:15:51Z

Description

This PR fixes an issue where an (empty) tooltip was shown for a NavigationViewItem with its content set to the empty string.

Motivation and Context

Fixes #3427

How Has This Been Tested?

Tested visually and added API test.

marcelwgn · 2020-10-16T12:22:49Z

The test if failing on my machine, and with the changes.

Felix-Dev · 2020-10-16T12:42:31Z

@chingucoding

Test runs fine for me:

Tooltips show up as expected:

PR branch is up-to-date with master.

marcelwgn · 2020-10-16T13:13:55Z

Edit: It was not, it was me not knowing what should expose tooltips.

I think that this is a regression with ToolTip service in the latest insider build, other tooltips are also failing on my machine now too.

StephenLPeters · 2020-10-16T18:22:29Z

I think that this is a regression with ToolTip service in the latest insider build, other tooltips are also failing on my machine now too.

@YuliKl do we know about this?

StephenLPeters · 2020-10-16T23:28:33Z

@chingucoding can you provide an example of your tooltips not working? if this is broken we need to track it down. Is it unique to uwp app? what about the title bar tooltips?

Felix-Dev · 2020-10-16T23:50:38Z

@chingucoding @StephenLPeters I've updated to the newest 20236 Insider build in my insider VM and checked the XAML Controls Gallery app there. All tooltips seemingly work fine (NavigationView tooltips, titlebar tooltips, tooltips on the XCG's ToolTip page,...).

In Windows itself, the shell tooltips appear to work just fine too (small tiles' tooltips in the start menu and Win32 tooltips in the task bar).

So the issue @chingucoding is seeing might not be present on every device running the newest insider build and might be caused by some stuff like A/B testing, different device configurations,...

marcelwgn · 2020-10-17T06:59:19Z

Edit: Tooltips work normally on my machine.

The new test that Felix added in this PR fails with the following:

The failures were encountered on version 10.0.20236 Build 20236.

Felix-Dev · 2020-10-17T10:31:47Z

@chingucoding I assume you have tested the WinUI NavigationView in Left mode with the pane closed? Only in that case (when we just show the "navigation strip") do we currently show tooltips for the NavigationViewItems. When the pane is open, as in your posted gif, we do not show tooltips:

microsoft-ui-xaml/dev/NavigationView/NavigationViewItem.cpp

Line 505 in 2341faa

bool NavigationViewItem::ShouldEnableToolTip() const

That said, should we consider changing that? I view tooltips as one way to convey information to the user which otherwise might not fit on the screen. Right now, if I set a NavigationViewItem content long enough to not fit into the NavigationView pane, then I only see the tooltip when I close the pane.

<muxc:NavigationView>
    <muxc:NavigationView.MenuItems>
        <muxc:NavigationViewItem Icon="Home" Content="Quite a long description which should not fit entirely" />
    </muxc:NavigationView.MenuItems>
</muxc:NavigationView>

We already expose APIs to configure the width of the open pane (for example NavigationView.OpenPaneLength) and in the future, we might make pane resizing even easier available to customers as proposed in #190. Point being that the open pane length could be changed by app users (if supported by the developer) to a new width where some menu items might not entirely fit into the opened pane width any longer. Tooltips can be used then to still show the entire menu item content.

Another aspect might be a fixed open pane length but due to localization, in some supported app languages we might have translated strings which don't entirely fit into the available space whereas they would (easily) fit in other app languages. So while the open pane length would stay the same across app language changes, the menu item's content width could be increased and suddenly not fit into the available space any longer.

Thus, I think we should show tooltips even when not in closed compact mode while using Left navigation. For Top navigation, no menu item content will be cutoff. If the item doesn't fit into the space currently available, it will be moved into the overflow menu which will take enough space to show the entire menu item (icon + content).

@StephenLPeters @YuliKl Your thoughts?

marcelwgn · 2020-10-17T10:47:43Z

@chingucoding I asume you have tested the WinUI NavigationView in Left mode with the pane closed? Only in that case (when we just show the "navigation strip") do we currently show tooltips for the NavigationViewItems. When the pane is open, as in your posted gif, we do not show tooltips:

I did in fact not, that tooltip works. So it was just me not knowing when tooltips of NavView show, sorry about that.

Still, the test failure remains. Looking at the code, I think the reason why it failed on my machine is that the API test was run on a screen large enough to expand the pane resulting in not showing the tooltip. The CI machines also run through fine as their resolution is HD.

Adding navView.PaneDisplayMode = NavigationViewPaneDisplayMode.LeftCompact; fixes the test for me. Maybe we can add that or enforce the CI dimensions for API tests too?

StephenLPeters · 2020-10-21T18:55:42Z

@chingucoding I think enforcing CI dimensions for API tests makes sense, Lets open a new issue for that.

@YuliKl can you weigh in on felix's tooltip question?

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs

Felix-Dev · 2020-10-31T08:55:35Z

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs

+        {
+            if (!PlatformConfiguration.IsOsVersionGreaterThanOrEqual(OSVersion.Redstone5))
+            {
+                Log.Warning("On RS4 and earlier the test needs to be modified slightly.");


This should only be a placeholder for now. The tooltip implementation changed with RS5+ and I can only test that behavior on my local machine for now. I do not have a VM up and running RS4 and earlier yet. Since I cannot locally test that scenario, I haven't written a test case for it yet, though the implementation used in RS4 and below still exists and I could write a test based on the source code. Testing that would have to rely on the test pipeline then to spot any errors made when writing that test.

Should I go ahead and create a test based on the available implementation?

Felix-Dev · 2020-10-31T08:55:53Z

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs

+        {
+            if (!PlatformConfiguration.IsOsVersionGreaterThanOrEqual(OSVersion.Redstone5))
+            {
+                Log.Warning("On RS4 and earlier the test needs to be modified slightly.");


Same as above.

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs

Felix-Dev · 2020-10-31T09:23:14Z

@YuliKl The Groove music app shows another scenario where showing tooltips even when the pane is open in Left mode is useful:

Groove allows the user to create custom playlists (with custom names without name length limitation) so the chosen playlist name might not fit the pane width entirely. A tooltip can be used to show the entire name to the user.

I think, alongside the other points I mentioned earlier, that it makes sense to show tooltips by default in Left mode (pane open/closed) so that developers don't have to take care of that.

ranjeshj · 2020-11-21T14:10:01Z

@Felix-Dev can you please merge with master ? Thanks!

Felix-Dev · 2020-11-23T17:51:10Z

Merged with master.

StephenLPeters · 2020-11-23T18:58:31Z

/azp run

azure-pipelines · 2020-11-23T18:58:47Z

Azure Pipelines successfully started running 1 pipeline(s).

StephenLPeters · 2020-11-23T19:01:43Z

@Felix-Dev I'm trying to find the discussion we had a while back about the dificulties you had with getting VS to recognize the new API test project you added. Do you happen to know where that discussion was? Sorry not really related to this PR at all.

Felix-Dev · 2020-11-23T19:12:59Z

@StephenLPeters Do you perhaps mean this conversation?

StephenLPeters · 2020-11-23T19:14:33Z

Hmm, I think so. Thank you!

ghost · 2021-03-05T17:45:04Z

🎉Microsoft.UI.Xaml v2.6.0-prerelease.210227001 has been released which incorporates this pull request.:tada:

Handy links:

Felix-Dev added 2 commits October 15, 2020 16:09

Fix empty tooltip being shown even though it shouldn't.

b8ca995

Add API test.

45d4aad

ghost added the needs-triage Issue needs to be triaged by the area owners label Oct 15, 2020

StephenLPeters added area-NavigationView NavView control team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Oct 15, 2020

StephenLPeters approved these changes Oct 16, 2020

View reviewed changes

StephenLPeters added the needs-triage Issue needs to be triaged by the area owners label Oct 16, 2020

ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Oct 19, 2020

marcelwgn reviewed Oct 22, 2020

View reviewed changes

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs Outdated Show resolved Hide resolved

marcelwgn reviewed Oct 22, 2020

View reviewed changes

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs Outdated Show resolved Hide resolved

Improve API testing.

6a7be7e

Felix-Dev commented Oct 31, 2020

View reviewed changes

marcelwgn reviewed Oct 31, 2020

View reviewed changes

dev/NavigationView/NavigationView_ApiTests/NavigationViewTests.cs Outdated Show resolved Hide resolved

Update API test.

acb4acc

Merge branch 'master' into user/Felix-Dev/navview-empty-tooltip-fix

bd079ec

StephenLPeters merged commit 6ab8ecc into microsoft:master Feb 13, 2021

ghost mentioned this pull request Mar 5, 2021

NavigationViewItem: Empty tooltip is shown when content is set to the empty string #3427

Closed

Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Oct 24, 2023

Port microsoft/microsoft-ui-xaml#3430

1d1df8c

Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Oct 24, 2023

Port microsoft/microsoft-ui-xaml#3430 (#582)

55ee269

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NavigationViewItem: Empty tooltip no longer shown #3430

NavigationViewItem: Empty tooltip no longer shown #3430

Felix-Dev commented Oct 15, 2020

marcelwgn commented Oct 16, 2020 •

edited

Loading

Felix-Dev commented Oct 16, 2020

marcelwgn commented Oct 16, 2020 •

edited

Loading

StephenLPeters commented Oct 16, 2020

StephenLPeters commented Oct 16, 2020

Felix-Dev commented Oct 16, 2020 •

edited

Loading

marcelwgn commented Oct 17, 2020 •

edited

Loading

Felix-Dev commented Oct 17, 2020 •

edited

Loading

marcelwgn commented Oct 17, 2020 •

edited

Loading

StephenLPeters commented Oct 21, 2020

Felix-Dev Oct 31, 2020

Felix-Dev Oct 31, 2020

Felix-Dev commented Oct 31, 2020

ranjeshj commented Nov 21, 2020

Felix-Dev commented Nov 23, 2020 •

edited

Loading

StephenLPeters commented Nov 23, 2020

azure-pipelines bot commented Nov 23, 2020

StephenLPeters commented Nov 23, 2020

Felix-Dev commented Nov 23, 2020 •

edited

Loading

StephenLPeters commented Nov 23, 2020

ghost commented Mar 5, 2021

NavigationViewItem: Empty tooltip no longer shown #3430

NavigationViewItem: Empty tooltip no longer shown #3430

Conversation

Felix-Dev commented Oct 15, 2020

Description

Motivation and Context

How Has This Been Tested?

marcelwgn commented Oct 16, 2020 • edited Loading

Felix-Dev commented Oct 16, 2020

marcelwgn commented Oct 16, 2020 • edited Loading

StephenLPeters commented Oct 16, 2020

StephenLPeters commented Oct 16, 2020

Felix-Dev commented Oct 16, 2020 • edited Loading

marcelwgn commented Oct 17, 2020 • edited Loading

Felix-Dev commented Oct 17, 2020 • edited Loading

marcelwgn commented Oct 17, 2020 • edited Loading

StephenLPeters commented Oct 21, 2020

Felix-Dev Oct 31, 2020

Choose a reason for hiding this comment

Felix-Dev Oct 31, 2020

Choose a reason for hiding this comment

Felix-Dev commented Oct 31, 2020

ranjeshj commented Nov 21, 2020

Felix-Dev commented Nov 23, 2020 • edited Loading

StephenLPeters commented Nov 23, 2020

azure-pipelines bot commented Nov 23, 2020

StephenLPeters commented Nov 23, 2020

Felix-Dev commented Nov 23, 2020 • edited Loading

StephenLPeters commented Nov 23, 2020

ghost commented Mar 5, 2021

marcelwgn commented Oct 16, 2020 •

edited

Loading

marcelwgn commented Oct 16, 2020 •

edited

Loading

Felix-Dev commented Oct 16, 2020 •

edited

Loading

marcelwgn commented Oct 17, 2020 •

edited

Loading

Felix-Dev commented Oct 17, 2020 •

edited

Loading

marcelwgn commented Oct 17, 2020 •

edited

Loading

Felix-Dev commented Nov 23, 2020 •

edited

Loading

Felix-Dev commented Nov 23, 2020 •

edited

Loading