Allow setting the action on Actions page (#10220)

This introduces the ability to set the action for a key binding. A combo box is used to let the user select a new action.

## References
#6900 - Actions page Epic
#9427 - Actions page design doc
#9949 - Actions page PR

## Detailed Description of the Pull Request / Additional comments
### Settings Model Changes
- `ActionAndArgs`
   - new ctor that just takes a `ShortcutAction`
- `ActionMap`
   - `AvailableActions` provides a map of all the "acceptable" actions to choose from. This is a merged list of (1) all `{ "command": X }` style actions and (2) any actions with args that are already defined in the ActionMap (or any parents).
   - `RegisterKeyBinding` introduces a new unnamed key binding to the action map.

### Editor Changes
- XAML
   - Pretty straightforward, when in edit mode, we replace the text block with a combo box. This combo box just presents the actions you can choose from.
- `RebindKeysEventArgs` --> `ModifyKeyBindingEventArgs`
- `AvailableActionAndArgs`
   - stores the list of actions to choose from in the combo box
   - _Unfortunately_, `KeyBindingViewModel` needs this so that we can populate the combo box
   - `Actions` stores and maintains this though. We populate this from the settings model on navigation.
- `ProposedAction` vs `CurrentAction`
   - similar to `ProposedKeys` and `Keys`, we need a way to distinguish the value from the settings model and the value of the control (i.e. combo box).
   - `CurrentAction` --> settings model
   - `ProposedAction` --> combo box selected item

## Validation Steps Performed
- Cancel:
   - ✔️ change action --> cancel button --> begin editing action again --> original action is selected
- Accept:
   - ✔️ don't change anything
   - ✔️ change action --> OK! --> Save!
      - NOTE: The original action is still left as a stub `{ "command": "closePane" }`. This is intentional because we want to prevent all modifications to the command palette.
   - ✔️ change action & change key chord --> OK! --> Save!
   - ✔️ change action & change key chord (conflicting key chord) --> OK! --> click ok on flyout --> Save!
      - NOTE: original action is left as a stub; original key chord explicitly unbound; new command/keys combo added.

src/cascadia/TerminalSettingsEditor/Actions.cpp

@@ -7,6 +7,7 @@
 #include "KeyBindingViewModel.g.cpp"
 #include "ActionsPageNavigationState.g.cpp"
 #include "LibraryResources.h"
+#include "../TerminalSettingsModel/AllShortcutActions.h"
 
 using namespace winrt::Windows::Foundation;
 using namespace winrt::Windows::Foundation::Collections;
@@ -20,10 +21,12 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;
 
 namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 {
-    KeyBindingViewModel::KeyBindingViewModel(const Control::KeyChord& keys, const Model::Command& cmd) :
+    KeyBindingViewModel::KeyBindingViewModel(const Control::KeyChord& keys, const hstring& actionName, const IObservableVector<hstring>& availableActions) :
         _Keys{ keys },
         _KeyChordText{ Model::KeyChordSerialization::ToString(keys) },
-        _Command{ cmd }
+        _CurrentAction{ actionName },
+        _ProposedAction{ box_value(actionName) },
+        _AvailableActions{ availableActions }
     {
         // Add a property changed handler to our own property changed event.
         // This propagates changes from the settings model to anybody listening to our
@@ -43,6 +46,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             {
                 _NotifyChanges(L"ShowEditButton");
             }
+            else if (viewModelProperty == L"CurrentAction")
+            {
+                _NotifyChanges(L"Name");
+            }
         });
     }
 
@@ -63,8 +70,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         if (_IsInEditMode)
         {
             // if we're in edit mode,
-            // pre-populate the text box with the current keys
+            // - pre-populate the text box with the current keys
+            // - reset the combo box with the current action
             ProposedKeys(KeyChordText());
+            ProposedAction(box_value(CurrentAction()));
         }
     }
 
