From 23a19c58181b489b10a2049232f23b033dbdab8d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 24 Aug 2021 04:49:45 -0500 Subject: [PATCH] Only focus the active pane once initialization is complete (#10978) ## Summary of the Pull Request Since the days immemorial of the Terminal, the TermControl has auto-focused itself when it finalizes its layout. This has led to the problem that `wt ; sp ; sp ; sp...` ends up focusing one of these panes at random. This PR fixes this issue by getting rid of the auto-focusing. Panes now manually get focused when created. We manually focus the active pane when a commandline is dispatched. since we're internally tracking "active" separate from "focused", this ends up working as you'd hope. ## References ## PR Checklist * [x] Closes #6586 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments I also had to turn the cursor off by default. Most `TermControl`s would never get the `LostFocus` event, so their cursors would get left `On`, and that's not right. ## Validation Steps Performed I've run the following things a bunch of times to make sure they work: * `wtd sp ; sp ; sp` * `wtd sp ; sp ; sp ; fp -t 0` * `newTab` * `splitPane` * use the command palette to do the above as well Where the result used to be random (cases 1 & 2), the result is exactly what you'd expect now. It doesn't work at all for ``` wtd sp ; sp ; sp ; mf left ``` Presumably because we can't `move-focus` directionally during startup. However, that doesn't work _today_ either, so it's not making it worse. Just highlights that single scenario doesn't work right. --- src/cascadia/TerminalApp/TerminalPage.cpp | 13 +++++++++++++ src/cascadia/TerminalControl/TermControl.cpp | 11 ++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 12cec3594..cba5ea4a9 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -457,6 +457,11 @@ namespace winrt::TerminalApp::implementation co_return; } } + + // GH#6586: now that we're done processing all startup commands, + // focus the active control. This will work as expected for both + // commandline invocations and for `wt` action invocations. + _GetActiveControl().Focus(FocusState::Programmatic); } if (initial) { @@ -1414,6 +1419,14 @@ namespace winrt::TerminalApp::implementation _UnZoomIfNeeded(); tab.SplitPane(realSplitType, splitSize, profile, newControl); + + // After GH#6586, the control will no longer focus itself + // automatically when it's finished being laid out. Manually focus + // the control here instead. + if (_startupState == StartupState::Initialized) + { + _GetActiveControl().Focus(FocusState::Programmatic); + } } CATCH_LOG(); } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index aaa562c34..e36fadd6a 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -681,8 +681,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation DispatcherTimer cursorTimer; cursorTimer.Interval(std::chrono::milliseconds(blinkTime)); cursorTimer.Tick({ get_weak(), &TermControl::_CursorTimerTick }); - cursorTimer.Start(); _cursorTimer.emplace(std::move(cursorTimer)); + // As of GH#6586, don't start the cursor timer immediately, and + // don't show the cursor initially. We'll show the cursor and start + // the timer when the control is first focused. cursorTimer.Start(); + _core.CursorOn(false); } else { @@ -711,12 +714,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Now that the renderer is set up, update the appearance for initialization _UpdateAppearanceFromUIThread(_settings); - // Focus the control here. If we do it during control initialization, then - // focus won't actually get passed to us. I believe this is because - // we're not technically a part of the UI tree yet, so focusing us - // becomes a no-op. - this->Focus(FocusState::Programmatic); - _initializedTerminal = true; // MSFT 33353327: If the AutomationPeer was created before we were done initializing,