-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Set Fixed RootView Width in E2ETest #7683
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
Fixes microsoft#7130 Should lead to test stability when the window is resized.
| x:Name="myRootView" | ||
| ComponentName="ReactUWPTestApp" | ||
| Background="{ThemeResource ApplicationPageBackgroundThemeBrush}" | ||
| Width="800" |
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.
Not sure if this is the default width, but CI should tell me 🤷♀️
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.
Snapshots match, so I think this was right.
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.
1074 is what we've needed so far, e.g. the display:none PR
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 looked through some snapshots and noticed that width, but also a lot of 800 width outer views.
IIRC my guess was that some views may overflow or be horizontally scrollable?
I previously verified changing root view height affect tree dumps, so my assumption would be that width should have a similar effect. I didn't validate that a template uwp app has an 800x600 window size, but that number would make sense, and seems to lead to consistent snapshots.
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.
Confirmed 800x600 in code
| ApplicationView.GetForCurrentView().TryResizeView(new Size(800, 600)); |
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.
Though that does make me wonder if she issue Agne was seeing in #7589 is the same as the issue this solves. It looks like we already resize every app launch. Might be good to confirm.
|
Hello @NickGerleman! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 8 hours 20 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Fixes #7130
Should lead to test stability when the window is resized.
Microsoft Reviewers: Open in CodeFlow