@@ -75,13 +84,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
 
     void KeyBindingViewModel::AttemptAcceptChanges(hstring newKeyChordText)
     {
-        auto args{ make_self<RebindKeysEventArgs>(_Keys, _Keys) };
+        auto args{ make_self<ModifyKeyBindingEventArgs>(_Keys, _Keys, _CurrentAction, unbox_value<hstring>(_ProposedAction)) };
+
+        // Key Chord Text
         try
         {
             // Attempt to convert the provided key chord text
             const auto newKeyChord{ KeyChordSerialization::FromString(newKeyChordText) };
             args->NewKeys(newKeyChord);
-            _RebindKeysRequestedHandlers(*this, *args);
         }
         catch (hresult_invalid_argument)
         {
@@ -94,6 +104,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             //  Alternatively, we want a full key chord editor/listener.
             //  If we implement that, we won't need this validation or error message.
         }
+
+        _ModifyKeyBindingRequestedHandlers(*this, *args);
     }
 
     Actions::Actions()
@@ -118,16 +130,28 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
     {
         _State = e.Parameter().as<Editor::ActionsPageNavigationState>();
 
+        // Populate AvailableActionAndArgs
+        _AvailableActionMap = single_threaded_map<hstring, Model::ActionAndArgs>();
+        std::vector<hstring> availableActionAndArgs;
+        for (const auto& [name, actionAndArgs] : _State.Settings().ActionMap().AvailableActions())
+        {
+            availableActionAndArgs.push_back(name);
+            _AvailableActionMap.Insert(name, actionAndArgs);
+        }
+        std::sort(begin(availableActionAndArgs), end(availableActionAndArgs));
+        _AvailableActionAndArgs = single_threaded_observable_vector(std::move(availableActionAndArgs));
+
         // Convert the key bindings from our settings into a view model representation
         const auto& keyBindingMap{ _State.Settings().ActionMap().KeyBindings() };
         std::vector<Editor::KeyBindingViewModel> keyBindingList;
         keyBindingList.reserve(keyBindingMap.Size());
         for (const auto& [keys, cmd] : keyBindingMap)
         {
-            auto container{ make_self<KeyBindingViewModel>(keys, cmd) };
+            // convert the cmd into a KeyBindingViewModel
+            auto container{ make_self<KeyBindingViewModel>(keys, cmd.Name(), _AvailableActionAndArgs) };
             container->PropertyChanged({ this, &Actions::_ViewModelPropertyChangedHandler });
             container->DeleteKeyBindingRequested({ this, &Actions::_ViewModelDeleteKeyBindingHandler });
-            container->RebindKeysRequested({ this, &Actions::_ViewModelRebindKeysHandler });
+            container->ModifyKeyBindingRequested({ this, &Actions::_ViewModelModifyKeyBindingHandler });
             container->IsAutomationPeerAttached(_AutomationPeerAttached);
             keyBindingList.push_back(*container);
         }
@@ -228,12 +252,42 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
     }
 
