From 055ffabcdc658befb027f376de60306e31b164cc Mon Sep 17 00:00:00 2001
From: Pankaj Bhojwani <pankaj.d.bhoj@gmail.com>
Date: Fri, 28 Feb 2025 10:16:26 -0800
Subject: [PATCH] use grids

---
 .../ActionsViewModel.cpp                      |   2 +-
 .../TerminalSettingsEditor/EditAction.xaml    | 191 ++++++++++++------
 .../Resources/en-US/Resources.resw            |  16 ++
 .../TerminalSettingsModel/ActionArgs.h        |  20 +-
 .../TerminalSettingsModel/ActionArgs.idl      |   1 -
 .../TerminalSettingsModel/ActionArgsMagic.h   |  10 -
 6 files changed, 159 insertions(+), 81 deletions(-)

diff --git a/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
index c3103942c1..d75d6fd1de 100644
--- a/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
+++ b/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
@@ -486,7 +486,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         //      selection color
         // the above arg types aren't implemented yet - they all have multiple values within them
         // and require a different approach to binding/displaying. INewContentArgs is a bunch of args
-        // in one object, selected color has color and IsIndex16, multiple actions is... multiple actions:
+        // in one object, selected color has color and IsIndex16, multiple actions is... multiple actions
     }
 
     void ArgWrapper::EnumValue(const Windows::Foundation::IInspectable& enumValue)
diff --git a/src/cascadia/TerminalSettingsEditor/EditAction.xaml b/src/cascadia/TerminalSettingsEditor/EditAction.xaml
index 848e6ae1c4..e8bd009a14 100644
--- a/src/cascadia/TerminalSettingsEditor/EditAction.xaml
+++ b/src/cascadia/TerminalSettingsEditor/EditAction.xaml
@@ -122,6 +122,8 @@
                 </ResourceDictionary>
             </ResourceDictionary.ThemeDictionaries>
 
+            <GridLength x:Key="ArgumentNameWidth">148</GridLength>
+
             <!--  Styles  -->
             <Style x:Key="KeyBindingContainerStyle"
                    BasedOn="{StaticResource DefaultListViewItemStyle}"
@@ -233,93 +235,123 @@
             <DataTemplate x:Key="Int32Template"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <muxc:NumberBox Value="{x:Bind UnboxInt32(Value), Mode=TwoWay, BindBack=Int32BindBack}"
                                         Style="{StaticResource NumberBoxSettingStyle}"
                                         Minimum="0"
                                         Maximum="999"
                                         LargeChange="1"
-                                        SmallChange="1"/>
-                    </StackPanel>
+                                        SmallChange="1"
+                                        Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="UInt32Template"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <muxc:NumberBox Value="{x:Bind UnboxUInt32(Value), Mode=TwoWay, BindBack=UInt32BindBack}"
                                         Style="{StaticResource NumberBoxSettingStyle}"
                                         Minimum="0"
                                         Maximum="999"
                                         LargeChange="1"
-                                        SmallChange="1"/>
-                    </StackPanel>
+                                        SmallChange="1"
+                                        Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="UInt32OptionalTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <muxc:NumberBox Value="{x:Bind UnboxUInt32Optional(Value), Mode=TwoWay, BindBack=UInt32OptionalBindBack}"
                                         Style="{StaticResource NumberBoxSettingStyle}"
                                         Minimum="0"
                                         Maximum="999"
                                         LargeChange="1"
-                                        SmallChange="1"/>
-                    </StackPanel>
+                                        SmallChange="1"
+                                        Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="FloatTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <muxc:NumberBox Value="{x:Bind UnboxFloat(Value), Mode=TwoWay, BindBack=FloatBindBack}"
                                         Style="{StaticResource NumberBoxSettingStyle}"
                                         Minimum="0"
                                         Maximum="999"
                                         LargeChange="1"
