Skip to content
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.

Sign up for GitHub

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

Jump to bottom

If you click on the separators between panes, then no panes will be focused #999

Closed
zadjii-msft opened this issue May 24, 2019 · 6 comments · Fixed by #3540
Closed

If you click on the separators between panes, then no panes will be focused #999

zadjii-msft opened this issue May 24, 2019 · 6 comments · Fixed by #3540
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.0

Comments

@zadjii-msft
Copy link
Member

zadjii-msft commented May 24, 2019 •
edited
Loading

Self explanatory. This is related to #528, but is definitely different.

How does this interact with #994? If we make the separator a margin, then this won't be an issue.
@zadjii-msft zadjii-msft added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label May 24, 2019
@ghost 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 May 24, 2019
@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels May 24, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 24, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone May 24, 2019
This was referenced May 24, 2019
Merged
Open
@DHowett-MSFT DHowett-MSFT removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 24, 2019
@glen-84
Copy link

glen-84 commented May 25, 2019

(two small errors in the issue title: separator's -> separators, on -> no)

@zadjii-msft zadjii-msft changed the title If you click on the separator's between panes, then on panes will be focused If you click on the separators between panes, then no panes will be focused May 28, 2019
@zadjii-msft
Copy link
Member Author

@glen-84 thanks :) That's what I get for typing up 10 issues in a text file and posting them without spellchecking first

@varemenos
Copy link

May I include that clicking and dragging that same area (screenshot included), theoretically move the terminal (right?).
Capture

@DHowett-MSFT
Copy link
Contributor

@varemenos does that seem related to "split panes" at all? #564.

@varemenos
Copy link

@DHowett-MSFT yes, it's precisely that.

@zadjii-msft zadjii-msft mentioned this issue Nov 14, 2019
Merged
4 tasks
@ghost ghost added the In-PR This issue has a related PR label Nov 14, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements and removed In-PR This issue has a related PR labels Nov 18, 2019
zadjii-msft added a commit that referenced this issue Nov 18, 2019
@zadjii-msft
Decouple "Active Terminal" and "Focused Control" (#3540)
9ed3da8 
## 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:/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
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Nov 18, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

🎉This issue was addressed in #3540, which has now been successfully released as Windows Terminal Preview v0.7.3291.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Decouple "Active Terminal" and "Focused Control" microsoft/terminal
4 participants
@glen-84 @varemenos @DHowett-MSFT @zadjii-msft