-    void Actions::_ViewModelRebindKeysHandler(const Editor::KeyBindingViewModel& senderVM, const Editor::RebindKeysEventArgs& args)
+    void Actions::_ViewModelModifyKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const Editor::ModifyKeyBindingEventArgs& args)
     {
+        auto applyChangesToSettingsModel = [=]() {
+            // If the key chord was changed,
+            // update the settings model and view model appropriately
+            if (args.OldKeys().Modifiers() != args.NewKeys().Modifiers() || args.OldKeys().Vkey() != args.NewKeys().Vkey())
+            {
+                // update settings model
+                _State.Settings().ActionMap().RebindKeys(args.OldKeys(), args.NewKeys());
+
+                // update view model
+                auto senderVMImpl{ get_self<KeyBindingViewModel>(senderVM) };
+                senderVMImpl->Keys(args.NewKeys());
+            }
+
+            // If the action was changed,
+            // update the settings model and view model appropriately
+            if (args.OldActionName() != args.NewActionName())
+            {
+                // convert the action's name into a view model.
+                const auto& newAction{ _AvailableActionMap.Lookup(args.NewActionName()) };
+
+                // update settings model
+                _State.Settings().ActionMap().RegisterKeyBinding(args.NewKeys(), newAction);
+
+                // update view model
+                auto senderVMImpl{ get_self<KeyBindingViewModel>(senderVM) };
+                senderVMImpl->CurrentAction(args.NewActionName());
+            }
+        };
+
+        // Check for this special case:
+        //  we're changing the key chord,
+        //  but the new key chord is already in use
         if (args.OldKeys().Modifiers() != args.NewKeys().Modifiers() || args.OldKeys().Vkey() != args.NewKeys().Vkey())
         {
-            // We're actually changing the key chord
-            const auto senderVMImpl{ get_self<KeyBindingViewModel>(senderVM) };
             const auto& conflictingCmd{ _State.Settings().ActionMap().GetActionByKeyChord(args.NewKeys()) };
             if (conflictingCmd)
             {
@@ -262,8 +316,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
                     senderVM.AcceptChangesFlyout(nullptr);
 
                     // update settings model and view model
-                    _State.Settings().ActionMap().RebindKeys(args.OldKeys(), args.NewKeys());
-                    senderVMImpl->Keys(args.NewKeys());
+                    applyChangesToSettingsModel();
                     senderVM.ToggleEditMode();
                 });
 
@@ -278,17 +331,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
                 senderVM.AcceptChangesFlyout(acceptChangesFlyout);
                 return;
             }
-            else
-            {
-                // update settings model
-                _State.Settings().ActionMap().RebindKeys(args.OldKeys(), args.NewKeys());
-
-                // update view model (keys)
-                senderVMImpl->Keys(args.NewKeys());
-            }
         }
 
-        // update view model (exit edit mode)
+        // update settings model and view model
+        applyChangesToSettingsModel();
+
+        // We NEED to toggle the edit mode here,
+        // so that if nothing changed, we still exit
+        // edit mode.
         senderVM.ToggleEditMode();
     }
 

src/cascadia/TerminalSettingsEditor/Actions.h

@@ -6,7 +6,7 @@
 #include "Actions.g.h"
 #include "KeyBindingViewModel.g.h"
 #include "ActionsPageNavigationState.g.h"
-#include "RebindKeysEventArgs.g.h"
+#include "ModifyKeyBindingEventArgs.g.h"
 #include "Utils.h"
 #include "ViewModelHelpers.h"
 
@@ -20,25 +20,28 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
     };
 
