[Windows] Fix setting flyout width (#19068) #19070
Conversation
ghost
added
the
|
Hey there @molesmoke! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Eilon
added
the
area/layout 🔲
|
Are you able to add some device tests? |
@mattleibow Basically, no. Although I was able to get the Controls.DeviceTests project to build by removing the non-Windows and profiling projects from the solution filter, I can't for the life of me get the unit tests to run. I was able to verify for myself that it fixed the crash by altering Maui.Controls.Sample, but it doesn't look to me like the following puts the project in a usable state to run the unit tests:
Is there perhaps some documentation on how to run the tests that I'm missing? The "Tests" output window is plastered with errors (17.9.0 Preview 1.1), and I wasn't able to get it to run from the command line either with dotnet test. |
|
@jeremy-visionaid you can't run the device tests through Test Explorer :-( You have to use the Visual Runner. So, set the Device Tests as your startup project and just launch it like you would any maui app. |
molesmoke
force-pushed
the
flyoutwidth
branch
from
e9eea40
to
066d611
Compare
|
@PureWeen That's great thanks, definitely the article I was missing. However, even now knowing the article title, I wasn't able to find it from the other salient pages - or google. Maybe a hyperlink here or there might make that one a bit easier to find? https://github.com/dotnet/maui/blob/main/.github/DEVELOPMENT.md Excluding them from Test Explorer discovery if somehow possible would also be good (since the unit tets work from there, but the device tests don't). Still, in hindsight, I probably should have figured it out on my own 😅 I've updated the commit with a device test that would otherwise crash without the guard. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@dotnet-policy-service agree |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| @@ -329,7 +329,7 @@ void UpdateFlyoutPaneSize() | |||
| if (PaneContentGrid == null) | |||
| return; | |||
|
|
|||
| var newSize = new Size(OpenPaneLength, PaneContentGrid.ActualHeight - PaneContentGrid.RowDefinitions[1].Height.Value); | |||
| var newSize = new Size(OpenPaneLength, Math.Max(PaneContentGrid.ActualHeight - PaneContentGrid.RowDefinitions[1].Height.Value, 0)); | |||
There was a problem hiding this comment.
Is there any reason to let this continue if we know the layout hasn't occurred yet?
would something like this work?
This code is going to have to run a second time with correct values once the flyout is opened so it seems like we shouldn't really set anything quite yet.
var newHeight = PaneContentGrid.ActualHeight - PaneContentGrid.RowDefinitions[1].Height.Value;
if (newHeight < 0)
return;
var newSize = new Size(OpenPaneLength, newHeight);
There was a problem hiding this comment.
Yeah, I was debating whether to go with the early return or the more minimal change. I prefer the early return myself, since I think it's technically better, but ironically thought the more minimal change was likely to be accepted. At the end of the day I just want my app not to crash, so I'm happy to change it.
PaneContentGrid width/height will be zero if the flyout has not yet been shown
molesmoke
force-pushed
the
flyoutwidth
branch
from
066d611
to
278724d
Compare
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thank you so much for your contribution! ✨ |
PaneContentGrid width/height will be zero if the flyout has not yet been shown
Description of Change
Ensures a valid size is calculated for the PaneContentGrid if the flyout has not been shown
Issues Fixed
Fixes #19068