-                                        SmallChange="1"/>
-                    </StackPanel>
+                                        SmallChange="1"
+                                        Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="StringTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
-                        <TextBox Text="{x:Bind UnboxString(Value), Mode=TwoWay, BindBack=StringBindBack}" />
-                    </StackPanel>
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
+                        <TextBox Text="{x:Bind UnboxString(Value), Mode=TwoWay, BindBack=StringBindBack}"
+                                 Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="BoolOptionalTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <CheckBox IsChecked="{x:Bind UnboxBoolOptional(Value), Mode=TwoWay, BindBack=BoolBindBack}"
-                                  IsThreeState="True" />
-                    </StackPanel>
+                                  IsThreeState="True"
+                                  Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
@@ -331,68 +363,96 @@
             <DataTemplate x:Key="EnumTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <ComboBox ItemTemplate="{StaticResource EnumComboBoxTemplate}"
                                   ItemsSource="{x:Bind EnumList, Mode=OneWay}"
                                   SelectedItem="{x:Bind EnumValue, Mode=TwoWay}"
-                                  Style="{StaticResource ComboBoxSettingStyle}" />
-                    </StackPanel>
+                                  Style="{StaticResource ComboBoxSettingStyle}"
+                                  Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="FlagItemTemplate"
                           x:DataType="local:FlagEntry">
-                <StackPanel Orientation="Horizontal"
-                            Spacing="8">
-                    <CheckBox IsChecked="{x:Bind IsSet, Mode=TwoWay}"/>
-                    <TextBlock Text="{x:Bind FlagName, Mode=OneWay}" />
-                </StackPanel>
+                <Grid ColumnSpacing="8">
+                    <Grid.ColumnDefinitions>
+                        <ColumnDefinition Width="Auto" />
+                        <ColumnDefinition Width="*" />
+                    </Grid.ColumnDefinitions>
+                    <CheckBox IsChecked="{x:Bind IsSet, Mode=TwoWay}"
+                              Grid.Column="0"/>
+                    <TextBlock Text="{x:Bind FlagName, Mode=OneWay}"
+                               Grid.Column="1"/>
+                </Grid>
             </DataTemplate>
 
             <DataTemplate x:Key="FlagTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid>
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <ListView ItemTemplate="{StaticResource FlagItemTemplate}"
                                   SelectionMode="None"
-                                  ItemsSource="{x:Bind FlagList, Mode=OneWay}" />
-                    </StackPanel>
+                                  ItemsSource="{x:Bind FlagList, Mode=OneWay}"
+                                  Grid.Column="1"
+                                  HorizontalAlignment="Left"
+                                  Padding="-8,0,0,0"
+                                  Margin="0"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="TerminalCoreColorOptionalTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <local:NullableColorPicker x:Uid="Actions_NullableColorPicker"
                                                    CurrentColor="{x:Bind UnboxTerminalCoreColorOptional(Value), Mode=TwoWay, BindBack=TerminalCoreColorBindBack}"
                                                    ColorSchemeVM="{x:Bind DefaultColorScheme, Mode=OneWay}"
-                                                   NullColorPreview="{x:Bind DefaultColorScheme.ForegroundColor.Color, Mode=OneWay}"/>
-                    </StackPanel>
+                                                   NullColorPreview="{x:Bind DefaultColorScheme.ForegroundColor.Color, Mode=OneWay}"
+                                                   Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
             <DataTemplate x:Key="WindowsUIColorOptionalTemplate"
                           x:DataType="local:ArgWrapper">
                 <ListViewItem>
-                    <StackPanel Orientation="Horizontal"
-                                Spacing="8">
+                    <Grid ColumnSpacing="8">
+                        <Grid.ColumnDefinitions>
+                            <ColumnDefinition Width="{StaticResource ArgumentNameWidth}" />
+                            <ColumnDefinition Width="Auto" />
+                        </Grid.ColumnDefinitions>
                         <TextBlock Text="{x:Bind Name}"
-                                   VerticalAlignment="Center" />
+                                   VerticalAlignment="Center"
+                                   Grid.Column="0"/>
                         <local:NullableColorPicker x:Uid="Actions_NullableColorPicker"
                                                    CurrentColor="{x:Bind UnboxWindowsUIColorOptional(Value), Mode=TwoWay, BindBack=WindowsUIColorBindBack}"
                                                    ColorSchemeVM="{x:Bind DefaultColorScheme, Mode=OneWay}"
