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

Manually select a new tab when we're in fullscreen mode #5809

Merged

Conversation

zadjii-msft
Copy link
Member

Summary of the Pull Request

If we're fullscreen, the TabView isn't Visible. If it's not Visible, it's not going to raise a SelectionChanged event, which is what we usually use to focus another tab. Instead, we'll have to do it manually here.

So, what we're going to try to do is move the focus to the tab to the left, within the bounds of how many tabs we have.

EX: we have 4 tabs: [A, B, C, D]. If we close:

  • A (tabIndex=0): We'll want to focus tab B (now in index 0)
  • B (tabIndex=1): We'll want to focus tab A (now in index 0)
  • C (tabIndex=2): We'll want to focus tab B (now in index 1)
  • D (tabIndex=3): We'll want to focus tab C (now in index 2)

_UpdatedSelectedTab will do the work of setting up the new tab as the focused one, and unfocusing all the others.

Also, we need to manually set the SelectedItem of the tabView here. If we don't, then the TabView will technically not have a selected item at all, which can make things like ClosePane not work correctly.

References

PR Checklist

Validation Steps Performed

Played with it a bunch

@ghost ghost added 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. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-DataLoss A bug that causes the user's data to be lost, unintentionally labels May 8, 2020
@zadjii-msft zadjii-msft requested a review from DHowett-MSFT May 8, 2020 13:59
@DHowett-MSFT
Copy link
Contributor

(do not automerge)

@DHowett-MSFT DHowett-MSFT merged commit d77745d into master May 8, 2020
@DHowett-MSFT DHowett-MSFT deleted the dev/migrie/b/5799-accidentally-the-whole-thing branch May 8, 2020 18:53
DHowett-MSFT pushed a commit that referenced this pull request May 8, 2020
If we're fullscreen, the TabView isn't `Visible`. If it's not `Visible`,
it's _not_ going to raise a `SelectionChanged` event, which is what we
usually use to focus another tab. Instead, we'll have to do it manually
here.

So, what we're going to try to do is move the focus to the tab to the
left, within the bounds of how many tabs we have.

EX: we have 4 tabs: [A, B, C, D]. If we close:
* A (`tabIndex=0`): We'll want to focus tab B (now in index 0)
* B (`tabIndex=1`): We'll want to focus tab A (now in index 0)
* C (`tabIndex=2`): We'll want to focus tab B (now in index 1)
* D (`tabIndex=3`): We'll want to focus tab C (now in index 2)

`_UpdatedSelectedTab` will do the work of setting up the new tab as the
focused one, and unfocusing all the others.

Also, we need to _manually_ set the SelectedItem of the tabView here. If
we don't, then the TabView will technically not have a selected item at
all, which can make things like ClosePane not work correctly.

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

## Validation Steps Performed

Played with it a bunch

(cherry picked from commit d77745d)
@ghost
Copy link

ghost commented May 13, 2020

🎉Windows Terminal Release Candidate v0.11.1333.0 (1.0rc2) has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request May 13, 2020
jelster pushed a commit to jelster/terminal that referenced this pull request May 28, 2020
If we're fullscreen, the TabView isn't `Visible`. If it's not `Visible`,
it's _not_ going to raise a `SelectionChanged` event, which is what we
usually use to focus another tab. Instead, we'll have to do it manually
here.

So, what we're going to try to do is move the focus to the tab to the
left, within the bounds of how many tabs we have.

EX: we have 4 tabs: [A, B, C, D]. If we close:
* A (`tabIndex=0`): We'll want to focus tab B (now in index 0)
* B (`tabIndex=1`): We'll want to focus tab A (now in index 0)
* C (`tabIndex=2`): We'll want to focus tab B (now in index 1)
* D (`tabIndex=3`): We'll want to focus tab C (now in index 2)

`_UpdatedSelectedTab` will do the work of setting up the new tab as the
focused one, and unfocusing all the others.

Also, we need to _manually_ set the SelectedItem of the tabView here. If
we don't, then the TabView will technically not have a selected item at
all, which can make things like ClosePane not work correctly.

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

## Validation Steps Performed

Played with it a bunch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-DataLoss A bug that causes the user's data to be lost, unintentionally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal freezes after tab close
3 participants