Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NumberBox: Updated the border and background of the spin button popup to match Fluent Design guidelines. #3130

Conversation

Felix-Dev
Copy link
Contributor

@Felix-Dev Felix-Dev commented Aug 14, 2020

Description

This PR updates the border and background brushes of the NumberBox spin button popup (showhen when the NumberBox is set to a SpinButtonPlacementMode of [Compact]. As the popup is a transient UI, per the acrylic guidelines it should sport in-app acrylic as its default background.

As an additional benefit, the popup can now easily be distinguished from the default page background in dark theme.

The updated brushes have been taken from the MenuFlyout default style as requested by the team. Below you find a table showing the updated design:

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

The in-app acrylic effect is noticeable as you can see here and below:

image

Light Theme Dark Theme
image image

In High-contrast mode, the look remains essentially unchanged from today:

Theme Current Updated
Light image image
Dark image image

Implementation Notes

In order to implement these changes, I introduced a couple new theme resources:

  • NumberBoxPopupBackground
  • NumberBoxPopupBorderBrush
  • NumberBoxPopupBorderThickness
  • NumberBoxPopupSpinButtonBackground

These resources where required because prior to this PR, the Popup's background, border brush & thickness were driven by the following theme resources:

  • SystemControlBackgroundAltHighBrush (Background)
  • ToolTipBorderBrush (BorderBrush)
  • ToolTipBorderThemeThickness (BorderThickness)

The new NumberBoxPopupSpinButtonBackground has been introduced to set the background of Popup's spin buttons to transparent so that we will get the full acrylic effect. Previously, the background of a spin button was set by the TextControlBackground theme resource to a background not fully transparent.

Updating these resources was not an option as they would have affected controls other than the NumberBox as well, so I made the decision here to introduce a couple of NumberBox specific theme resources - which is something which should be done anyway (see tracking issue #2844).

Motivation and Context

Fixes #2843.

How Has This Been Tested?

Tested visually (see screenshots above).

Additional Notes

@chigy @kikisaints FYI.

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Aug 14, 2020
Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

@kikisaints @YuliKl and/or @SavoySchuler can you take a look at this change from a PM perspective?

@StephenLPeters StephenLPeters added area-NumberBox NumberBox Control team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Aug 17, 2020
@kikisaints
Copy link

kikisaints commented Aug 19, 2020

Looks good on my end... :shipit:

@StephenLPeters StephenLPeters merged commit f01c448 into microsoft:master Aug 20, 2020
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Aug 20, 2020
@SavoySchuler
Copy link
Member

This also looks good to me.

@MikeHillberg, I just want to double check, do the theme resource names used here require any kind of review/approval?

  • NumberBoxPopupBackground
  • NumberBoxPopupBorderBrush
  • NumberBoxPopupBorderThickness
  • NumberBoxPopupSpinButtonBackground

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NumberBox NumberBox Control team-Controls Issue for the Controls team
Projects
None yet
6 participants