-                                                   NullColorPreview="{x:Bind DefaultColorScheme.ForegroundColor.Color, Mode=OneWay}"/>
-                    </StackPanel>
+                                                   NullColorPreview="{x:Bind DefaultColorScheme.ForegroundColor.Color, Mode=OneWay}"
+                                                   Grid.Column="1"/>
+                    </Grid>
                 </ListViewItem>
             </DataTemplate>
 
@@ -416,7 +476,8 @@
         <Grid MaxWidth="600"
               HorizontalAlignment="Left"
               Style="{StaticResource SettingsStackStyle}"
-              RowSpacing="8">
+              RowSpacing="8"
+              ColumnSpacing="16">
             <Grid.RowDefinitions>
                 <RowDefinition Height="Auto" />
                 <RowDefinition Height="Auto" />
@@ -428,16 +489,28 @@
                 <ColumnDefinition Width="Auto" />
                 <ColumnDefinition Width="*" />
             </Grid.ColumnDefinitions>
+            <TextBlock x:Uid="Actions_Name"
+                       VerticalAlignment="Center"
+                       Grid.Column="0"
+                       Grid.Row="0"/>
             <TextBox Text="{x:Bind ViewModel.Name, Mode=TwoWay}"
                      IsEnabled="{x:Bind ViewModel.IsUserAction}"
                      Grid.Row="0"
                      Grid.Column="1"/>
+            <TextBlock x:Uid="Actions_ShortcutAction"
+                       VerticalAlignment="Center"
+                       Grid.Column="0"
+                       Grid.Row="1"/>
             <ComboBox VerticalAlignment="Center"
                       ItemsSource="{x:Bind ViewModel.AvailableShortcutActions, Mode=OneWay}"
                       SelectedItem="{x:Bind ViewModel.ProposedShortcutAction, Mode=TwoWay}"
                       IsEnabled="{x:Bind ViewModel.IsUserAction}"
                       Grid.Row="1"
                       Grid.Column="1"/>
+            <TextBlock x:Uid="Actions_Arguments"
+                       VerticalAlignment="Center"
+                       Grid.Column="0"
+                       Grid.Row="2"/>
             <ListView ItemsSource="{x:Bind ViewModel.ActionArgsVM.ArgValues, Mode=OneWay}"
                       ChoosingItemContainer="_choosingItemContainer"
                       SelectionMode="None"
@@ -447,6 +520,10 @@
                       Grid.Row="2"
                       Grid.Column="1">
             </ListView>
+            <TextBlock x:Uid="Actions_Keybindings"
+                       VerticalAlignment="Center"
+                       Grid.Column="0"
+                       Grid.Row="3"/>
             <ListView ItemTemplate="{StaticResource KeyChordTemplate}"
                       ItemsSource="{x:Bind ViewModel.KeyChordViewModelList, Mode=OneWay}"
                       SelectionMode="None"
@@ -461,7 +538,7 @@
             <Button Style="{StaticResource DeleteButtonStyle}"
                     IsEnabled="{x:Bind ViewModel.IsUserAction}"
                     Grid.Row="4"
-                    Grid.Column="1">
+                    Grid.Column="0">
                 <Button.Content>
                     <StackPanel Orientation="Horizontal">
                         <FontIcon FontSize="{StaticResource StandardIconSize}"
diff --git a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
index 00f64041d9..311a44bb27 100644
--- a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
+++ b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
@@ -1792,6 +1792,22 @@
     <value>Delete action</value>
     <comment>Button label that deletes the selected action.</comment>
   </data>