-    struct RebindKeysEventArgs : RebindKeysEventArgsT<RebindKeysEventArgs>
+    struct ModifyKeyBindingEventArgs : ModifyKeyBindingEventArgsT<ModifyKeyBindingEventArgs>
     {
     public:
-        RebindKeysEventArgs(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys) :
+        ModifyKeyBindingEventArgs(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys, const hstring oldActionName, const hstring newActionName) :
             _OldKeys{ oldKeys },
-            _NewKeys{ newKeys } {}
+            _NewKeys{ newKeys },
+            _OldActionName{ std::move(oldActionName) },
+            _NewActionName{ std::move(newActionName) } {}
 
         WINRT_PROPERTY(Control::KeyChord, OldKeys, nullptr);
         WINRT_PROPERTY(Control::KeyChord, NewKeys, nullptr);
+        WINRT_PROPERTY(hstring, OldActionName);
+        WINRT_PROPERTY(hstring, NewActionName);
     };
 
     struct KeyBindingViewModel : KeyBindingViewModelT<KeyBindingViewModel>, ViewModelHelper<KeyBindingViewModel>
     {
     public:
-        KeyBindingViewModel(const Control::KeyChord& keys, const Settings::Model::Command& cmd);
+        KeyBindingViewModel(const Control::KeyChord& keys, const hstring& name, const Windows::Foundation::Collections::IObservableVector<hstring>& availableActions);
 
-        hstring Name() const { return _Command.Name(); }
+        hstring Name() const { return _CurrentAction; }
         hstring KeyChordText() const { return _KeyChordText; }
-        Settings::Model::Command Command() const { return _Command; };
 
         // UIA Text
         hstring EditButtonName() const noexcept;
@@ -59,20 +62,35 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         void AttemptAcceptChanges(hstring newKeyChordText);
         void DeleteKeyBinding() { _DeleteKeyBindingRequestedHandlers(*this, _Keys); }
 
-        VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsInEditMode, false);
+        // 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.
+        // AvailableActions: the list of options in the combo box; both actions above must be in this list.
+        // NOTE: ProposedAction and CurrentAction may disagree mainly due to the "edit mode" system in place.
+        //       Current Action serves as...
+        //       1 - a record of what to set ProposedAction to on a cancellation
+        //       2 - a form of translation between ProposedAction and the settings model
+        //       We would also need an ActionMap reference to remove this, but this is a better separation
+        //       of responsibilities.
+        VIEW_MODEL_OBSERVABLE_PROPERTY(IInspectable, ProposedAction);
+        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);
+
+        VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsInEditMode, false);
         VIEW_MODEL_OBSERVABLE_PROPERTY(Windows::UI::Xaml::Controls::Flyout, AcceptChangesFlyout, nullptr);
         VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsAutomationPeerAttached, false);
         VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsHovered, false);
         VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsContainerFocused, false);
         VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsEditButtonFocused, false);
         VIEW_MODEL_OBSERVABLE_PROPERTY(Windows::UI::Xaml::Media::Brush, ContainerBackground, nullptr);
-        TYPED_EVENT(RebindKeysRequested, Editor::KeyBindingViewModel, Editor::RebindKeysEventArgs);
+        TYPED_EVENT(ModifyKeyBindingRequested, Editor::KeyBindingViewModel, Editor::ModifyKeyBindingEventArgs);
         TYPED_EVENT(DeleteKeyBindingRequested, Editor::KeyBindingViewModel, Terminal::Control::KeyChord);
 
     private:
-        Settings::Model::Command _Command{ nullptr };
         hstring _KeyChordText{};
     };
 
@@ -101,11 +119,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
     private:
         void _ViewModelPropertyChangedHandler(const Windows::Foundation::IInspectable& senderVM, const Windows::UI::Xaml::Data::PropertyChangedEventArgs& args);
         void _ViewModelDeleteKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const Control::KeyChord& args);
-        void _ViewModelRebindKeysHandler(const Editor::KeyBindingViewModel& senderVM, const Editor::RebindKeysEventArgs& args);
+        void _ViewModelModifyKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const Editor::ModifyKeyBindingEventArgs& args);
 
         std::optional<uint32_t> _GetContainerIndexByKeyChord(const Control::KeyChord& keys);
 
         bool _AutomationPeerAttached{ false };
+        Windows::Foundation::Collections::IObservableVector<hstring> _AvailableActionAndArgs;
+        Windows::Foundation::Collections::IMap<hstring, Model::ActionAndArgs> _AvailableActionMap;
     };
 }
 

src/cascadia/TerminalSettingsEditor/Actions.idl

