maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

8247: Custom key bindings are broken for tab navigation (#8250)

Browse source 
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.

## Validation Steps Performed
* Manual testing of both custom and non-custom bindings with different
  amount of tabs.

Closes #8247
This commit is contained in:
Don-Vito 2020-11-13 03:33:50 +02:00 committed by GitHub
parent 1f54efff2f
commit 0437fe9d8e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 12 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -1208,6 +1208,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;
}
const auto tabSwitchMode = _settings.GlobalSettings().TabSwitcherMode();
const bool useInOrderTabIndex = tabSwitchMode != TabSwitcherMode::MostRecentlyUsed;
@ -1252,19 +1260,10 @@ namespace winrt::TerminalApp::implementation
CommandPalette().SetTabActions(_mruTabActions, true);
}
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);
}
else
{
// 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);
}
// 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() })
{