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.

Sign up for GitHub

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

Jump to bottom

Update resources names #3886

Merged
merged 2 commits into from
Jan 5, 2021
Merged

Update resources names #3886

merged 2 commits into from
Jan 5, 2021

Conversation

beervoley
Copy link
Contributor

Change the names of recently checked in resources.

Description

Minor changes in missing resources names that were recently checked in.

@beervoley beervoley added the team-Controls Issue for the Controls team label Jan 4, 2021
@beervoley beervoley requested a review from ranjeshj January 4, 2021 23:47
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jan 4, 2021
@beervoley
Copy link
Contributor Author

@ranjeshj I will run the pipeline after the certificate PR is merged and this branch is updated

@beervoley beervoley removed the needs-triage Issue needs to be triaged by the area owners label Jan 4, 2021
@kmahone @beervoley
Update pfx and cer used for test signing (#3882)
fa047a5 
* update pfx

* update CalendarView.xml master

(cherry picked from commit 022de1b)
@beervoley
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@beervoley
Copy link
Contributor Author

I've also cherry picked the certificate commit so we can merge it into token experiment.

StephenLPeters
StephenLPeters suggested changes Jan 5, 2021
View reviewed changes
dev/CommonStyles/MediaTransportControls_themeresources.xaml
@@ -11,24 +11,24 @@
<StaticResource x:Key="MediaTransportControlsPanelBackground" ResourceKey="GhostFillColorSecondaryBrush" />
<StaticResource x:Key="MediaTransportControlsFlyoutBackground" ResourceKey="SystemControlTransientBackgroundBrush" />
<StaticResource x:Key="MediaTransportControlsThumbBorderBackground" ResourceKey="ControlAAFillColorDefaultBrush" />
<StaticResource x:Key="MediaTransportControlsTimeElapsedText" ResourceKey="TextFillColorSecondaryBrush" />
<StaticResource x:Key="MediaTransportControlsMediaText" ResourceKey="TextFillColorPrimaryBrush" />
<StaticResource x:Key="MediaTransportControlsFillTimeElapsedText" ResourceKey="TextFillColorSecondaryBrush" />
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't just rename resources like this, because there might be an app which references the resource. We should put a resource with the old name and value in the dictionary as well (or a separate dictionary for deprecated resources) and put a comment saying that it is no longer used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it is necessary since this change only resides in token-experiment branch (it's not in master) and was introduced in this commit. Unless a developer cloned the token-experiment branch after Dec 1 and created an app what is very unlikely :)

Copy link
Contributor

Choose a reason for hiding this comment

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

These are new resources so we should be okay to rename them.

Copy link
Contributor

Choose a reason for hiding this comment

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

sweet

@ranjeshj ranjeshj requested a review from YuliKl January 5, 2021 16:40
ranjeshj
ranjeshj reviewed Jan 5, 2021
View reviewed changes
test/MUXControlsTestApp/verification/CalendarView-7.xml
@@ -5828,6 +5828,32 @@
FocusVisualPrimaryBrush=#FF000000
Visibility=Visible
RenderSize=22,27
[Windows.UI.Xaml.Controls.CalendarViewDayItem]
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why calendarView baselines have to be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know for sure but in this PR @kmahone also updated them.
https://github.com/microsoft/microsoft-ui-xaml/pull/3882/checks?sha=f82b1a9660db0b3002b40fdb86e0629b69d64848 - failing check on that PR

Copy link
Member

Choose a reason for hiding this comment

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

I think if you merge the latest from master you should pick up the required change where I updated the master.

@beervoley beervoley merged commit 576b1b5 into feature/token-experiment Jan 5, 2021
@beervoley beervoley deleted the vsiliush/update-resources branch January 5, 2021 19:45
licanhua pushed a commit that referenced this pull request Jan 8, 2021
@beervoley @licanhua
Update resources names (#3886)
7b3c45a 
* Update resources names and fix certificate issue

(cherry picked from commit 022de1b)
@licanhua licanhua mentioned this pull request Jan 8, 2021
Merged
ranjeshj pushed a commit that referenced this pull request Jan 8, 2021
@licanhua @beervoley
Cherry-pick #3898 and #3886 from feature/token-experiment to master (#…
8620603 
…3908)

* Update resources names (#3886)

* Update resources names and fix certificate issue

(cherry picked from commit 022de1b)

* Update ApplicationPageBackgroundThemeBrush (#3898)

Co-authored-by: Vsevolod <sevkorobot@gmail.com>
ranjeshj pushed a commit that referenced this pull request Jan 12, 2021
@licanhua @beervoley
Avoid hard-coded "Segoe MDL2 Assets" (#3923)
99e9139 
* Update resources names (#3886)

* Update resources names and fix certificate issue

(cherry picked from commit 022de1b)

* Update ApplicationPageBackgroundThemeBrush (#3898)

* Avoid hardcoded 'Segoe MDL2 Assets' and use indirect reference for font asset

Co-authored-by: Vsevolod <sevkorobot@gmail.com>
licanhua added a commit that referenced this pull request Jan 14, 2021
@licanhua @beervoley
Remove VerifyRSxxDictionariesWereMergedCorrectly test cases (#3935)
3290f0c 
* Update resources names (#3886)

* Update resources names and fix certificate issue

(cherry picked from commit 022de1b)

* Update ApplicationPageBackgroundThemeBrush (#3898)

* remove VerifyRS2ThemeResourceDictionariesWereMergedCorrectly and other similar test cases

Co-authored-by: Vsevolod <sevkorobot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ranjeshj ranjeshj ranjeshj left review comments

@kmahone kmahone kmahone left review comments

@StephenLPeters StephenLPeters StephenLPeters approved these changes

@YuliKl YuliKl Awaiting requested review from YuliKl

Assignees
No one assigned
Labels
team-Controls Issue for the Controls team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@beervoley @StephenLPeters @ranjeshj @kmahone