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

Arrow navigation dismisses tab renamer #9632

Closed
Don-Vito opened this issue Mar 26, 2021 · 7 comments
Closed

Arrow navigation dismisses tab renamer #9632

Don-Vito opened this issue Mar 26, 2021 · 7 comments
Labels
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 Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@Don-Vito
Copy link
Contributor

Windows Terminal version (or Windows build number)

Dev - latest

Other Software

No response

Steps to reproduce

With 1 open tab, open tab renamer
Go to the end of the title
Click Right.
The renamer is dissmissed

With 2 tabs, open tab renamer in the second tab
Go to beginning of the title
Press Left
The renamer is dismissed

Same applies to Up and Down

Expected Behavior

The renamer shouldn't be dismissed if by mistake the user clicks extra arrow.

Actual Behavior

The renamer is dismissed.

Reported initially by @KalleOlaviNiemitalo in #9520.

The root-cause is that if witin renamer we hit Left but the cursor cannot move as it is already in the beginning, the KeyDown event propagates to the parent control. This way it bubbles up, until it reaches the TabView that handles this key as a navigation between tabs.
@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 Mar 26, 2021
@Don-Vito
Copy link
Contributor Author

I guess I know how to fix this.

@Don-Vito Don-Vito mentioned this issue Mar 26, 2021
Merged
6 tasks
@ghost ghost added the In-PR This issue has a related PR label Mar 26, 2021
@KalleOlaviNiemitalo
Copy link

I don't think the keypress navigated between tabs, though. The tab that I was renaming remained selected.

@Don-Vito
Copy link
Contributor Author

I don't think the keypress navigated between tabs, though. The tab that I was renaming remained selected.

This is probably because you had one tab. In this case, focus moves to the + button, and then auto resets to the terminal.

In any case the problem is that text box doesn't handle an arrow if the cursor cannot move, and leaves it to the parent. It is a feature. But in our case it is dangerous as it commits the tab name.

@KalleOlaviNiemitalo
Copy link

I tried it again on Windows Terminal Preview 1.7.572.0. I have three tabs open. I double-click the title of the second tab to start renaming it, then press the Left arrow key twice. Approximately half of the time, a black focus rectangle briefly flashes around the tab header of the first tab. Very soon after that, the session in the second tab gets the focus again.

@Don-Vito
Copy link
Contributor Author

Don-Vito commented Mar 29, 2021 •
edited

@KalleOlaviNiemitalo - what you describe sounds like some race between TabRenamerDeactivated and _OnTabSelectionChanged handler in the TerminalPage. I think it probably should be a separate issue. Though preventing the arrows from switching focus renders it not reproducible 😄

@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 Mar 29, 2021
@ghost ghost closed this as completed in 704836e Mar 29, 2021
DHowett pushed a commit that referenced this issue Apr 13, 2021
@Don-Vito @DHowett
Prevent arrow keys from dismissing tab renamer (#9633)
fadd185 
## PR Checklist
* [x] Closes #9632
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.

(cherry picked from commit 704836e)
@ghost
Copy link

ghost commented Apr 14, 2021

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

Handy links:

@ghost
Copy link

ghost commented Apr 14, 2021

🎉This issue was addressed in #9633, which has now been successfully released as Windows Terminal Preview v1.8.1032.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
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 Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Don-Vito @KalleOlaviNiemitalo