-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix "Italic" checkstate bug + fix "italic" typo in InfiniteCanvas control #3117
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
Fix "Italic" checkstate bug + fix "italic" typo in InfiniteCanvas control #3117
Conversation
itlaic -> italic
Thanks BurkusCat 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 🙌 |
f5370c5
to
a4d7309
Compare
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.
Thanks for the fix @BurkusCat. The changes look good!
@@ -166,7 +166,7 @@ protected override void OnApplyTemplate() | |||
_canvasTextBoxTools = (StackPanel)GetTemplateChild("CanvasTextBoxTools"); | |||
_canvasTextBoxColorPicker = (ColorPicker)GetTemplateChild("CanvasTextBoxColorPicker"); | |||
_canvasTextBoxFontSizeTextBox = (TextBox)GetTemplateChild("CanvasTextBoxFontSizeTextBox"); | |||
_canvasTextBoxItlaicButton = (ToggleButton)GetTemplateChild("CanvasTextBoxItlaicButton"); | |||
_canvasTextBoxItalicButton = (ToggleButton)GetTemplateChild("CanvasTextBoxItalicButton"); |
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 was going to say changing the name in the Style is going to be a breaking change, but we don't actually call them out as TemplatePart
s like we should... (for instance see TabView).
@BurkusCat would you be up for fixing that too? Let me know if you want to do it as part of this change or another.
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.
@michael-hawker I can take a look at that next weekend.
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.
@BurkasCat I'm approving this PR, just let me know, but we can merge this one first, and just open another one for those changes if you get to them. Thanks for the contribution! 🎉
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.
@BurkusCat Is there any update on this or still being worked on?
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.
@Kyaa-dost Apologies, I did not get a chance to look at this. I can try take a look at this on this weekend if it suits.
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 30 days of this comment. |
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 can fix the template part in another PR. Let's just merge this. It looks good.
Sorry, hectic times as you can imagine 🙈 Thanks for approving! |
@BurkusCat no worries, that was my intent, we'll get this merged in now, and I'll quickly do the TemplatePart. I'm actually working on this on a live stream at the moment now :) |
PR Type
What kind of change does this PR introduce?
Bugfix
Code style update (formatting)
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
Other information