Proposal: lightweight styling for NavigationView top button spacing

[Sergio0694]

Proposal: lightweight styling for NavigationView top button spacing

This issue is a follow up from a conversation we've had with some WinUI engineers and other developers over at the UWP Community discord server, specifically the thread starting with this message.

Summary

Allow developers to customize the spacing of the buttons in the NavigationView in top navigation mode through lightweight styling (by overriding a specific resource that's used in the template), instead of forcing them to use explicit code that interacts with the visual tree directly.
This results in an easier experience for developers, which is also more portable and less error prone (and with the additional benefit of requiring no code behind trickery).

Additional info

I'm trying to create the following look for buttons in the NavigationView, with a 2px spacing:

This is different from the default look, which uses no spacing at all, which I personally don't like for stacked buttons with the reveal highlight effect:

The template for the NavigationView is here, and I've linked the area reserved to the top buttons. As you can see, the ItemsRepeater is not using any resource to set the Spacing property, making it impossible to use lightweight styling for this. Furthermore, the "More" button doesn't have spacing either, forcing users to also manually set some left margin there to keep the look consistent with the buttons. Here's the code I'm using right now, under a Loaded handler for the NavigationView:

NavigationView navigationView = (NavigationView)sender;

if ((navigationView.FindDescendantByName("TopNavMenuItemsHost") as ItemsRepeater)?.Layout is StackLayout panel)
{
    panel.Spacing = 2;
}

if (navigationView.FindDescendantByName("TopNavOverflowButton") is Button button)
{
    button.Margin = new Thickness(2, 0, 0, 0);
}

This is not really ideal, for obvious reasons. It'd be nice to be able to set the spacing for both buttons with a simple resource that can be overridden with some lightweight styling.

Rationale

• Allow users to easily customize the buttons spacing
• Discourage the use of weird tricks with code behind (see solution above)

Scope

Capability	Priority
Use a resource to control the buttons spacing	Must
Use a resource for the margin of the "More" button	Must
Use the same resource to control both the points above	Should

Open Questions

Not really related, but...
Why is the "More" button lacking the reveal highlight completely?
See here:

I think it should use the same exact visual style as the other top navigation buttons.

[Felix-Dev]

I would like to try fixing this.

My current thinking is to introduce at least the following resources (and potentially more as I further look into NavigationView, such as OverflowButton/Header/AutoSuggestBox margin resources):

<!--NavigationViewItem margins for NavigationView DisplayMode Top & Left-->
<Thickness x:Key="TopNavigationViewItemMargin">0</Thickness>
<Thickness x:Key="NavigationViewItemMargin">0</Thickness>

<!--NavigationViewItemSeparator margins for NavigationView DisplayMode Top & Left-->
<Thickness x:Key="TopNavigationViewItemSeparatorMargin">10,0</Thickness>
<Thickness x:Key="NavigationViewItemSeparatorMargin">16,10</Thickness>

<!--NavigationViewItemSettings margins for NavigationView DisplayMode Top & Left-->
<Thickness x:Key="TopNavigationViewItemSettingsMargin">0</Thickness>
<Thickness x:Key="NavigationViewItemSettingsMargin">0</Thickness>

As such, this would also minimally cover #2139.

Thus, if adding resources for easier NavigationView theming is desired and @ojhad is not working on adding them already, I would like to look into this.

[YuliKl]

Thank you, @Felix-Dev, this approach sounds great.

[Felix-Dev]

@YuliKl I currently settled on the following theme resources to introduce with a PR for this issue:

• NavigationViewItemMargin
• TopNavigationViewItemMargin
• TopNavigationViewOverflowButtonMargin
• TopNavigationViewAutoSuggestBoxMargin

I would also like to add the following theme resource:

• TopNavigationViewItemHeaderInnerMargin

I separated this theme resource from the ones above as the addition of this one would be a UI breaking change. Currently, the margin of the NavigationViewItemHeader can be set in conjunction via its NavigationViewItemHeader.Margin API and the existing theme resource NavigationViewItemInnerHeaderMargin. The latter is used in both Top and Left display modes of the NavigationView, so adding a specific theme resource for the margin in Top display mode could break existing customized NavigationViewItemHeader layouts in Top display mode.
Furthermore, the naming used here for the theme resource is not consistent with the standard theme resource naming in the NavigationView and suggests it influences the margin of the inner header of the NavigationViewItem which is incorrect: It controls the inner margin of a NavigationViewItemHeader and that should be reflected in the naming. Thus, I also propose to rename the existing theme resource from NavigationViewItemInnerHeaderMargin to NavigationViewItemHeaderInnerMargin. In order to not break apps, I would keep the current NavigationViewItemInnerHeaderMargin theme resource (and add a comment in the resource dictionary that it is no longer in use) while adding the new NavigationViewItemHeaderInnerMargin theme resource..

While I do think the lightweight customization options for the margin of the NavigationViewItemHeader should be improved as outlined above, this won't be absolutely necessary here in a PR covering this issue. The NavigationViewItemHeader.Margin API can be used to set different header margins in both Top and Left pane display modes if so required, albeit not as elegant as with theme resources. I personally would liek to make the changes outlined above though as part of this PR.

Alongside PR #2206 which enables lightweight customization of the NavigationViewItemSeparator margin these new theme resources will provide ample flexibility to customers to customize the margins of the NavigationView pane elements in top mode.

[Felix-Dev]

@YuliKl Friendly ping.

[YuliKl]

I'm definitely in favor of adding the first four resources you suggested:

• NavigationViewItemMargin
• TopNavigationViewItemMargin
• TopNavigationViewOverflowButtonMargin
• TopNavigationViewAutoSuggestBoxMargin

I feel less good about TopNavigationViewItemHeaderInnerMargin. I can't dispute your observation that NavigationViewItemInnerHeaderMargin feels incorrectly named, but I fear we may be stuck with this resource name. This comment does not feel right to me:

In order to not break apps, I would keep the current NavigationViewItemInnerHeaderMargin theme resource (and add a comment in the resource dictionary that it is no longer in use)

because it introduces dead weight, and breaks any existing customization - app developers would need to notice that the resource is no longer in use and change their code to customize a differently named resource. So I would recommend omitting this last change from a PR.

[Felix-Dev]

@YuliKl

I can't dispute your observation that NavigationViewItemInnerHeaderMargin feels incorrectly named, but I fear we may be stuck with this resource name

I do hope that at some point we can use semantic versioning to look at all these theme resources which are currently lacking precision or are incorrectly named so that we can make these breaking changes (by just removing outdated or incorrectly named theme resources) at some point in the future.

This comment does not feel right to me [...] because it introduces dead weight, and breaks any existing customization

It seems to me as if the team is looking at this on a case-by-case basis as we had cases like these where we were left with now unsued theme resources. It was decided to keep them for now so that we won't cause apps to crash at runtime (in the slight chance they are using theme resources in a way which would lead the app to crash). This leads back to my first remark - hoping that at some point for some WinUI verison, we can look at this holistically for WinUI and accept the possibly generated "developer pain" to provide a consistent and well-named lightweight-styling experience.

So to conclude:
I will add the four theme resources you've OK'd. As for the NavigationViewItemHeader: Developers can use the theme resource NavigationViewItemInnerHeaderMargin to configure the "inner content margin" of the NavigationViewItemHeader independently of the pane display mode. If developers need to set a NavigationViewItemHeader margin dependent on the pane display mode (for example some additional top margin in Left mode or some additional left margin in top mode) then they will have to use the NavigationViewItemHeader.Margin API and listen to pane display mode changes (for example in visual states where they switch the pane display mode between Auto and Top, for example). This is unfortunately not consistent with the rest of the theme resources we would offer to developers here.

Does that sound ok to you, @YuliKl?

[YuliKl]

Thank you, @Felix-Dev, this sounds good.

@MikeHillberg - do you have any thoughts about the unused or misnamed theme resources, and when an opportune time may present itself to clean things up?

[ranjeshj]

From my understanding WinUI 4.0 if we stick to semver.