Skip to content

Commit

Permalink
Decouple "Active Terminal" and "Focused Control" (#3540)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

Unties the concept of "focused control" from "active control".

Previously, we were exclusively using the "Focused" state of `TermControl`s to determine which one was active. This was fraught with gotchas - if anything else became focused, then suddenly there was _no_ pane focused in the Tab. This happened especially frequently if the user clicked on a tab to focus the window. Furthermore, in experimental branches with more UI added to the Terminal (such as [dev/migrie/f/2046-command-palette](https://github.com/microsoft/terminal/tree/dev/migrie/f/2046-command-palette)), when these UIs were added to the Terminal, they'd take focus, which again meant that there was no focused pane.

This fixes these issue by having each Tab manually track which Pane is active in that tab. The Tab is now the arbiter of who in the tree is "active". Panes still track this state, for them to be able to MoveFocus appropriately. 

It also contains a related fix to prevent the tab separator from stealing focus from the TermControl. This required us to set the color of the un-focused Pane border to some color other that Transparent, so I went with the TabViewBackground. Panes now look like the following:

![image](https://user-images.githubusercontent.com/18356694/68697343-41ea2380-0544-11ea-8218-601b57fdd835.png)


## References

See also: #2046

## PR Checklist
* [x] Closes #1205
* [x] Closes #522
* [x] Closes #999
* [x] I work here
* [😢] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed

Tested manually opening panes, closing panes, clicking around panes, the whole dance.

---------------------------------------------------

* this is janky but is close for some reason?

* This is _almost_ right to solve #1205

  If I want to double up and also fix #522 (which I do), then I need to also
  * when a tab GetsFocus, send the focus instead to the Pane
  * When the border is clicked on, focus that pane's control

  And like a lot of cleanup, because this is horrifying

* hey this autorevoker is really nice

* Encapsulate Pane::pfnGotFocus

* Propogate the events back up on close

* Encapsulate Tab::pfnFocusChanged, and clean up TerminalPage a bit

* Mostly just code cleanup, commenting

* This works to hittest on the borders

  If the border is `Transparent`, then it can't hittest for Tapped events, and it'll fall through (to someone)

  THis at least works, but looks garish

* Match the pane border to the TabViewHeader

* Fix a bit of dead code and a bad copy-pasta

* This _works_ to use a winrt event, but it's dirty

* Clean up everything from the winrt::event debacle.

* This is dead code that shouldn't have been there

* Turn Tab's callback into a winrt::event as well
  • Loading branch information
zadjii-msft authored Nov 18, 2019
1 parent cc8faaf commit 9ed3da8
Show file tree
Hide file tree
Showing 8 changed files with 313 additions and 183 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::AdjustFontSizeArgs>())
{
const auto termControl = _GetFocusedControl();
const auto termControl = _GetActiveControl();
termControl.AdjustFontSize(realArgs.Delta());
args.Handled(true);
}
Expand Down
Loading

0 comments on commit 9ed3da8

Please sign in to comment.