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

Use regular resource lookup for FocusVisualStyle #1165

Merged
merged 4 commits into from
Jul 13, 2022
Merged

Use regular resource lookup for FocusVisualStyle #1165

merged 4 commits into from
Jul 13, 2022

Conversation

batzen
Copy link
Contributor

@batzen batzen commented Jul 3, 2019 •
edited
Loading

This fixes #1164

Description

Because KeyboardNavigation uses SystemResources.FindResourceInternal instead of FrameworkElement.FindResourceInternal the default FocusVisualStyle can't be overwritten globally.

Customer Impact

Fixes a regression.

Regression

Yes.

Testing

Where can i add tests for this?

Risk

Low. Adds ability to overwrite FocusVisualStyle.

@ghost ghost requested review from vatsan-madhavan, rladuca, ryalanms and stevenbrix July 3, 2019 19:52
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jul 3, 2019
@ghost ghost requested a review from SamBent July 3, 2019 19:52
@lindexi
Copy link
Member

lindexi commented Jan 16, 2021

@batzen Thank you and I pick your code to my custom wpf repo.

Base automatically changed from master to main March 17, 2021 17:38
@ryalanms ryalanms requested a review from a team as a code owner March 17, 2021 17:38
@batzen
Copy link
Contributor Author

batzen commented Mar 12, 2022

@dotnet/wpf-developers any news on this one?

ThomasGoulet73
ThomasGoulet73 reviewed Mar 12, 2022
View reviewed changes
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! Could you update your PR to use the template, this PR was probably opened before the template was added but the WPF team will probably want to know a couple of things in the template to consider whether or not to merge this PR, like "Testing" and "Risk".

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Input/KeyboardNavigation.cs Outdated Show resolved Hide resolved
src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Input/KeyboardNavigation.cs Outdated Show resolved Hide resolved
@batzen
Copy link
Contributor Author

batzen commented Mar 12, 2022

Nice! Could you update your PR to use the template, this PR was probably opened before the template was added but the WPF team will probably want to know a couple of things in the template to consider whether or not to merge this PR, like "Testing" and "Risk".

To be honest, i don't understand why i should do that after waiting for 3 years to get any feedback...

batzen and others added 2 commits March 12, 2022 20:40
@batzen @ThomasGoulet73
Update src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Wind…
6d2f93a 
…ows/Input/KeyboardNavigation.cs

Co-authored-by: ThomasGoulet73 <[email protected]>
@batzen @ThomasGoulet73
Update src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Wind…
cab9be4 
…ows/Input/KeyboardNavigation.cs

Co-authored-by: ThomasGoulet73 <[email protected]>
@ThomasGoulet73
Copy link
Contributor

To be honest, i don't understand why i should do that after waiting for 3 years to get any feedback...

That's true but I'm just trying to anticipate comments that the WPF team might have to avoid back and forth which would delay this PR.

@lindexi
Copy link
Member

lindexi commented Mar 14, 2022

Where can i add tests for this?

https://github.com/dotnet/wpf-test/

@batzen
Copy link
Contributor Author

batzen commented Mar 14, 2022

@lindexi thanks, but I was already aware of that repo. The question was more like "were can i find the existing tests for FocusVisual?". As a search in the test repo didn't yield any results.

@lindexi
Copy link
Member

lindexi commented Mar 14, 2022

@batzen LoL, it is seems the test repo is slowly too.

@batzen batzen added the Community Contribution A label for all community Contributions label May 5, 2022
@ghost ghost assigned batzen May 5, 2022
lindexi added a commit to dotnet-campus/dotnetCampus.CustomWpf that referenced this pull request Jun 9, 2022
@lindexi
Cherry pick Use regular resource lookup for FocusVisualStyle by batzen
304c0c6 
dotnet/wpf#1165
@lindexi lindexi mentioned this pull request Jun 9, 2022
Open
@lindexi
Copy link
Member

lindexi commented Jun 9, 2022

@batzen I reorganized my custom wpf test repo and pick your code. Thank you.

@singhashish-wpf
Copy link
Member

This PR will be taken in the next community test pass.

lindexi added a commit to dotnet-campus/wpf that referenced this pull request Jul 11, 2022
@lindexi
Cherry Use regular resource lookup for FocusVisualStyle
be4d447 
dotnet#1165
@singhashish-wpf singhashish-wpf merged commit 43adc20 into dotnet:main Jul 13, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ThomasGoulet73 ThomasGoulet73 ThomasGoulet73 left review comments

@vatsan-madhavan vatsan-madhavan Awaiting requested review from vatsan-madhavan

@rladuca rladuca Awaiting requested review from rladuca

@ryalanms ryalanms Awaiting requested review from ryalanms

@stevenbrix stevenbrix Awaiting requested review from stevenbrix

@SamBent SamBent Awaiting requested review from SamBent

Assignees

@batzen batzen

Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
WPF Community Projects
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FocusVisualStyle can't be overwritten globally
5 participants
@batzen @lindexi @ThomasGoulet73 @singhashish-wpf @dipeshmsft