Feature: Improved visual elements in the content area #13054

0x5bfa · 2023-07-24T14:07:01Z

Feature: Improved visual elements in the content area

Motivation & Context

Improved DataGridHeader control
Improved divider controls in High Contrast (red to white in dark HC mode)
Improved divider margin

PR Checklist

Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
Closes Bug: Divider color is red in High Contrast #13077
Closes Bug: Backplates will be displayed when hovering in High Contrast #13078
Did you build the app and test your changes?
Did you check for accessibility? You can use Accessibility Insights for this.
Did you remove any strings from the en-us resource file?
- Did you search the solution to see if the string is still being used?
Did you implement any design changes to an existing feature?
- Was this change approved?
Are there any other steps that were used to validate these changes?

Screenshots

None

mdtauk · 2023-07-25T10:49:33Z

mdtauk · 2023-07-25T10:51:19Z

High Contrast

0x5bfa · 2023-07-25T12:55:32Z

A bit off? if so, padding should be 17 from 16.

mdtauk · 2023-07-25T13:25:42Z

!

The text should not have a backplate in High Contrast.

Some time ago the WinUI team took steps to fix these issues
microsoft/microsoft-ui-xaml#1663 (comment)

…a/Files into 5bfa/Update-DataGridHeader

0x5bfa · 2023-07-25T13:46:42Z

There's a lot of backplates in Files. that improvement should be out of the scope but I can if yair'd like.

Adding HighContrastAdjustment="None" will fix, then?

mdtauk · 2023-07-25T13:56:58Z

There's a lot of backplates in Files. that improvement should be out of the scope but I can if yair'd like.

Adding HighContrastAdjustment="None" will fix, then?

I am not sure of the technical detail, only that it was a change made in preparation for Windows 11.

https://learn.microsoft.com/en-gb/uwp/api/windows.ui.xaml.uielement.highcontrastadjustment?view=winrt-22621

The change should be made across most of Files' UI in High Contrast

…dHeader

yaira2 · 2023-07-25T20:25:05Z

@0x5bfa can you link the issue/feature request for this PR?

0x5bfa · 2023-07-26T14:37:49Z

Adjust placements of datagrid.
See that was really fixed.

yaira2 · 2023-07-26T14:41:51Z

src/Files.App/Views/PaneHolderPage.xaml

@@ -122,7 +122,7 @@
 				<icore:ChangePropertyAction
 					PropertyName="CurrentInstanceBorderBrush"
 					TargetObject="{Binding ElementName=PaneLeft}"
-					Value="{ThemeResource ControlStrokeColorDefault}" />
+					Value="{ThemeResource DividerStrokeColorDefaultBrush}" />


Is this correct? I think we need a custom brush so that we have the correct color in each theme.

I've confirmed that brush is appropriate in light(light dark), dark(light dark), high contrast(white). But previous one was red in hc theme. but we probably need to wrap this brush like App.Theme.DividerStrokeColorDefaultBrush.

but we probably need to wrap this brush like App.Theme.DividerStrokeColorDefaultBrush.

That was my assumption

src/Files.App/Views/LayoutModes/DetailsLayoutBrowser.xaml

yaira2 · 2023-07-26T14:44:34Z

src/Files.App/App.xaml

@@ -2,7 +2,8 @@
 <Application
 	x:Class="Files.App.App"
 	xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
-	xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
+	xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+	HighContrastAdjustment="None">


What does this property do?

What does this property do?
It is used to remove the backplate from text inside elements - but when activated, you will need to make sure every element has the correct foreground colour with the background colours.

https://learn.microsoft.com/en-gb/uwp/api/windows.ui.xaml.uielement.highcontrastadjustment?view=winrt-22621

written in the remarks section.

@mdtauk adding it in the app.xaml doesn't work. so added each page, i confirmed they are at work.

Let's hold off on this change, I think we need to do some testing to see how it affects the rest of the app.

we can test in this PR. This PR's purpose is to improve visual elements and so I think it is not out of the scope of the PR.

And I tested in the contents area and the settings area. The controls from MUXC like TabView has already applied this property in the custom style (custom TabView style).

@mdtauk adding it in the app.xaml doesn't work. so added each page, i confirmed they are at work.

This seems to be a bug, do you know if it's reported on the WinUI repo yet?

yaira2 · 2023-07-26T17:18:04Z

@Jay-o-Way we're well aware of this recommendation and we follow it around 95% of the time, but it's not always possible and it doesn't always make sense. As a side point, it's worth mentioning that according to WinUI, this rule doesn't apply anymore but we still follow it for consistency.

mdtauk · 2023-07-27T01:33:25Z

FYI

