maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Only focus the active pane once initialization is complete (#10978)

Browse source 
## 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.
This commit is contained in:
Mike Griese 2021-08-24 04:49:45 -05:00 committed by GitHub
parent 7712104983
commit 23a19c5818
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -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();
}

11
src/cascadia/TerminalControl/TermControl.cpp
View file

@ -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,