Skip to content

#3185 - Oddness with Theming and Selection in InfiniteCanvas Control #3194

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

Merged
merged 5 commits into from
Mar 31, 2020

Conversation

deanchalk
Copy link

Fixes #
#3185

PR Type

What kind of change does this PR introduce?
Bugfix

What is the current behavior?

The InfiniteCanvas control had issues with the highlighting of the 'Bold' toggle button in the toolbar once a TextBox had been added to the Canvas.
The reason for this issue is because the original developer in an attempt to set the TextBox's SelectionHighlight color, had directly changed the bound theme brush's color - affecting the whole application.
The code that performed this change didnt actually have the desired effect anyway because in UWP the TextSelection background color ignores the opacity setting and color alpha channel.
By removing this ineffective code, I avoided the corruption of the Theme brush's color

What is the new behavior?

The 'Bold' Toggle button now adheres to theming correctly

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x ] Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • [x ] Contains NO breaking changes

Other information

@ghost
Copy link

ghost commented Mar 25, 2020

Thanks deanchalk for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost assigned michael-hawker Mar 25, 2020
Copy link
Contributor

@Kyaa-dost Kyaa-dost left a comment

Choose a reason for hiding this comment

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

@deanchalk Thanks for the fix. Perfectly highlighted in all different themes now.

@deanchalk
Copy link
Author

@deanchalk Thanks for the fix. Perfectly highlighted in all different themes now.

Glad to be useful :)

@michael-hawker
Copy link
Member

Thanks @deanchalk, I'll try this out later this afternoon on my machine too. I was surprised to find the XAML for this component buried in the main component's XAML here. I wonder why we had code-behind changing some highlight colors in the past? 🤔 Anyway, great find! 🦙❤

@michael-hawker michael-hawker added this to the 6.1 milestone Mar 31, 2020
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

🦙❤ @deanchalk thanks, that was huge find that it was corrupting the whole theme state! This is a great fix.

@michael-hawker michael-hawker merged commit 45cdd25 into CommunityToolkit:master Mar 31, 2020
@michael-hawker michael-hawker mentioned this pull request Apr 1, 2020
36 tasks
@Kyaa-dost Kyaa-dost added bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ labels Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants