Skip to content

Commit

Permalink
Add a KeyChordListener to the Settings UI (#10652)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Replaces the key chord editor in the actions page with a listener instead of a plain text box.

## References
#6900 - Settings UI Epic

## Detailed Description of the Pull Request / Additional comments
- `Actions` page:
   - Replace `Keys` with `CurrentKeys` for consistency with `Action`/`CurrentAction`
   - `ProposedKeys` is now a `Control::KeyChord`
   - removes key chord validation (now we don't need it)
   - removes accept/cancel shortcuts (nowhere we could use it now)
- `KeyChordListener`:
   - `Keys`: dependency property that hooks us up to a system to the committed setting value
      - this is the key binding view model, which propagates the change to the settings model clone on "accept changes"
   - We bind to `PreviewKeyDown` to intercept the key event _before_ some special key bindings are handled (i.e. "select all" in the text box)
   - `CoreWindow` is used to get the modifier keys because (1) it's easier than updating on each key press and (2) that approach resulted in a strange bug where the <kbd>Alt</kbd> key-up event was not detected
   - `LosingFocus` means that we have completed our operation and want to commit our changes to the key binding view model
   - `KeyDown` does most of the magic of updating `Keys`. We filter out any key chords that could be problematic (i.e. <kbd>Shift</kbd>+<kbd>Tab</kbd> and <kbd>Tab</kbd> for keyboard navigation)

## Validation Steps Performed
- Tested a few key chords:
   - ✅single key: <kbd>X</kbd>
   - ✅key with modifier(s): <kbd>Ctrl</kbd>+<kbd>Alt</kbd>+<kbd>X</kbd>
   - ❌plain modifier: <kbd>Ctrl</kbd>
   - ✅key that is used by text box: <kbd>Ctrl+A</kbd>
   - ✅key that is used by Windows Terminal: <kbd>Alt</kbd>+<kbd>F4</kbd>
   - ❌key that is taken by Windows OS: <kbd>Windows</kbd>+<kbd>X</kbd>
   - ✅key that is not taken by Windows OS: <kbd>Windows</kbd>+<kbd>Shift</kbd>+<kbd>X</kbd>
- Known issue:
   - global key taken by Windows Terminal: (i.e. quake mode keybinding)
      - Behavior: global key binding executed
      - Expected: key chord recorded

## Demo
![Key Chord Listener Demo](https://user-images.githubusercontent.com/11050425/125538094-08ea4eaa-21eb-4488-a74c-6ce65324cdf1.gif)
  • Loading branch information
carlos-zamora committed Jul 16, 2021
1 parent 293c36d commit 8947909
Show file tree
Hide file tree
Showing 12 changed files with 259 additions and 86 deletions.
78 changes: 14 additions & 64 deletions src/cascadia/TerminalSettingsEditor/Actions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
KeyBindingViewModel(nullptr, availableActions.First().Current(), availableActions) {}

KeyBindingViewModel::KeyBindingViewModel(const Control::KeyChord& keys, const hstring& actionName, const IObservableVector<hstring>& availableActions) :
_Keys{ keys },
_KeyChordText{ Model::KeyChordSerialization::ToString(keys) },
_CurrentKeys{ keys },
_KeyChordText{ KeyChordSerialization::ToString(keys) },
_CurrentAction{ actionName },
_ProposedAction{ box_value(actionName) },
_AvailableActions{ availableActions }
Expand All @@ -36,9 +36,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// unique view model members.
PropertyChanged([this](auto&&, const PropertyChangedEventArgs& args) {
const auto viewModelProperty{ args.PropertyName() };
if (viewModelProperty == L"Keys")
if (viewModelProperty == L"CurrentKeys")
{
_KeyChordText = Model::KeyChordSerialization::ToString(_Keys);
_KeyChordText = KeyChordSerialization::ToString(_CurrentKeys);
_NotifyChanges(L"KeyChordText");
}
else if (viewModelProperty == L"IsContainerFocused" ||
Expand Down Expand Up @@ -75,8 +75,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// if we're in edit mode,
// - pre-populate the text box with the current keys
// - reset the combo box with the current action
ProposedKeys(KeyChordText());
ProposedAction(box_value(CurrentAction()));
ProposedKeys(_CurrentKeys);
ProposedAction(box_value(_CurrentAction));
}
}

Expand All @@ -85,35 +85,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
AttemptAcceptChanges(_ProposedKeys);
}

void KeyBindingViewModel::AttemptAcceptChanges(hstring newKeyChordText)
void KeyBindingViewModel::AttemptAcceptChanges(const Control::KeyChord newKeys)
{
try
{
// empty string --> don't accept changes
if (newKeyChordText.empty())
{
return;
}

// ModifyKeyBindingEventArgs
const auto args{ make_self<ModifyKeyBindingEventArgs>(_Keys, // OldKeys
KeyChordSerialization::FromString(newKeyChordText), // NewKeys: Attempt to convert the provided key chord text
_IsNewlyAdded ? hstring{} : _CurrentAction, // OldAction
unbox_value<hstring>(_ProposedAction)) }; //
_ModifyKeyBindingRequestedHandlers(*this, *args);
}
catch (hresult_invalid_argument)
{
// Converting the text into a key chord failed.
// Don't accept the changes.
// TODO GH #6900:
// This is tricky. I still haven't found a way to reference the
// key chord text box. It's hidden behind the data template.
// Ideally, some kind of notification would alert the user, but
// to make it look nice, we need it to somehow target the text box.
// Alternatively, we want a full key chord editor/listener.
// If we implement that, we won't need this validation or error message.
}
const auto args{ make_self<ModifyKeyBindingEventArgs>(_CurrentKeys, // OldKeys
newKeys, // NewKeys
_IsNewlyAdded ? hstring{} : _CurrentAction, // OldAction
unbox_value<hstring>(_ProposedAction)) }; // NewAction
_ModifyKeyBindingRequestedHandlers(*this, *args);
}

void KeyBindingViewModel::CancelChanges()
Expand Down Expand Up @@ -179,34 +157,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_KeyBindingList = single_threaded_observable_vector(std::move(keyBindingList));
}

void Actions::KeyChordEditor_KeyDown(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e)
{
const auto& senderTB{ sender.as<TextBox>() };
const auto& kbdVM{ senderTB.DataContext().as<Editor::KeyBindingViewModel>() };
if (e.OriginalKey() == VirtualKey::Enter)
{
// Fun fact: this is happening _before_ "_ProposedKeys" gets updated
// with the two-way data binding. So we need to directly extract the text
// and tell the view model to update itself.
get_self<KeyBindingViewModel>(kbdVM)->AttemptAcceptChanges(senderTB.Text());

// For an unknown reason, when 'AcceptChangesFlyout' is set in the code above,
// the flyout isn't shown, forcing the 'Enter' key to do nothing.
// To get around this, detect if the flyout was set, and display it
// on the text box.
if (kbdVM.AcceptChangesFlyout() != nullptr)
{
kbdVM.AcceptChangesFlyout().ShowAt(senderTB);
}
e.Handled(true);
}
else if (e.OriginalKey() == VirtualKey::Escape)
{
kbdVM.CancelChanges();
e.Handled(true);
}
}

void Actions::AddNew_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*eventArgs*/)
{
// Create the new key binding and register all of the event handlers.
Expand Down Expand Up @@ -314,7 +264,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

// update view model
auto senderVMImpl{ get_self<KeyBindingViewModel>(senderVM) };
senderVMImpl->Keys(args.NewKeys());
senderVMImpl->CurrentKeys(args.NewKeys());
}

// If the action was changed,
Expand Down Expand Up @@ -418,7 +368,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
for (uint32_t i = 0; i < _KeyBindingList.Size(); ++i)
{
const auto kbdVM{ get_self<KeyBindingViewModel>(_KeyBindingList.GetAt(i)) };
const auto& otherKeys{ kbdVM->Keys() };
const auto& otherKeys{ kbdVM->CurrentKeys() };
if (keys.Modifiers() == otherKeys.Modifiers() && keys.Vkey() == otherKeys.Vkey())
{
return i;
Expand Down
13 changes: 6 additions & 7 deletions src/cascadia/TerminalSettingsEditor/Actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void ToggleEditMode();
void DisableEditMode() { IsInEditMode(false); }
void AttemptAcceptChanges();
void AttemptAcceptChanges(hstring newKeyChordText);
void AttemptAcceptChanges(const Control::KeyChord newKeys);
void CancelChanges();
void DeleteKeyBinding() { _DeleteKeyBindingRequestedHandlers(*this, _Keys); }
void DeleteKeyBinding() { _DeleteKeyBindingRequestedHandlers(*this, _CurrentKeys); }

// ProposedAction: the entry selected by the combo box; may disagree with the settings model.
// CurrentAction: the combo box item that maps to the settings model value.
Expand All @@ -77,10 +77,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
VIEW_MODEL_OBSERVABLE_PROPERTY(hstring, CurrentAction);
WINRT_PROPERTY(Windows::Foundation::Collections::IObservableVector<hstring>, AvailableActions, nullptr);

// ProposedKeys: the text shown in the text box; may disagree with the settings model.
// Keys: the key chord bound in the settings model.
VIEW_MODEL_OBSERVABLE_PROPERTY(hstring, ProposedKeys);
VIEW_MODEL_OBSERVABLE_PROPERTY(Control::KeyChord, Keys, nullptr);
// ProposedKeys: the keys proposed by the control; may disagree with the settings model.
// CurrentKeys: the key chord bound in the settings model.
VIEW_MODEL_OBSERVABLE_PROPERTY(Control::KeyChord, ProposedKeys);
VIEW_MODEL_OBSERVABLE_PROPERTY(Control::KeyChord, CurrentKeys, nullptr);

VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsInEditMode, false);
VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsNewlyAdded, false);
Expand Down Expand Up @@ -114,7 +114,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void OnNavigatedTo(const winrt::Windows::UI::Xaml::Navigation::NavigationEventArgs& e);
Windows::UI::Xaml::Automation::Peers::AutomationPeer OnCreateAutomationPeer();
void KeyChordEditor_KeyDown(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);
void AddNew_Click(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsEditor/Actions.idl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace Microsoft.Terminal.Settings.Editor
Boolean ShowEditButton { get; };
Boolean IsInEditMode { get; };
Boolean IsNewlyAdded { get; };
String ProposedKeys;
Microsoft.Terminal.Control.KeyChord ProposedKeys;
Object ProposedAction;
Windows.UI.Xaml.Controls.Flyout AcceptChangesFlyout;
String EditButtonName { get; };
Expand Down
21 changes: 8 additions & 13 deletions src/cascadia/TerminalSettingsEditor/Actions.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,9 @@
<Setter Property="TextWrapping" Value="WrapWholeWords" />
</Style>
<Style x:Key="KeyChordEditorStyle"
BasedOn="{StaticResource DefaultTextBoxStyle}"
TargetType="TextBox">
TargetType="local:KeyChordListener">
<Setter Property="HorizontalAlignment" Value="Right" />
<Setter Property="VerticalAlignment" Value="Center" />
<Setter Property="TextAlignment" Value="Right" />
<Setter Property="TextWrapping" Value="Wrap" />
<Setter Property="IsSpellCheckEnabled" Value="False" />
</Style>
<x:Int32 x:Key="EditButtonSize">32</x:Int32>
<x:Double x:Key="EditButtonIconSize">15</x:Double>
Expand Down Expand Up @@ -202,7 +198,8 @@
Visibility="{x:Bind IsInEditMode, Mode=OneWay, Converter={StaticResource InvertedBooleanToVisibilityConverter}}" />

<!-- Edit Mode: Action Combo-box -->
<ComboBox Grid.Column="0"
<ComboBox x:Uid="Actions_ActionComboBox"
Grid.Column="0"
VerticalAlignment="Center"
ItemsSource="{x:Bind AvailableActions, Mode=OneWay}"
SelectedItem="{x:Bind ProposedAction, Mode=TwoWay}"
Expand All @@ -222,13 +219,11 @@
TextWrapping="WrapWholeWords" />
</Border>

<!-- Edit Mode: Key Chord Text Box -->
<TextBox Grid.Column="1"
DataContext="{x:Bind Mode=OneWay}"
KeyDown="KeyChordEditor_KeyDown"
Style="{StaticResource KeyChordEditorStyle}"
Text="{x:Bind ProposedKeys, Mode=TwoWay}"
Visibility="{x:Bind IsInEditMode, Mode=OneWay}" />
<!-- Edit Mode: Key Chord Listener -->
<local:KeyChordListener Grid.Column="1"
Keys="{x:Bind ProposedKeys, Mode=TwoWay}"
Style="{StaticResource KeyChordEditorStyle}"
Visibility="{x:Bind IsInEditMode, Mode=OneWay}" />

<!-- Edit Button -->
<Button x:Uid="Actions_EditButton"
Expand Down
135 changes: 135 additions & 0 deletions src/cascadia/TerminalSettingsEditor/KeyChordListener.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "pch.h"
#include "KeyChordListener.h"
#include "KeyChordListener.g.cpp"
#include "LibraryResources.h"

using namespace winrt::Windows::UI::Core;
using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Xaml::Controls;
using namespace winrt::Windows::UI::Xaml::Data;
using namespace winrt::Windows::Foundation;
using namespace winrt::Windows::System;
using namespace winrt::Windows::UI::Xaml::Input;

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
DependencyProperty KeyChordListener::_KeysProperty{ nullptr };

static constexpr std::array ModifierKeys{
VirtualKey::Shift,
VirtualKey::Control,
VirtualKey::Menu,
VirtualKey::LeftWindows,
VirtualKey::RightWindows,
VirtualKey::LeftShift,
VirtualKey::LeftControl,
VirtualKey::RightControl,
VirtualKey::LeftMenu,
VirtualKey::RightMenu
};

static VirtualKeyModifiers _GetModifiers()
{
const auto window{ CoreWindow::GetForCurrentThread() };

VirtualKeyModifiers flags = VirtualKeyModifiers::None;
for (const auto mod : ModifierKeys)
{
const auto state = window.GetKeyState(mod);
const auto isDown = WI_IsFlagSet(state, CoreVirtualKeyStates::Down);

if (isDown)
{
switch (mod)
{
case VirtualKey::Control:
case VirtualKey::LeftControl:
case VirtualKey::RightControl:
flags |= VirtualKeyModifiers::Control;
break;
case VirtualKey::Menu:
case VirtualKey::LeftMenu:
case VirtualKey::RightMenu:
flags |= VirtualKeyModifiers::Menu;
break;
case VirtualKey::Shift:
case VirtualKey::LeftShift:
flags |= VirtualKeyModifiers::Shift;
break;
case VirtualKey::LeftWindows:
case VirtualKey::RightWindows:
flags |= VirtualKeyModifiers::Windows;
break;
}
}
}
return flags;
}

KeyChordListener::KeyChordListener()
{
InitializeComponent();
_InitializeProperties();
}

void KeyChordListener::_InitializeProperties()
{
// Initialize any KeyChordListener dependency properties here.
// This performs a lazy load on these properties, instead of
// initializing them when the DLL loads.
if (!_KeysProperty)
{
_KeysProperty =
DependencyProperty::Register(
L"Keys",
xaml_typename<Control::KeyChord>(),
xaml_typename<Editor::KeyChordListener>(),
PropertyMetadata{ nullptr, PropertyChangedCallback{ &KeyChordListener::_OnKeysChanged } });
}
}

void KeyChordListener::_OnKeysChanged(DependencyObject const& d, DependencyPropertyChangedEventArgs const& e)
{
if (auto control{ d.try_as<Editor::KeyChordListener>() })
{
auto controlImpl{ get_self<KeyChordListener>(control) };
TextBox tb{ controlImpl->FindName(L"KeyChordTextBox").as<TextBox>() };
tb.Text(Model::KeyChordSerialization::ToString(unbox_value<Control::KeyChord>(e.NewValue())));
if (auto automationPeer{ Automation::Peers::FrameworkElementAutomationPeer::FromElement(tb) })
{
automationPeer.RaiseNotificationEvent(
Automation::Peers::AutomationNotificationKind::ActionCompleted,
Automation::Peers::AutomationNotificationProcessing::MostRecent,
tb.Text(),
L"KeyChordListenerText");
}
}
}

void KeyChordListener::KeyChordTextBox_KeyDown(IInspectable const& /*sender*/, KeyRoutedEventArgs const& e)
{
const auto key{ e.OriginalKey() };
for (const auto mod : ModifierKeys)
{
if (key == mod)
{
// Ignore modifier keys
return;
}
}

const auto modifiers{ _GetModifiers() };
if (key == VirtualKey::Tab && (modifiers == VirtualKeyModifiers::None || modifiers == VirtualKeyModifiers::Shift))
{
// [Shift]+[Tab] && [Tab] are needed for keyboard navigation
return;
}

// Permitted key events are used to update _Keys
Keys({ modifiers, static_cast<int32_t>(key) });
e.Handled(true);
}
}
29 changes: 29 additions & 0 deletions src/cascadia/TerminalSettingsEditor/KeyChordListener.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#pragma once

#include "KeyChordListener.g.h"
#include "Utils.h"

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
struct KeyChordListener : KeyChordListenerT<KeyChordListener>
{
public:
KeyChordListener();

void KeyChordTextBox_KeyDown(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);

DEPENDENCY_PROPERTY(Control::KeyChord, Keys);

private:
static void _InitializeProperties();
static void _OnKeysChanged(Windows::UI::Xaml::DependencyObject const& d, Windows::UI::Xaml::DependencyPropertyChangedEventArgs const& e);
};
}

namespace winrt::Microsoft::Terminal::Settings::Editor::factory_implementation
{
BASIC_FACTORY(KeyChordListener);
}
13 changes: 13 additions & 0 deletions src/cascadia/TerminalSettingsEditor/KeyChordListener.idl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

namespace Microsoft.Terminal.Settings.Editor
{
[default_interface] runtimeclass KeyChordListener : Windows.UI.Xaml.Controls.UserControl
{
KeyChordListener();

Microsoft.Terminal.Control.KeyChord Keys;
static Windows.UI.Xaml.DependencyProperty KeysProperty { get; };
}
}
Loading

0 comments on commit 8947909

Please sign in to comment.