Note If you do use measurement values, all dimensions, margins, and padding should be in increments of 4 epx. When XAML uses effective pixels and scaling to make your app legible on all devices and screen sizes, it scales UI elements by multiples of 4. Using values in increments of 4 results in the best rendering by aligning with whole pixels.

https://learn.microsoft.com/windows/apps/design/layout/alignment-margin-padding

I have been sure to adhere to this with the designs. Occasionally like with the sidebar items, we have 2px for top and bottom padding - so when stacked vertically, you get a 4px gap between items. So you will see raw values in xaml that are not in multiples of 4, but when placed into the layouts, will in fact, meet this expectation.

…a/Files into 5bfa/Update-DataGridHeader

yaira2 · 2023-07-28T02:42:58Z

src/Files.App/App.xaml

@@ -26,6 +27,10 @@
 			<CornerRadius x:Key="GridViewThumbnailCornerRadius">2</CornerRadius>
 			<CornerRadius x:Key="DetailsLayoutThumbnailCornerRadius">2</CornerRadius>

+			<Style TargetType="FlyoutPresenter">
+				<Setter Target="HighContrastAdjustment" Value="None" />
+			</Style>


Let's hold off on this change.

Actually I fixed #13128 here lol

0x5bfa added 3 commits July 24, 2023 23:04

Convert into custom control

a4b91f6

Update

170691d

Merge branch 'main' into 5bfa/Update-DataGridHeader

20a2d93

0x5bfa added 2 commits July 25, 2023 22:40

Update visuals

e7b2e7a

Merge branch '5bfa/Update-DataGridHeader' of https://github.com/0x5bf…

e46caea

…a/Files into 5bfa/Update-DataGridHeader

0x5bfa changed the title ~~Feature: Replace DataGridHeader with ResourceDicrionary-based one~~ Feature: Improved visual elements in the content area Jul 25, 2023

0x5bfa added 2 commits July 25, 2023 23:01

Added HighContrastAdjustment

232968a

Merge remote-tracking branch 'upstream/main' into 5bfa/Update-DataGri…

6d6f0ca

…dHeader

0x5bfa marked this pull request as ready for review July 25, 2023 15:52

Merge branch 'main' into 5bfa/Update-DataGridHeader

9372e5f

yaira2 reviewed Jul 26, 2023

View reviewed changes

src/Files.App/Views/LayoutModes/DetailsLayoutBrowser.xaml Show resolved Hide resolved

yaira2 reviewed Jul 26, 2023

View reviewed changes

src/Files.App/Views/LayoutModes/DetailsLayoutBrowser.xaml Show resolved Hide resolved

yaira2 reviewed Jul 26, 2023

View reviewed changes

This comment was marked as off-topic.

Sign in to view

0x5bfa added 2 commits July 27, 2023 22:00

Fixed

4653151

Merge branch '5bfa/Update-DataGridHeader' of https://github.com/0x5bf…

b32b925

…a/Files into 5bfa/Update-DataGridHeader

yaira2 reviewed Jul 28, 2023

View reviewed changes

0x5bfa closed this Aug 25, 2023

0x5bfa deleted the 5bfa/Update-DataGridHeader branch September 16, 2023 05:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Improved visual elements in the content area #13054

Feature: Improved visual elements in the content area #13054

0x5bfa commented Jul 24, 2023 •

edited

Loading

mdtauk commented Jul 25, 2023

mdtauk commented Jul 25, 2023 •

edited

Loading

0x5bfa commented Jul 25, 2023 •

edited

Loading

mdtauk commented Jul 25, 2023

0x5bfa commented Jul 25, 2023 •

edited

Loading

mdtauk commented Jul 25, 2023

yaira2 commented Jul 25, 2023

0x5bfa commented Jul 26, 2023 •

edited

Loading

yaira2 Jul 26, 2023

0x5bfa Jul 27, 2023

yaira2 Jul 28, 2023

yaira2 Jul 26, 2023

mdtauk Jul 27, 2023 •

edited

Loading

0x5bfa Jul 27, 2023

0x5bfa Jul 28, 2023

0x5bfa Jul 28, 2023

yaira2 Jul 28, 2023

0x5bfa Aug 1, 2023

0x5bfa Aug 1, 2023

yaira2 Sep 21, 2023

This comment was marked as off-topic.

yaira2 commented Jul 26, 2023

This comment was marked as off-topic.

mdtauk commented Jul 27, 2023

yaira2 Jul 28, 2023

0x5bfa Aug 5, 2023

0x5bfa Aug 5, 2023

Feature: Improved visual elements in the content area #13054

Feature: Improved visual elements in the content area #13054

Conversation

0x5bfa commented Jul 24, 2023 • edited Loading