Add additional shortcut validation logic in Remap Shortcuts UI (#4068)

* Add more validation logic for shortcut remaps UI

* Fixed a bug in CheckRepeatedModifier logic

src/modules/keyboardmanager/ui/KeyDropDownControl.cpp

@@ -125,11 +125,9 @@ void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& singleKeyC
     });
 }
 
-// Function to set selection handler for shortcut drop down. Needs to be called after the constructor since the shortcutControl StackPanel is null if called in the constructor
+std::pair<KeyboardManagerHelper::ErrorType, int> KeyDropDownControl::ValidateShortcutSelection(Grid table, StackPanel shortcutControl, StackPanel parent, int colIndex, std::vector<std::vector<Shortcut>>& shortcutRemapBuffer, std::vector<std::unique_ptr<KeyDropDownControl>>& keyDropDownControlObjects)
-void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& shortcutControl, StackPanel parent, int colIndex, std::vector<std::vector<Shortcut>>& shortcutRemapBuffer, std::vector<std::unique_ptr<KeyDropDownControl>>& keyDropDownControlObjects)
 {
-    auto onSelectionChange = [&, table, shortcutControl, colIndex, parent](winrt::Windows::Foundation::IInspectable const& sender) {
+    ComboBox currentDropDown = dropDown;
-        ComboBox currentDropDown = sender.as<ComboBox>();
     int selectedKeyIndex = currentDropDown.SelectedIndex();
     uint32_t dropDownIndex = -1;
     bool dropDownFound = parent.Children().IndexOf(currentDropDown, dropDownIndex);
@@ -138,10 +136,11 @@ void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& shortcutCo
     bool controlIindexFound = table.Children().IndexOf(shortcutControl, controlIndex);
     KeyboardManagerHelper::ErrorType errorType = KeyboardManagerHelper::ErrorType::NoError;
     bool IsDeleteDropDownRequired = false;
+    int rowIndex = -1;
 
     if (controlIindexFound)
     {
-            int rowIndex = (controlIndex - KeyboardManagerConstants::ShortcutTableHeaderCount) / KeyboardManagerConstants::ShortcutTableColCount;
+        rowIndex = (controlIndex - KeyboardManagerConstants::ShortcutTableHeaderCount) / KeyboardManagerConstants::ShortcutTableColCount;
         if (selectedKeyIndex != -1 && keyCodeList.size() > selectedKeyIndex && dropDownFound)
         {
             // If only 1 drop down and action key is chosen: Warn that a modifier must be chosen
@@ -153,11 +152,11 @@ void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& shortcutCo
             // If it is the last drop down
             else if (dropDownIndex == parent.Children().Size() - 1)
             {
-                    // If last drop down and a modifier is selected: add a new drop down (max of 5 drop downs should be enforced)
+                // If last drop down and a modifier is selected: add a new drop down (max drop down count should be enforced)
                 if (KeyboardManagerHelper::IsModifierKey(keyCodeList[selectedKeyIndex]) && parent.Children().Size() < KeyboardManagerConstants::MaxShortcutSize)
                 {
                     // If it matched any of the previous modifiers then reset that drop down
-                        if (CheckRepeatedModifier(parent, dropDownIndex, selectedKeyIndex, keyCodeList))
+                    if (CheckRepeatedModifier(parent, selectedKeyIndex, keyCodeList))
                     {
                         // warn and reset the drop down
                         errorType = KeyboardManagerHelper::ErrorType::ShortcutCannotHaveRepeatedModifier;
@@ -188,7 +187,7 @@ void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& shortcutCo
                 if (KeyboardManagerHelper::IsModifierKey(keyCodeList[selectedKeyIndex]))
                 {
                     // If it matched any of the previous modifiers then reset that drop down
-                        if (CheckRepeatedModifier(parent, dropDownIndex, selectedKeyIndex, keyCodeList))
+                    if (CheckRepeatedModifier(parent, selectedKeyIndex, keyCodeList))
                     {
                         // warn and reset the drop down
                         errorType = KeyboardManagerHelper::ErrorType::ShortcutCannotHaveRepeatedModifier;
@@ -294,8 +293,40 @@ void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& shortcutCo
             keyDropDownControlObjects.erase(keyDropDownControlObjects.begin() + dropDownIndex);
             parent.UpdateLayout();
         }
+    }
+
+    return std::make_pair(errorType, rowIndex);
+}
+
+// Function to set selection handler for shortcut drop down. Needs to be called after the constructor since the shortcutControl StackPanel is null if called in the constructor
+void KeyDropDownControl::SetSelectionHandler(Grid& table, StackPanel& shortcutControl, StackPanel parent, int colIndex, std::vector<std::vector<Shortcut>>& shortcutRemapBuffer, std::vector<std::unique_ptr<KeyDropDownControl>>& keyDropDownControlObjects)
+{
+    auto onSelectionChange = [&, table, shortcutControl, colIndex, parent](winrt::Windows::Foundation::IInspectable const& sender) {
+        std::pair<KeyboardManagerHelper::ErrorType, int> validationResult = ValidateShortcutSelection(table, shortcutControl, parent, colIndex, shortcutRemapBuffer, keyDropDownControlObjects);
+
+        // Check if the drop down row index was identified from the return value of validateSelection
+        if (validationResult.second != -1)
+        {
+            // If an error occurred
+            if (validationResult.first != KeyboardManagerHelper::ErrorType::NoError)
+            {
+                // Iterate over all drop downs from left to right in that row/col and validate if there is an error in any of the drop downs. After this the state should be error-free (if it is a valid shortcut)
+                for (int i = 0; i < keyDropDownControlObjects.size(); i++)
+                {
+                    // Check for errors only if the current selection is a valid shortcut
+                    Shortcut tempComputedShortcut;
+                    tempComputedShortcut.SetKeyCodes(keyDropDownControlObjects[i]->GetKeysFromStackPanel(parent));
+
+                    // If the shortcut is valid and that drop down is not empty
+                    if (tempComputedShortcut.IsValidShortcut() && keyDropDownControlObjects[i]->GetComboBox().SelectedIndex() != -1)
+                    {
+                        keyDropDownControlObjects[i]->ValidateShortcutSelection(table, shortcutControl, parent, colIndex, shortcutRemapBuffer, keyDropDownControlObjects);
+                    }
+                }
+            }
+
             // Reset the buffer based on the new selected drop down items
-            shortcutRemapBuffer[rowIndex][colIndex].SetKeyCodes(GetKeysFromStackPanel(parent));
+            shortcutRemapBuffer[validationResult.second][colIndex].SetKeyCodes(GetKeysFromStackPanel(parent));
         }
 
         // If the user searches for a key the selection handler gets invoked however if they click away it reverts back to the previous state. This can result in dangling references to added drop downs which were then reset.
@@ -370,15 +401,27 @@ std::vector<DWORD> KeyDropDownControl::GetKeysFromStackPanel(StackPanel parent)
 }
 
 // Function to check if a modifier has been repeated in the previous drop downs
-bool KeyDropDownControl::CheckRepeatedModifier(StackPanel parent, uint32_t dropDownIndex, int selectedKeyIndex, const std::vector<DWORD>& keyCodeList)
+bool KeyDropDownControl::CheckRepeatedModifier(StackPanel parent, int selectedKeyIndex, const std::vector<DWORD>& keyCodeList)
 {
     // check if modifier has already been added before in a previous drop down
     std::vector<DWORD> currentKeys = GetKeysFromStackPanel(parent);
+    int currentDropDownIndex = -1;
+
+    // Find the key index of the current drop down selection so that we skip that index while searching for repeated modifiers
+    for (int i = 0; i < currentKeys.size(); i++)
+    {
+        if (currentKeys[i] == keyCodeList[selectedKeyIndex])
+        {
+            currentDropDownIndex = i;
+            break;
+        }
+    }
+
     bool matchPreviousModifier = false;
     for (int i = 0; i < currentKeys.size(); i++)
     {
         // Skip the current drop down
-        if (i != dropDownIndex)
+        if (i != currentDropDownIndex)
         {
             // If the key type for the newly added key matches any of the existing keys in the shortcut
             if (KeyboardManagerHelper::GetKeyType(keyCodeList[selectedKeyIndex]) == KeyboardManagerHelper::GetKeyType(currentKeys[i]))

src/modules/keyboardmanager/ui/KeyDropDownControl.h

@@ -35,6 +35,9 @@ public:
     // Function to set selection handler for single key remap drop down. Needs to be called after the constructor since the singleKeyControl StackPanel is null if called in the constructor
     void SetSelectionHandler(Grid& table, StackPanel& singleKeyControl, int colIndex, std::vector<std::vector<DWORD>>& singleKeyRemapBuffer);
 
+    // Function for validating the selection of shortcuts for the drop down
+    std::pair<KeyboardManagerHelper::ErrorType, int> ValidateShortcutSelection(Grid table, StackPanel shortcutControl, StackPanel parent, int colIndex, std::vector<std::vector<Shortcut>>& shortcutRemapBuffer, std::vector<std::unique_ptr<KeyDropDownControl>>& keyDropDownControlObjects);
+
     // Function to set selection handler for shortcut drop down. Needs to be called after the constructor since the shortcutControl StackPanel is null if called in the constructor
     void SetSelectionHandler(Grid& table, StackPanel& shortcutControl, StackPanel parent, int colIndex, std::vector<std::vector<Shortcut>>& shortcutRemapBuffer, std::vector<std::unique_ptr<KeyDropDownControl>>& keyDropDownControlObjects);
 
@@ -48,10 +51,10 @@ public:
     static void AddDropDown(Grid table, StackPanel shortcutControl, StackPanel parent, const int colIndex, std::vector<std::vector<Shortcut>>& shortcutRemapBuffer, std::vector<std::unique_ptr<KeyDropDownControl>>& keyDropDownControlObjects);
 
     // Function to get the list of key codes from the shortcut combo box stack panel
-    std::vector<DWORD> GetKeysFromStackPanel(StackPanel parent);
+    static std::vector<DWORD> GetKeysFromStackPanel(StackPanel parent);
 
     // Function to check if a modifier has been repeated in the previous drop downs
-    bool CheckRepeatedModifier(StackPanel parent, uint32_t dropDownIndex, int selectedKeyIndex, const std::vector<DWORD>& keyCodeList);
+    static bool CheckRepeatedModifier(StackPanel parent, int selectedKeyIndex, const std::vector<DWORD>& keyCodeList);
 
     // Function to set the warning message
     void SetDropDownError(ComboBox currentDropDown, hstring message);