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.

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

Passing through moveFocus keys when moving to another pane failed #10806

Merged
4 commits merged into from
Jul 28, 2021

Conversation

FWest98
Copy link
Contributor

@FWest98 FWest98 commented Jul 27, 2021

Summary of the Pull Request

Implementation of #6219 with a small tweak, not just passing the keys when no panes are present, but passing on the keys when there is no other pane to move to. This enables another usecase: 2 panes in terminal split vertically; in one of these panes running tmux with two panes that are split horizontally. This allows the user to still navigate between tmux panes even though they have terminal panes open.

References

Not that I know of

PR Checklist

  • Closes Pass through moveFocus keys when the user has no panes #6219
  • CLA signed.
  • Tests added/passed
  • Documentation updated. I don't think that's necessary
  • Schema updated. N/A
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

Detailed Description of the Pull Request / Additional comments

Implementation by propagating the boolean indicating success of moving focus all the way to the action handler, where this result will determine whether the action will be considered handled or not. When the action is not handled, the keychord will be propagated to the terminal.

Validation Steps Performed

Manual testing; all relevant unit tests still work

@ghost ghost added 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. labels Jul 27, 2021
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks!

src/cascadia/TerminalApp/AppActionHandlers.cpp Outdated Show resolved Hide resolved
Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com>
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 28, 2021
@ghost
Copy link

ghost commented Jul 28, 2021

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 10222a2 into microsoft:main Jul 28, 2021
DHowett pushed a commit that referenced this pull request Aug 25, 2021
…0806)

<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
Implementation of #6219 with a small tweak, not just passing the keys when no panes are present, but passing on the keys when there is no other pane to move to. This enables another usecase: 2 panes in terminal split vertically; in one of these panes running tmux with two panes that are split horizontally. This allows the user to still navigate between tmux panes even though they have terminal panes open.

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
Not that I know of

<!-- Please review the items on the PR checklist before submitting-->
* [x] Closes #6219
* [x] CLA signed.
* [x] Tests added/passed
* [x] Documentation updated. I don't think that's necessary
* [x] Schema updated. N/A
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
Implementation by propagating the boolean indicating success of moving focus all the way to the action handler, where this result will determine whether the action will be considered handled or not. When the action is not handled, the keychord will be propagated to the terminal.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
Manual testing; all relevant unit tests still work
@ghost
Copy link

ghost commented Aug 31, 2021

🎉Windows Terminal Preview v1.10.2383.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Aug 31, 2021

🎉Windows Terminal Preview v1.11.2421.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
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 AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass through moveFocus keys when the user has no panes
3 participants