Make panes use XAML star sizing #4953
Conversation
carlos-zamora
added
the
Area-User Interface
|
After looking at it for a while, I think it's not worth it. We calculate the minimum size of each pane (and snap it to the text buffer grid) using pixels. We could probably rewrite all of that to just create a ratio. But that's risky and not really worth it. Also, I could be wrong about this, but I'd imagine it's not a huge perf impact anyways. |
|
I was less worried about perf and more worried about how when we start up the terminal our size is 0x0 and when we try to do math on that we hyperexplode 😄 |
|
(This is why new-tab and split-pane "defer" their execution, in part) |
|
Ewwww. So you're worried about |
carlos-zamora
added
the
AutoMerge
|
Hello @carlos-zamora! Because this pull request has the 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 (
|
ghost
merged commit
|
I'm not worried about 0 being passed to xaml, I'm more worried about us saying "our size was 0, and we have a 4-pixel pane separator, so your size is -4 and YOUR size is -4, good luck" then xaml explodes us |
|
Oh, well I only wrote that this seems like a perfect solution. But then I realized that it is not and I was about to comment it, and then I ended up seeing that this got merged and I that I apparently forgot about it ;). So because of the snapping and 0-size issues it may actually be easier to adjust the size manually. Especially with #4068, there will be an event in a parent pane that one of the children got closed, so we could just set the remaining one the size of ourself here and be done. |
## Summary of the Pull Request Dustin and Mike had a discussion on star sizing. Mcpiroman connected the dots. I just swooped in and implemented it. ## References #3996 (comment) #3744 (comment) ## PR Checklist * [X] Closes #3744 ## Detailed Description of the Pull Request / Additional comments Star sizing allows us to keep things proportional. Since a 1::1 proportion is the same as a 100::100 proportion, I just went in and added star sizing. In the words of a some dude with a big metal fist, everything is perfectly balanced now.  ## Validation Steps Performed Verified for vertical, horizontal, and uneven splits. (cherry picked from commit 1d8c5ba)
|
🎉 Handy links: |
ghost
mentioned this pull request
This PR has evolved to encapsulate two related fixes that I can't really untie anymore. #2455 - Duplicating a tab that doesn't exist anymore This was the bug I was originally fixing in #4429. When the user tries to `duplicateTab` with a profile that doesn't exist anymore (like might happen after a settings reload), don't crash. As I was going about adding tests for this, got blocked by the fact that the Terminal couldn't open _any_ panes while the `TerminalPage` was size 0x0. This had two theoretical solutions: * Fake the `TerminalPage` into thinking it had a real size in the test - probably possible, though I'm unsure how it would work in practice. * Change `Pane`s to not require an `ActualWidth`, `ActualHeight` on initialization. Fortuately, the second option was something else that was already on my backlog of bugs. #4618 - `wt` command-line can't consistently parse more than one arg Presently, the Terminal just arbitrarily dispatches a bunch of handlers to try and handle all the commands provided on the commandline. That's lead to a bunch of reports that not all the commands will always get executed, nor will they all get executed in the same order. This PR also changes the `TerminalPage` to be able to dispatch all the commands sequentially, all at once in the startup. No longer will there be a hot second where the commands seem to execute themselves in from of the user - they'll all happen behind the scenes on startup. This involved a couple other changes areound the `TerminalPage` * I had to make sure that panes could be opened at a 0x0 size. Now they use a star sizing based off the percentage of the parent they're supposed to consume, so that when the parent _does_ get laid out, they'll take the appropriate size of that parent. * I had to do some math ahead of time to try and calculate what a `SplitState::Automatic` would be evaluated as, despite the fact that we don't actually know how big the pane will be. * I had to ensure that `focus-tab` commands appropriately mark a single tab as focused while we're in startup, without roundtripping to the Dispatcher thread and back ## References #4429 - the original PR for #2455 #5047 - a follow-up task from discussion in #4429 #4953 - a PR for making panes use star sizing, which was immensly helpful for this PR. ## Detailed Description of the Pull Request / Additional comments `CascadiaSettings::BuildSettings` can throw if the GUID doesn't exist. This wraps those calls up with a try/catch. It also adds a couple tests - a few `SettingsTests` for try/catching this state. It also adds a XAML-y test in `TabTests` that creates a `TerminalPage` and then performs som UI-like actions on it. This test required a minor change to how we generate the new tab dropdown - in the tests, `Application::Current()` is _not_ a `TerminalApp::App`, so it doesn't have a `Logic()` to query. So wrap that in a try/catch as well. While working on these tests, I found that we'd crash pretty agressively for mysterious reasons if the TestHostApp became focused while the test was running. This was due to a call in `TSFInputControl::NotifyFocusEnter` that would callback to `TSFInputControl::_layoutRequested`, which would crash on setting the `MaxSize` of the canvas to a negative value. This PR includes a hotfix for that bug as well. ## Validation Steps Performed * Manual testing with a _lot_ of commands in a commandline * run the tests * Team tested in selfhost Closes #2455 Closes #4618
Summary of the Pull Request
Dustin and Mike had a discussion on star sizing. Mcpiroman connected the dots. I just swooped in and implemented it.
References
#3996 (comment)
#3744 (comment)
PR Checklist
Detailed Description of the Pull Request / Additional comments
Star sizing allows us to keep things proportional. Since a 1::1 proportion is the same as a 100::100 proportion, I just went in and added star sizing. In the words of a some dude with a big metal fist, everything is perfectly balanced now.
Validation Steps Performed
Verified for vertical, horizontal, and uneven splits.