-
Notifications
You must be signed in to change notification settings - Fork 677
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
TabViewItem: Background/Foreground APIs are now respected #3216
Closed
Felix-Dev
wants to merge
11
commits into
microsoft:master
from
Felix-Dev:user/Felix-Dev/tabview-foreground-background-api-support
Closed
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
137a796
TabViewItem's Foreground/Background APIs are now respected.
Felix-Dev cb0cc0b
Moved now obsolete TabViewItemIconForeground* theme resources into ne…
Felix-Dev fb66593
Added API tests.
Felix-Dev c235895
forgot to remove comment syntax outcommenting code
Felix-Dev d3ef257
Update winui placeholder version used by MUXControlsReleaseTest.
Felix-Dev 005592c
(partial) update with master
Felix-Dev 983dc90
Merge branch 'user/Felix-Dev/MUXReleaseTestApp-update-nuget-package' …
Felix-Dev 8180c1b
Add test page to MUXControlsReleaseTest app to consume deprecated res…
Felix-Dev 9590297
Move remaining deprecated TabView theme resources into new global res…
Felix-Dev 47f46fc
fix merge conflicts
Felix-Dev 6ee5daf
Fix merge conflict.
Felix-Dev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. --> | ||
<ResourceDictionary | ||
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" | ||
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" | ||
xmlns:local="using:Microsoft.UI.Xaml.Controls"> | ||
|
||
<!-- The resources listed in this file are no longer used by WinUI and have been retained for backward compatibility. | ||
They will be removed in a future major WinUI update. --> | ||
|
||
<ResourceDictionary.ThemeDictionaries> | ||
<ResourceDictionary x:Key="Light"> | ||
<StaticResource x:Key="TabViewItemIconForeground" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundPressed" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundSelected" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundPointerOver" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundDisabled" ResourceKey="SystemControlDisabledBaseMediumLowBrush" /> | ||
<StaticResource x:Key="TabViewButtonBackgroundActiveTab" ResourceKey="SystemControlTransparentBrush" /> | ||
<StaticResource x:Key="TabViewButtonForegroundActiveTab" ResourceKey="SystemControlBackgroundBaseMediumBrush" /> | ||
</ResourceDictionary> | ||
|
||
<ResourceDictionary x:Key="Dark"> | ||
<StaticResource x:Key="TabViewItemIconForeground" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundPressed" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundSelected" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundPointerOver" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundDisabled" ResourceKey="SystemControlDisabledBaseMediumLowBrush" /> | ||
<StaticResource x:Key="TabViewButtonBackgroundActiveTab" ResourceKey="SystemControlTransparentBrush" /> | ||
<StaticResource x:Key="TabViewButtonForegroundActiveTab" ResourceKey="SystemControlBackgroundBaseMediumBrush" /> | ||
</ResourceDictionary> | ||
|
||
<ResourceDictionary x:Key="HighContrast"> | ||
<StaticResource x:Key="TabViewItemIconForeground" ResourceKey="SystemControlForegroundBaseMediumBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundPressed" ResourceKey="SystemControlHighlightAltBaseHighBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundSelected" ResourceKey="SystemControlHighlightAltBaseHighBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundPointerOver" ResourceKey="SystemControlHighlightAltBaseHighBrush" /> | ||
<StaticResource x:Key="TabViewItemIconForegroundDisabled" ResourceKey="SystemControlDisabledBaseMediumLowBrush" /> | ||
<StaticResource x:Key="TabViewButtonBackgroundActiveTab" ResourceKey="SystemControlTransparentBrush" /> | ||
<StaticResource x:Key="TabViewButtonForegroundActiveTab" ResourceKey="SystemControlBackgroundBaseMediumBrush" /> | ||
</ResourceDictionary> | ||
</ResourceDictionary.ThemeDictionaries> | ||
|
||
</ResourceDictionary> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we verify that this change doesn't break apps? Basically that an app that references this resource continues to work after this change. I suspect it does but we should confirm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested that locally here and and apps still work fine using these resources. Can create a test for this if required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stmoy would it be possible to make XCG reference one of these resources so that we can validate that it still works when we update the release version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Felix-Dev I think better than using the xcg would be to add something to the IXMP test app that references these moved resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@StephenLPeters I'm having troubled adding the IXMP test projects to my VS solution. While I can add the
IXMPTestApp
andIXMPTestApp.TAEF
projects seemingly fine, adding the sharedIXMPTestApp.Shared
project seems to work not correctly:These two top-level folders (3c184...., da296....) - I assume - should not be shown in the Solution explorer and I have no idea why VS just decides to add them. They do not exist on the file system.
I suspect these are causing issues here because while I can build the
IXMPTestApp
project, I cannot run it. The run process is terminated with error messagebefore even the Application constructor is called (no breakpoint hit).
Thoughts here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@StephenLPeters Would something like this look okay for now?
(I am using a 2px gray border around the theme resource value here to act as a visual border in case the page background has the same color as the theme resource value.)
This is a new page in the MUXControlsReleaseTest apps which for now tests one of the now deprecated tabview theme resources (so this single resource test is meant to act as a test for all theme resources in question here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly are we testing through this? What would happen if the resource disappears/breaks, would we be able to notice that through the CI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are testing here that moving the resources into the new deprecated theme resource file will not break apps. It is my understanding that the CI will then run the tests (launch the app and navigate to the different test pages,...) on the newest available WinUI nuget package and that a missing resource will thus be noticed because the app will crash after navigating to the page consuming said resource.
@StephenLPeters to confirm though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is more than enough, I wouldn't really expect anyone to look at it so as long as the page load the resource I think its sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the proposed new page to the MUXControlsReleaseTest app now (and updated the WinUI placeholder version).