Skip to content

Commit

Permalink
8247: Custom key bindings are broken for tab navigation (#8250)
Browse files Browse the repository at this point in the history
There are two code paths for Ctrl+Tab and for everything else:

Ctrl + Tab is working perfectly
* On the first tab navigation TerminalPage::_SelectNextTab resets the
  tab commands, and since palette is not visible selects the relevant
  tab index
* On the second navigation Ctrl+Tab is intercepted by
  CommandPalette::_previewKeyDownHandler and everything works fine

But with custom binding things are screwed:
* On the first tab navigation TerminalPage::_SelectNextTab resets the
  tab commands, and since palette is not visible selects the relevant
  tab index
* On the second navigation keys are not intercepted and thus
  TerminalPage::_SelectNextTab is called again. It resets the commands,
  but now since the palette is visible we simply invoke
  CommandPalette::SelectNextItem. Which in turn misbehaves because no
  item is selected.

The approach for the solution is not to reset anything if the command
palette is already open.

* Manual testing of both custom and non-custom bindings with different
  amount of tabs.

Closes #8247

(cherry picked from commit 0437fe9)
  • Loading branch information
Don-Vito authored and DHowett committed Nov 20, 2020
1 parent c2bc927 commit a512ddd
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,14 @@ namespace winrt::TerminalApp::implementation
// - Sets focus to the tab to the right or left the currently selected tab.
void TerminalPage::_SelectNextTab(const bool bMoveRight)
{
if (CommandPalette().Visibility() == Visibility::Visible)
{
// If the tab switcher is currently open, don't change its mode.
// Just select the new tab.
CommandPalette().SelectNextItem(bMoveRight);
return;
}

if (_settings.GlobalSettings().UseTabSwitcher())
{
CommandPalette().SetTabActions(_mruTabActions);
Expand All @@ -1227,15 +1235,10 @@ namespace winrt::TerminalApp::implementation
uint32_t tabCount = _mruTabActions.Size();
auto newTabIndex = ((tabCount + (bMoveRight ? 1 : -1)) % tabCount);

if (CommandPalette().Visibility() == Visibility::Visible)
{
CommandPalette().SelectNextItem(bMoveRight);
}
else
{
CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
CommandPalette().Visibility(Visibility::Visible);
}
// Otherwise, set up the tab switcher in the selected mode, with
// the given ordering, and make it visible.
CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
CommandPalette().Visibility(Visibility::Visible);
}
else if (auto index{ _GetFocusedTabIndex() })
{
Expand Down

0 comments on commit a512ddd

Please sign in to comment.