Microsoft-ui-xaml: NumberBox: Background of the SpinButton Popup cannot be distinguished from a default page background in Dark theme

[Felix-Dev]

Describe the bug
The background of the NumberBox SpinButton Popup cannot be distinguished from a default page background in Dark theme.

Steps to reproduce the bug

1. Open the XAML Controls Gallery.
2. Go to the NumberBox page and the specific NumberBox SpinButton example.
3. Make sure the NumberBox is in compact SpinButtonPlacementMode and focus the NumberBox.

Expected behavior
Since the Popup is a transient UI like the ComboBox's dropdown menu or the MenuBarItem flyout menu, it should have the same background like these, as per the acrylic guidance:

| ComboBox | NumberBox |
|-------|------------|
|||

NuGet package version:
Current WinUI master builds.

| Windows 10 version | Saw the problem? |
| :--------------------------------- | :-------------------- |
| Insider Build (xxxxx) | |
| May 2020 Update (19041) | Yes |
| November 2019 Update (18363) | |
| May 2019 Update (18362) | |
| October 2018 Update (17763) | |
| April 2018 Update (17134) | |
| Fall Creators Update (16299) | |
| Creators Update (15063) | |

| Device form factor | Saw the problem? |
| :----------------- | :--------------- |
| Desktop | Yes |
| Xbox | |
| Surface Hub | |
| IoT | |

Additional Context

If I were to work on a PR for this I would likely want to tie this into https://github.com/microsoft/microsoft-ui-xaml/issues/2844 as I would create a dedicated theme resource here for the Popup background.

[StephenLPeters]

@chigy can you weigh in on the appropriate color for this? @SavoySchuler FYI

[Felix-Dev]

For reference, I looked at the ComboBox theme resource file and just copied them over to the NumberBox:
https://github.com/microsoft/microsoft-ui-xaml/blob/f68b281afb086e37d62e3b39380108b10baa88ad/dev/ComboBox/ComboBox_themeresources.xaml#L75

Here are the resulting NumberBox theme resources then (Dark theme example)

<contract7NotPresent:StaticResource x:Key="NumberBoxPopupBackground" ResourceKey="SystemControlBackgroundChromeMediumLowBrush" />
<contract7Present:StaticResource x:Key="NumberBoxPopupBackground" ResourceKey="SystemControlTransientBackgroundBrush" />
<contract7NotPresent:StaticResource x:Key="NumberBoxPopupBorderBrush" ResourceKey="SystemControlForegroundChromeHighBrush" />
<contract7Present:StaticResource x:Key="NumberBoxPopupBorderBrush" ResourceKey="SystemControlTransientBorderBrush" />

where SystemControlTransientBackgroundBrush maps to

<AcrylicBrush x:Key="SystemControlTransientBackgroundBrush" BackgroundSource="HostBackdrop" TintColor="{StaticResource SystemChromeAltHighColor}" TintOpacity="0.8" FallbackColor="{StaticResource SystemChromeMediumLowColor}" />

and SystemControlTransientBorderBrush to

<SolidColorBrush x:Key="SystemControlTransientBorderBrush" Color="#000000" Opacity="0.36" />

in Dark theme.

| Theme | Current | Updated - 1809+ (contract 7 present) | Updated - 1803 and less (contract 7 not present) |
|-----|------|-----|----|
| Light | | ||
| Dark|| ||

I used the ComboBox dropdown theme resources here, as said above. I picked them for some initial experimenting here due to the similarity as both are a popup/flyout and thus their background appearance should probably match.

[mdtauk]

The shadow does not show up well with the pure black of the Dark Theme - so if this were to use Acrylic and borders, it would improve the experience.

[chigy]

@kikisaints , I agree with the suggestion but I want to make sure acrylic does not cause problem with this small surface. Kiki, is the size of this popup big enough not to cause the problem we had with Tooltip that made us decide not to use acrylic there?

[Felix-Dev]

@chigy @kikisaints As a helper, here's how the acrylic brush looks with colored background given this design:

| Light Theme | Dark Theme |
|----|-----|
|||

[Felix-Dev]

@chigy Do you have an update on this?

I checked the CommandBarFlyout control and that is also using an in-app background for its primary command surface:

(And while that CommandBarFlyout has for buttons in the pic above for illustration purposes, it could also have less, such as 2-3 so it would also represent a small surface like the NumberBox spin buttons popup here.)

[chigy]

@Felix-Dev , I was waiting for @kikisaints for her response but she is on vacation at the moment.
Her respond pending, the right design style that match our guidance and existing control would to do acrylic and do 1px border which matches the color that we use on controls like MenuFlyout.

[Felix-Dev]

@chigy Thanks for the update. I will wait for @kikisaints then to give her view on this.

[kikisaints]

I believe less than 32x32 on both axes was the issue. If this new suggestion is bigger than 32 on either axis, it should be fine.

It's also not that we can't do it, it's more that something that small is spending a lot of cycles/performance to render something that's virtually undetectable. So if we feel this acrylic surface (which is in-line with the guidance by being a popup) is actually noticeable as an acrylic surface than I'm all for it.

[Felix-Dev]

@kikisaints The popup spin buttons are currently of size 40x32 (WidthxHeight): https://github.com/microsoft/microsoft-ui-xaml/blob/9b5509d02d1c8d074dbe6b0705d5893bfa87a7d4/dev/NumberBox/NumberBox.xaml#L194

I will hopefully be able today to create another screenshot showing how the design will look with the guidance given by @chigy.

[Felix-Dev]

