Skip to content

Commit

Permalink
Only focus the active pane once initialization is complete (#10978)
Browse files Browse the repository at this point in the history
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.

* [x] Closes #6586
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

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.

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.
  • Loading branch information
zadjii-msft authored and DHowett committed Aug 25, 2021
1 parent c384ef6 commit d052617
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
13 changes: 13 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,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)
{
Expand Down Expand Up @@ -1248,6 +1253,14 @@ namespace winrt::TerminalApp::implementation
_UnZoomIfNeeded();

focusedTab->SplitPane(realSplitType, splitSize, realGuid, 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();
}
Expand Down
11 changes: 4 additions & 7 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,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
{
Expand Down Expand Up @@ -732,12 +735,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;

// Likewise, run the event handlers outside of lock (they could
Expand Down

0 comments on commit d052617

Please sign in to comment.