+  <data name="Actions_Name.Text" xml:space="preserve">
+    <value>Action name</value>
+    <comment>Label for the text box that edits the action name.</comment>
+  </data>
+  <data name="Actions_ShortcutAction.Text" xml:space="preserve">
+    <value>Action type</value>
+    <comment>Label for the combo box that edits the action type.</comment>
+  </data>
+  <data name="Actions_Arguments.Text" xml:space="preserve">
+    <value>Additional arguments</value>
+    <comment>Label for the list of editable arguments for the currently selected action.</comment>
+  </data>
+  <data name="Actions_Keybindings.Text" xml:space="preserve">
+    <value>Keybindings</value>
+    <comment>Label for the list of editable keybindings for the current command.</comment>
+  </data>
   <data name="Actions_AddKeyChord.Text" xml:space="preserve">
     <value>Add keybinding</value>
     <comment>Button label that adds a keybinding to the current action.</comment>
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h
index afd20d6474..f37f116166 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgs.h
+++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h
@@ -646,12 +646,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
         void SetArgAt(uint32_t /*index*/, IInspectable /*value*/)
         {
+            throw winrt::hresult_not_implemented();
         }
         void SetAllArgsToDefault()
         {
-        }
-        void SetRequiredArgsToDefault()
-        {
+            throw winrt::hresult_not_implemented();
         }
     };
 
@@ -759,12 +758,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
         void SetArgAt(uint32_t /*index*/, IInspectable /*value*/)
         {
+            throw winrt::hresult_not_implemented();
         }
         void SetAllArgsToDefault()
         {
-        }
-        void SetRequiredArgsToDefault()
-        {
+            throw winrt::hresult_not_implemented();
         }
     };
 
@@ -830,12 +828,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
         void SetArgAt(uint32_t /*index*/, IInspectable /*value*/)
         {
+            throw winrt::hresult_not_implemented();
         }
         void SetAllArgsToDefault()
         {
-        }
-        void SetRequiredArgsToDefault()
-        {
+            throw winrt::hresult_not_implemented();
         }
     };
 
@@ -990,12 +987,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
         void SetArgAt(uint32_t /*index*/, IInspectable /*value*/)
         {
+            throw winrt::hresult_not_implemented();
         }
         void SetAllArgsToDefault()
         {
-        }
-        void SetRequiredArgsToDefault()
-        {
+            throw winrt::hresult_not_implemented();
         }
     };
 
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
index 303f33da2c..01840635c7 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl
+++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl
@@ -24,7 +24,6 @@ namespace Microsoft.Terminal.Settings.Model
         IInspectable GetArgAt(UInt32 index);
         void SetArgAt(UInt32 index, Object value);
         void SetAllArgsToDefault();
-        void SetRequiredArgsToDefault();
     };
 
     interface IActionEventArgs
diff --git a/src/cascadia/TerminalSettingsModel/ActionArgsMagic.h b/src/cascadia/TerminalSettingsModel/ActionArgsMagic.h
index 957db260be..417881639d 100644
--- a/src/cascadia/TerminalSettingsModel/ActionArgsMagic.h
+++ b/src/cascadia/TerminalSettingsModel/ActionArgsMagic.h
@@ -106,12 +106,6 @@ struct InitListPlaceholder
 #define SET_ARG_TO_DEFAULT(type, name, jsonKey, required, ...) \
     _##name = static_cast<type>(__VA_ARGS__);
 
-#define SET_REQUIRED_ARG_TO_DEFAULT(type, name, jsonKey, required, ...) \
-    if (std::wstring_view(L## #required) != L"false")                   \
-    {                                                                   \
-        _##name = static_cast<type>(__VA_ARGS__);                       \
-    }
-
 // JSON deserialization. If the parameter is required to pass any validation,
 // add that as the `required` parameter here, as the body of a conditional
 // EX: For the RESIZE_PANE_ARGS
@@ -227,8 +221,4 @@ public:                                                          \
     void SetAllArgsToDefault()                                   \
     {                                                            \
         argsMacro(SET_ARG_TO_DEFAULT)                            \
-    }                                                            \
-    void SetRequiredArgsToDefault()                              \
-    {                                                            \
-        argsMacro(SET_REQUIRED_ARG_TO_DEFAULT)                   \
     }