@@ -5,10 +5,12 @@ import "EnumEntry.idl";
 
 namespace Microsoft.Terminal.Settings.Editor
 {
-    runtimeclass RebindKeysEventArgs
+    runtimeclass ModifyKeyBindingEventArgs
     {
         Microsoft.Terminal.Control.KeyChord OldKeys { get; };
         Microsoft.Terminal.Control.KeyChord NewKeys { get; };
+        String OldActionName { get; };
+        String NewActionName { get; };
     }
 
     runtimeclass KeyBindingViewModel : Windows.UI.Xaml.Data.INotifyPropertyChanged
@@ -21,6 +23,7 @@ namespace Microsoft.Terminal.Settings.Editor
         Boolean ShowEditButton { get; };
         Boolean IsInEditMode { get; };
         String ProposedKeys;
+        Object ProposedAction;
         Windows.UI.Xaml.Controls.Flyout AcceptChangesFlyout;
         String EditButtonName { get; };
         String CancelButtonName { get; };
@@ -34,11 +37,12 @@ namespace Microsoft.Terminal.Settings.Editor
         void ActionLostFocus();
         void EditButtonGettingFocus();
         void EditButtonLosingFocus();
+        IObservableVector<String> AvailableActions { get; };
         void ToggleEditMode();
         void AttemptAcceptChanges();
         void DeleteKeyBinding();
 
-        event Windows.Foundation.TypedEventHandler<KeyBindingViewModel, RebindKeysEventArgs> RebindKeysRequested;
+        event Windows.Foundation.TypedEventHandler<KeyBindingViewModel, ModifyKeyBindingEventArgs> ModifyKeyBindingRequested;
         event Windows.Foundation.TypedEventHandler<KeyBindingViewModel, Microsoft.Terminal.Control.KeyChord> DeleteKeyBindingRequested;
     }
 

src/cascadia/TerminalSettingsEditor/Actions.xaml

@@ -198,7 +198,15 @@
                         <!--  Command Name  -->
                         <TextBlock Grid.Column="0"
                                    Style="{StaticResource KeyBindingNameTextBlockStyle}"
-                                   Text="{x:Bind Name, Mode=OneWay}" />
+                                   Text="{x:Bind Name, Mode=OneWay}"
+                                   Visibility="{x:Bind IsInEditMode, Mode=OneWay, Converter={StaticResource InvertedBooleanToVisibilityConverter}}" />
+
+                        <!--  Edit Mode: Action Combo-box  -->
+                        <ComboBox Grid.Column="0"
+                                  VerticalAlignment="Center"
+                                  ItemsSource="{x:Bind AvailableActions, Mode=OneWay}"
+                                  SelectedItem="{x:Bind ProposedAction, Mode=TwoWay}"
+                                  Visibility="{x:Bind IsInEditMode, Mode=OneWay}" />
 
                         <!--  Key Chord Text  -->
                         <Border Grid.Column="1"

src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp

@@ -5,6 +5,7 @@
 #include "ActionAndArgs.g.cpp"
 
 #include "JsonUtils.h"
+#include "HashUtils.h"
 
 #include <LibraryResources.h>
 
@@ -117,6 +118,35 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 #undef ON_ALL_ACTIONS_WITH_ARGS
     };
 
+    ActionAndArgs::ActionAndArgs(ShortcutAction action)
+    {
+        // Find the deserializer
+        const auto deserializersIter = argSerializerMap.find(action);
+        if (deserializersIter != argSerializerMap.end())
+        {
+            auto pfn = deserializersIter->second.first;
+            if (pfn)
+            {
+                // Call the deserializer on an empty JSON object.
+                // This ensures that we have a valid ActionArgs
+                std::vector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings> parseWarnings;
+                std::tie(_Args, parseWarnings) = pfn({});
+            }
+
+            // if an arg parser was registered, but failed,
+            // return the invalid ActionAndArgs we started with.
+            if (pfn && _Args == nullptr)
+            {
+                return;
+            }
+        }
+
+        // Either...
+        // (1) we don't have a deserializer, so it's ok for _Args to be null, or
+        // (2) we had one AND it worked, so _Args is set up properly
+        _Action = action;
+    }
+
     // Function Description:
     // - Attempts to match a string to a ShortcutAction. If there's no match, then
     //   returns ShortcutAction::Invalid

src/cascadia/TerminalSettingsModel/ActionAndArgs.h

@@ -18,6 +18,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         static Json::Value ToJson(const Model::ActionAndArgs& val);
 
         ActionAndArgs() = default;
+        ActionAndArgs(ShortcutAction action);
         ActionAndArgs(ShortcutAction action, IActionArgs args) :
             _Action{ action },
             _Args{ args } {};