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

ESC from the Tab ContextMenu should return focus to the Terminal #5750

Closed
zadjii-msft opened this issue May 5, 2020 · 5 comments · Fixed by #10048
Closed

ESC from the Tab ContextMenu should return focus to the Terminal #5750

zadjii-msft opened this issue May 5, 2020 · 5 comments · Fixed by #10048
Labels
Area-UserInterface 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. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v2.0

Comments

@zadjii-msft
Copy link
Member

zadjii-msft commented May 5, 2020 •
edited
Loading

Repro steps

  1. Right click a tab
  2. Hit esc

Expected

  1. Terminal gets focus again

Actual

  1. Focus lands on the tab itself
    image

regressed in #3789

Also, fixing this might just magically fix #3609.
@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels May 5, 2020
@zadjii-msft zadjii-msft added this to the Terminal v1.1 milestone May 5, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 5, 2020
@zadjii-msft zadjii-msft mentioned this issue May 6, 2020
Merged
8 tasks
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 7, 2020
@zadjii-msft
Copy link
Member Author

Note from an offline discussion: maybe we could prevent the right click on the tab from focusing the TabViewItem, and then focus would just return to the Terminal? 🤞

ghost pushed a commit that referenced this issue May 28, 2020
@zadjii-msft
Enable tab renaming at runtime from the UI (#5775)
f17b6c7 
## Summary of the Pull Request

Adds support for setting, from the UI, a runtime override for the tab title text. The user can use this to effectively "rename" a tab.

If set, the tab will _always_ use the runtime override string. If the user has multiple panes with different titles in a pane, then the tab's override text will be used _regardless_ of which pane was focused when the tab was renamed.

The override text can be removed by just deleting the entire contents of the box. Then, the tab will revert to using the terminal's usual title. 

## References
* Wouldn't be possible without the context menu from #3789
* Focus doesn't return to the active terminal after hitting <kbd>enter</kbd>/<kbd>esc</kbd>, but that's tracked by #5750 

## PR Checklist
* [x] Closes #1079
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated

## TODO
* [x] `Tab::SetTabText` might be able to be greatly simplified/removed?
* [x] I'm _pretty sure_ if they set an override title, we won't bubble that up to set the window title.
* [x] I'm unsure how this behaves when the terminal's title changes _while_ the TextBox is visible. I don't think it should change the current contents of the box, but it might currently.
* [ ] **for discussion**: If the user doesn't actually change the text of the tab, then we probably shouldn't set the override text, right? 
  - EX: if they open the box and the text is "cmd", and immediately hit <kbd>enter</kbd>, then run `title foo`, should the text change to "foo" or stay "cmd"?

## Detailed Description of the Pull Request / Additional comments
![image](https://user-images.githubusercontent.com/18356694/81230615-713f9180-8fb7-11ea-8945-6681eec02a4f.png)
![image](https://user-images.githubusercontent.com/18356694/81230640-7ac8f980-8fb7-11ea-9e6b-22f0e0ed128a.png)
![image](https://user-images.githubusercontent.com/18356694/81230665-86b4bb80-8fb7-11ea-90f0-16d4ffb60d89.png)
![image](https://user-images.githubusercontent.com/18356694/81230686-9207e700-8fb7-11ea-94a9-f3f5a59be139.png)
![image](https://user-images.githubusercontent.com/18356694/81230732-a350f380-8fb7-11ea-9901-6dd4f36154f1.png)
![image](https://user-images.githubusercontent.com/18356694/81230746-a8ae3e00-8fb7-11ea-94fa-d2578f9241a7.png)
![image](https://user-images.githubusercontent.com/18356694/81230787-bc59a480-8fb7-11ea-8edf-2bd7fad343fc.png)
![image](https://user-images.githubusercontent.com/18356694/81230851-dc896380-8fb7-11ea-98c1-918b943543e4.png)
@zadjii-msft zadjii-msft mentioned this issue Jun 1, 2020
Closed
4 tasks
@ghost ghost added the In-PR This issue has a related PR label Jun 1, 2020
@zadjii-msft zadjii-msft self-assigned this Jun 2, 2020
@DHowett DHowett mentioned this issue Jun 16, 2020
Closed
@zadjii-msft zadjii-msft mentioned this issue Jun 25, 2020
Closed
6 tasks
@zadjii-msft

This comment has been minimized.

Sign in to view
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 6, 2021
@zadjii-msft zadjii-msft added Resolution-Fix-Available It's available in an Insiders build or a release and removed In-PR This issue has a related PR labels May 6, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 6, 2021
@zadjii-msft zadjii-msft removed their assignment May 6, 2021
@zadjii-msft
Copy link
Member Author

Wait no what? I'm crazy, had the wrong thread open this whole time. Ignore me.

@zadjii-msft zadjii-msft reopened this May 6, 2021
@zadjii-msft zadjii-msft removed the Resolution-Fix-Available It's available in an Insiders build or a release label May 6, 2021
zadjii-msft added a commit that referenced this issue May 6, 2021
@zadjii-msft
This fixes #5750 specifically, but a little dirty
9ed5e8c
@zadjii-msft zadjii-msft mentioned this issue May 6, 2021
Merged
14 tasks
@ghost ghost added the In-PR This issue has a related PR label May 6, 2021
@ghost ghost closed this as completed in #10048 May 11, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels May 11, 2021
ghost pushed a commit that referenced this issue May 11, 2021
@zadjii-msft
In specific scenarios, focus the active control (#10048)
8564b26 
A redo of #6290. That PR was overkill. In that one, we'd toss focus back to the active control any time that the tab view item got focus. That's maybe not the _best_ solution.

Instead, this PR is precision strikes. We're re-using a lot of what we already have from #9260. 
* When the context menu is closed, yeet focus to the control.
* When the renamer is dismissed, yeet focus to the control.
* When the TabViewItem is tapped (meaning no one else handled it), yeet focus to the control.

### checklist 
* [x] I work here
* [ ] This is UI so it doesn't have tests
* [x] Closes #3609
* [x] Closes #5750
* [x] Closes #6680

### scenarios:

* [x] focus the window by clicking on the tab -> Control is focused. 
* [x] Open the color picker with the context menu, can move the focus inside the picker with the arrow keys.
* [x] Dismiss the picker with esc -> Control is focused. 
* [x] Dismiss the picker with enter -> Control is focused. 
* [x] Dismiss the renamer with esc -> Control is focused. 
* [x] Dismiss the renamer with enter -> Control is focused. 
* [x] Dismiss the context menu with esc -> Control is focused. 
* [x] Start renaming, then click on the tab -> Rename is committed, Control is focused. 
* [x] Start renaming, then click on the text box -> focus is still in the text box
DHowett pushed a commit that referenced this issue May 14, 2021
@zadjii-msft @DHowett
In specific scenarios, focus the active control (#10048)
b345d39 
A redo of #6290. That PR was overkill. In that one, we'd toss focus back to the active control any time that the tab view item got focus. That's maybe not the _best_ solution.

Instead, this PR is precision strikes. We're re-using a lot of what we already have from #9260.
* When the context menu is closed, yeet focus to the control.
* When the renamer is dismissed, yeet focus to the control.
* When the TabViewItem is tapped (meaning no one else handled it), yeet focus to the control.

* [x] I work here
* [ ] This is UI so it doesn't have tests
* [x] Closes #3609
* [x] Closes #5750
* [x] Closes #6680

* [x] focus the window by clicking on the tab -> Control is focused.
* [x] Open the color picker with the context menu, can move the focus inside the picker with the arrow keys.
* [x] Dismiss the picker with esc -> Control is focused.
* [x] Dismiss the picker with enter -> Control is focused.
* [x] Dismiss the renamer with esc -> Control is focused.
* [x] Dismiss the renamer with enter -> Control is focused.
* [x] Dismiss the context menu with esc -> Control is focused.
* [x] Start renaming, then click on the tab -> Rename is committed, Control is focused.
* [x] Start renaming, then click on the text box -> focus is still in the text box

(cherry picked from commit 8564b26)
DHowett pushed a commit that referenced this issue May 14, 2021
@zadjii-msft @DHowett
In specific scenarios, focus the active control (#10048)
042f9ef 
A redo of #6290. That PR was overkill. In that one, we'd toss focus back to the active control any time that the tab view item got focus. That's maybe not the _best_ solution.

Instead, this PR is precision strikes. We're re-using a lot of what we already have from #9260.
* When the context menu is closed, yeet focus to the control.
* When the renamer is dismissed, yeet focus to the control.
* When the TabViewItem is tapped (meaning no one else handled it), yeet focus to the control.

* [x] I work here
* [ ] This is UI so it doesn't have tests
* [x] Closes #3609
* [x] Closes #5750
* [x] Closes #6680

* [x] focus the window by clicking on the tab -> Control is focused.
* [x] Open the color picker with the context menu, can move the focus inside the picker with the arrow keys.
* [x] Dismiss the picker with esc -> Control is focused.
* [x] Dismiss the picker with enter -> Control is focused.
* [x] Dismiss the renamer with esc -> Control is focused.
* [x] Dismiss the renamer with enter -> Control is focused.
* [x] Dismiss the context menu with esc -> Control is focused.
* [x] Start renaming, then click on the tab -> Rename is committed, Control is focused.
* [x] Start renaming, then click on the text box -> focus is still in the text box

(cherry picked from commit 8564b26)
DHowett pushed a commit that referenced this issue May 14, 2021
@zadjii-msft @DHowett
In specific scenarios, focus the active control (#10048)
aead759 
A redo of #6290. That PR was overkill. In that one, we'd toss focus back to the active control any time that the tab view item got focus. That's maybe not the _best_ solution.

Instead, this PR is precision strikes. We're re-using a lot of what we already have from #9260.
* When the context menu is closed, yeet focus to the control.
* When the renamer is dismissed, yeet focus to the control.
* When the TabViewItem is tapped (meaning no one else handled it), yeet focus to the control.

* [x] I work here
* [ ] This is UI so it doesn't have tests
* [x] Closes #3609
* [x] Closes #5750
* [x] Closes #6680

* [x] focus the window by clicking on the tab -> Control is focused.
* [x] Open the color picker with the context menu, can move the focus inside the picker with the arrow keys.
* [x] Dismiss the picker with esc -> Control is focused.
* [x] Dismiss the picker with enter -> Control is focused.
* [x] Dismiss the renamer with esc -> Control is focused.
* [x] Dismiss the renamer with enter -> Control is focused.
* [x] Dismiss the context menu with esc -> Control is focused.
* [x] Start renaming, then click on the tab -> Rename is committed, Control is focused.
* [x] Start renaming, then click on the text box -> focus is still in the text box

(cherry picked from commit 8564b26)
@ghost
Copy link

ghost commented May 25, 2021

🎉This issue was addressed in #10048, which has now been successfully released as Windows Terminal v1.8.1444.0.:tada:

Handy links:

@ghost
Copy link

ghost commented May 25, 2021

🎉This issue was addressed in #10048, which has now been successfully released as Windows Terminal Preview v1.9.1445.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-UserInterface 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. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v2.0
3 participants
@DHowett-MSFT @zadjii-msft @cinnamon-msft