-
Notifications
You must be signed in to change notification settings - Fork 712
Replace MainWindow with plain Window in sample projects #2063
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
Conversation
| private void createNewWindow_Click(object sender, RoutedEventArgs e) | ||
| { | ||
| var newWindow = new MainWindow(); | ||
| var newWindow = new Window |
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.
Reading this was painful 😅, can't we just create a NewWindow or SecondaryWindow?
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.
My intension was to avoid adding custom code to the sample as much as possible, but sure I can do that.🙂
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 agree with the both of you, its difficult to read but for the sake of the sample we should not rely on helpers. Do we need to need to have ThemeHelper provide the theme?
Also can we call the constructor instead of omitting the "()"? 😄
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.
for the sake of the sample we should not rely on helpers
I completely agree.
Do we need to need to have ThemeHelper provide the theme?
If we want the new window use the app theme, which might differ from the system theme, we should set it here using ThemeHelper. We can remove it from the sample code (CreateWindowSample1.txt).
We can get the theme from the parent.
Also can we call the constructor instead of omitting the "()"? 😄
Yes, we can, but I'll add them back since I'm not sure is preferred or not. 🙂
| private void createNewWindow_Click(object sender, RoutedEventArgs e) | ||
| { | ||
| var newWindow = new MainWindow(); | ||
| var newWindow = new Window |
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 agree with the both of you, its difficult to read but for the sake of the sample we should not rely on helpers. Do we need to need to have ThemeHelper provide the theme?
Also can we call the constructor instead of omitting the "()"? 😄
|
/azp run |
Description
This PR updates
WindowHelper.CreateWindow()to return a plainWindowinstead ofMainWindow, which was tightly coupled to several static members.Motivation and Context
Fixes #2062.
How Has This Been Tested?
Manually tested.
Screenshots (if appropriate):
Types of changes