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

Teach terminal page to go to the last used tab in MRU mode #8610

Merged
2 commits merged into from
Dec 18, 2020
Merged

Teach terminal page to go to the last used tab in MRU mode #8610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6ee0cc
Initial draft
Don-Vito Dec 18, 2020
684b007
Suppress _OnTabSelectionChanged during removal
Don-Vito Dec 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 53 additions & 38 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,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 @@ -1137,43 +1144,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 @@ -2212,7 +2227,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
Open in desktop
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