Skip to content

Commit

Permalink
Teach terminal page to go to the last used tab in MRU mode (#8610)
Browse files Browse the repository at this point in the history 
## PR Checklist
* [x] Closes #8550
* [x] CLA signed. 
* [ ] Tests added/passed
* [ ] Documentation updated. 
* [ ] Schema updated.
* [x] I've discussed this with core contributors already.
  • Loading branch information
@Don-Vito
Don-Vito authored Dec 18, 2020
1 parent d37df8a commit 96b9ba9
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 38 deletions.
91 changes: 53 additions & 38 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,13 @@ namespace winrt::TerminalApp::implementation
// - tabIndex: the index of the tab to be removed
void TerminalPage::_RemoveTabViewItemByIndex(uint32_t tabIndex)
{
// We use _removing flag to suppress _OnTabSelectionChanged events
// that might get triggered while removing
_removing = true;
auto unsetRemoving = wil::scope_exit([&]() noexcept { _removing = false; });

const auto focusedTabIndex{ _GetFocusedTabIndex() };

// Removing the tab from the collection should destroy its control and disconnect its connection,
// but it doesn't always do so. The UI tree may still be holding the control and preventing its destruction.
auto tab{ _tabs.GetAt(tabIndex) };
Expand All @@ -1139,43 +1146,51 @@ namespace winrt::TerminalApp::implementation
{
_lastTabClosedHandlers(*this, nullptr);
}
else if (_isFullscreen || _rearranging || _isInFocusMode)
{
// GH#5799 - 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.
//
// GH#7916 - Similarly, TabView isn't visible in focus mode.
// We need to focus another tab manually from the same considerations as above.
//
// GH#5559 Similarly, we suppress _OnTabItemsChanged events during a
// rearrange, so if a tab is closed while we're rearranging tabs, do
// this manually.
//
// We can't use
// auto selectedIndex = _tabView.SelectedIndex();
// Because this will always return -1 in this scenario unfortunately.
//
// 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)
const auto newSelectedIndex = std::clamp<int32_t>(tabIndex - 1, 0, _tabs.Size());
// _UpdatedSelectedTab will do the work of setting up the new tab as
// the focused one, and unfocusing all the others.
_UpdatedSelectedTab(newSelectedIndex);

// 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.
auto newSelectedTab{ _tabs.GetAt(newSelectedIndex) };
_tabView.SelectedItem(newSelectedTab.TabViewItem());
else if (focusedTabIndex.has_value() && focusedTabIndex.value() == gsl::narrow_cast<uint32_t>(tabIndex))
{
// Manually select the new tab to get focus, rather than relying on TabView since:
// 1. We want to customize this behavior (e.g., use MRU logic)
// 2. In fullscreen (GH#5799) and focus (GH#7916) modes the _OnTabItemsChanged is not fired
// 3. When rearranging tabs (GH#7916) _OnTabItemsChanged is suppressed
const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode();

if (tabSwitchMode == TabSwitcherMode::MostRecentlyUsed)
{
const auto newSelectedTab = _mruTabs.GetAt(0);

uint32_t newSelectedIndex;
if (_tabs.IndexOf(newSelectedTab, newSelectedIndex))
{
_UpdatedSelectedTab(newSelectedIndex);
_tabView.SelectedItem(newSelectedTab.TabViewItem());
}
}
else
{
// We can't use
// auto selectedIndex = _tabView.SelectedIndex();
// Because this will always return -1 in this scenario unfortunately.
//
// 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)
const auto newSelectedIndex = std::clamp<int32_t>(tabIndex - 1, 0, _tabs.Size());
// _UpdatedSelectedTab will do the work of setting up the new tab as
// the focused one, and unfocusing all the others.
_UpdatedSelectedTab(newSelectedIndex);

// 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.
auto newSelectedTab{ _tabs.GetAt(newSelectedIndex) };
_tabView.SelectedItem(newSelectedTab.TabViewItem());
}
}

// GH#5559 - If we were in the middle of a drag/drop, end it by clearing
Expand Down Expand Up @@ -2215,7 +2230,7 @@ namespace winrt::TerminalApp::implementation
// - eventArgs: the event's constituent arguments
void TerminalPage::_OnTabSelectionChanged(const IInspectable& sender, const WUX::Controls::SelectionChangedEventArgs& /*eventArgs*/)
{
if (!_rearranging)
if (!_rearranging && !_removing)
{
auto tabView = sender.as<MUX::Controls::TabView>();
auto selectedIndex = tabView.SelectedIndex();
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ namespace winrt::TerminalApp::implementation
bool _rearranging;
std::optional<int> _rearrangeFrom;
std::optional<int> _rearrangeTo;
bool _removing{ false };

uint32_t _systemRowsToScroll{ DefaultRowsToScroll };

Expand Down

0 comments on commit 96b9ba9

Please sign in to comment.