Her respond pending, the right design style that match our guidance and existing control would to do acrylic and do 1px border which matches the color that we use on controls like MenuFlyout.

@kikisaints @chigy For demo purposes only I modified the NumberBox to show three SpinButton popups next to each other. The MenuFlyout and ComboBox styles use the SystemControlTransientBackgroundBrush theme resource for their transient UIs. The CommandBarFlyout style uses the SystemControlAcrylicElementBrush. So we have three transient UIs but the CommandBarFlyout uses a different background brush for its transient UI.

The SystemControlTransientBackgroundBrush is defined like this for dark mode in the Windows 1903 SDK's Generic.xml file:

<AcrylicBrush x:Key="SystemControlTransientBackgroundBrush" BackgroundSource="HostBackdrop" TintColor="{StaticResource SystemChromeAltHighColor}" TintOpacity="0.8" FallbackColor="{StaticResource SystemChromeMediumLowColor}" />

whereas SystemControlAcrylicElementBrush is defined like this:

<AcrylicBrush x:Key="SystemControlAcrylicElementBrush" BackgroundSource="Backdrop" TintColor="{StaticResource SystemChromeAltHighColor}" TintOpacity="0.8" FallbackColor="{StaticResource SystemChromeMediumColor}" />

As you can see, one major difference between these two brushes is that SystemControlTransientBackgroundBrush uses a background source of "HostBackdrop" and SystemControlAcrylicElementBrush explicitly uses in-app backgroudn acrylic with "Backdrop" as the background source. The fallback color also differs between those two brushes.

Below I've compared both brushes and how they would look like for the NumberBox spin buttons popup:

| Theme | SystemControlTransientBackgroundBrush | SystemControlAcrylicElementBrush |
|----|----|----|
| Light | | |
| Dark | | |

As you can hopefully see there is a slight visual difference here depending on which brush I am using.

I've also made a comparison without between these two brushes without any fancy modifications to the NumberBox or specially colored backgrounds to show the color contrast with the default page backgrounds:

| Theme | SystemControlTransientBackgroundBrush | SystemControlAcrylicElementBrush |
|----|----|----|
| Light | | |
| Dark | | |

Which brush should I use here?

[mdtauk]

@Felix-Dev the border in the Dark Theme, could do with being lighter, to help the buttons pop from the dark background

[Felix-Dev]

@mdtauk The border I am currently using here is the same one used for the MenuFlyout and CommandBarFlyout - SystemControlTransientBorderBrush:

(A CommandBarFlyout shown on the default page background in dark theme)

As the SystemControlTransientBorderBrush is slightly visible in Light theme but not in Dark theme, we could update that theme resource for the dark theme to make the border slightly visible here as well. The new borderbrush would then be reflected across the board for the different popups and flyouts used in WinUI.

@chigy @kikisaints FYI

[mdtauk]

Maybe the top edge highlight should be used for these controls, as with 10X

[chigy]

@mdtauk , @Felix-Dev ,
We use the same brush for the borders across all of our controls. Please stick with the same value.

[mdtauk]

@mdtauk , @Felix-Dev ,
We use the same brush for the borders across all of our controls. Please stick with the same value.

That makes sense, however it is still harder to see the Popup Buttons in the Dark Theme, as it uses a dark border and not a lighter one.

[Felix-Dev]

@chigy I believe @mdtauk's suggestion here is to look at the borderbrush used in dark theme (SystemControlTransientBorderBrush) for transient controls and modify it slighty for better visibility. For example, see the color of the SystemControlTransientBorderBrush in Light theme: You can clearly see the border of the transient control. That's not the case in dark theme for now. The suggestion here is to look at the SystemControlTransientBorderBrush value in dark mode and perhaps update it for better border visibility.

Modifying this brush would not break consistency within WinUI as I believe all transient UIs use that theme resource as their borderbrush.

[chigy]

@mdtauk , @Felix-Dev ,
What I am asking is please don't design new behavior for this control. If you are suggesting to update the border, then please open a separate new GitHub item and address it for all controls. We do not want to introduce one-off.

[Felix-Dev]

@chigy It seems you have missunderstood me. I was not planning to update the border just for the NumberBox control here.

[mdtauk]

It seems to me that if the border is there to improve visibility, the dark theme should use a lighter colour

[Felix-Dev]

@mdtauk Can we move updating the borderbrush for transient UIs in dark mode into its separate issue? It's not a NumberBox spin buttons popup specific issue (though if you plan to open a tracking issue for this you can reference the NumberBox as an example).

[mdtauk]

Feel free to use the images when setting up a new issue.

[chigy]

@chigy It seems you have missunderstood me. I was not planning to update the border _just_ for the NumberBox control here.

@Felix-Dev , I understood you perfectly.
I am asking, please do not introduce a change that impacts all of the controls through an issue that is specifically about NumberBox. Please open a separate one and we will discuss it separately.

[Felix-Dev]

@chigy Ok, then it seems i have missunderstood you :)

Anyway, rest assured, I wasn't thinking about doing that here since that would be an update much broader in scope than just the NumberBox. This will be tracked in its own issue which with @mdtauk's consent I would open later today or tomorrow.

Back to the NumberBox specific issue here: Do you have an opinion on whether I should use the SystemControlTransientBorderBrush theme resource or the SystemControlAcrylicElementBrush theme resource?

[chigy]

@Felix-Dev , I don't know the specific brushes used but please match the color exactly with the MenuFlyout.

[Felix-Dev]

@chigy Alright, will do.