-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Untie "active control" from "currently XAML-focused control" #1205
Labels
Area-User Interface
Issues pertaining to the user interface of the Console or Terminal
Issue-Task
It's a feature request, but it doesn't really need a major design.
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
ghost
added
Needs-Triage
It's a new issue that the core contributor team needs to triage at the next triage meeting
Needs-Tag-Fix
Doesn't match tag requirements
labels
Jun 11, 2019
DHowett-MSFT
added
Area-User Interface
Issues pertaining to the user interface of the Console or Terminal
Help Wanted
We encourage anyone to jump in on these.
Issue-Task
It's a feature request, but it doesn't really need a major design.
Product-Terminal
The new Windows Terminal.
and removed
Needs-Tag-Fix
Doesn't match tag requirements
Needs-Triage
It's a new issue that the core contributor team needs to triage at the next triage meeting
labels
Jun 11, 2019
3 tasks
DHowett-MSFT
changed the title
Untie "active control" from "currntly XAML-focused control"
Untie "active control" from "currently XAML-focused control"
Jun 11, 2019
The bug reported in 1862 is super annoying, so this should definitely go in for 1.0. |
5 tasks
2 tasks
zadjii-msft
added a commit
that referenced
this issue
Nov 11, 2019
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
4 tasks
ghost
added
Needs-Tag-Fix
Doesn't match tag requirements
and removed
Help Wanted
We encourage anyone to jump in on these.
In-PR
This issue has a related PR
labels
Nov 18, 2019
ghost
added
the
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
label
Nov 18, 2019
zadjii-msft
added a commit
that referenced
this issue
Nov 18, 2019
## 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
🎉This issue was addressed in #3540, which has now been successfully released as Handy links: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Area-User Interface
Issues pertaining to the user interface of the Console or Terminal
Issue-Task
It's a feature request, but it doesn't really need a major design.
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
A tab should have a notion of its "active control", even if that control isn't currently focused. Right now, we have some problematic mixing of user intent and UI state that cause us to have tabs with no active control and tabs whose titles flicker when they become "focused" even though what's "active" hasn't changed.
Changes in which control is active can be propagated out through a single "ActiveTerminalChanged" event.
The text was updated successfully, but these errors were encountered: