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

Add the 'Add new' button to the Actions page (#10550)

Browse source 
## Summary of the Pull Request
This adds the "add new" button to the actions page. It build on the work of #10220 by basically just adding a new list item to the top of the key binding list.

This also makes it so that if you click the "accept changes" button when you have an invalid key chord, we don't do anything.

## References
#6900 - Actions page Epic
#9427 - Actions page design doc
#10220 - Actions page PR - set action

## Detailed Description of the Pull Request / Additional comments
- `ModifyKeyBindingEventArgs` is used to introduce new key bindings. We just ignore `OldKeys` and `OldActionName` because both didn't exist before.
- `IsNewlyAdded` tracks if this is an action that was added, but has not been confirmed to add to the settings model.
- `CancelChanges()` is directly bound to the cancel button. This allows us to delete the key binding when it's clicked on a "newly added" action.

## Validation Steps Performed
- Cancel:
   - Deletes the action (because it doesn't truly exist until you confirm changes)
- Accept:
   - Adds the new action.
   - If you attempt to edit it, the delete button is back.
- Add Action:
   - Delete button should not be visible (redundant with 'Cancel')
   - Action should be initialized to a value
   - Key chord should be empty
   - Cannot add another action if a newly added action exists
- Keyboard interaction:
   - escape --> cancel
   - enter --> accept
- Accessibility:
   - "add new" button has a name
- Interaction with other key bindings:
   - editing another action --> delete the "newly added" action (it hasn't been added yet)
   - only one action can be edited at a time
This commit is contained in:
Carlos Zamora 2021-07-07 16:43:40 -07:00 committed by GitHub
parent a50731119a
commit 192d6debba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 122 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

111
src/cascadia/TerminalSettingsEditor/Actions.cpp
View file

@ -21,6 +21,9 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;
namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
KeyBindingViewModel::KeyBindingViewModel(const Windows::Foundation::Collections::IObservableVector<hstring>& availableActions) :
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) },
@ -84,18 +87,25 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void KeyBindingViewModel::AttemptAcceptChanges(hstring newKeyChordText)
{
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);
// 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
// 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.
@ -104,13 +114,25 @@ 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);
void KeyBindingViewModel::CancelChanges()
{
if (_IsNewlyAdded)
{
_DeleteNewlyAddedKeyBindingHandlers(*this, nullptr);
}
else
{
ToggleEditMode();
}
}
Actions::Actions()
{
InitializeComponent();
Automation::AutomationProperties::SetName(AddNewButton(), RS_(L"Actions_AddNewTextBlock/Text"));
}
Automation::Peers::AutomationPeer Actions::OnCreateAutomationPeer()
@ -149,10 +171,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
// 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->ModifyKeyBindingRequested({ this, &Actions::_ViewModelModifyKeyBindingHandler });
container->IsAutomationPeerAttached(_AutomationPeerAttached);
_RegisterEvents(container);
keyBindingList.push_back(*container);
}
@ -183,11 +202,30 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
else if (e.OriginalKey() == VirtualKey::Escape)
{
kbdVM.ToggleEditMode();
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.
auto kbdVM{ make_self<KeyBindingViewModel>(_AvailableActionAndArgs) };
_RegisterEvents(kbdVM);
kbdVM->DeleteNewlyAddedKeyBinding({ this, &Actions::_ViewModelDeleteNewlyAddedKeyBindingHandler });
// Manually add the editing background. This needs to be done in Actions not the view model.
// We also have to do this manually because it hasn't been added to the list yet.
kbdVM->IsInEditMode(true);
const auto& containerBackground{ Resources().Lookup(box_value(L"ActionContainerBackgroundEditing")).as<Windows::UI::Xaml::Media::Brush>() };
kbdVM->ContainerBackground(containerBackground);
// IMPORTANT: do this _after_ setting IsInEditMode. Otherwise, it'll get deleted immediately
// by the PropertyChangedHandler below (where we delete any IsNewlyAdded items)
kbdVM->IsNewlyAdded(true);
_KeyBindingList.InsertAt(0, *kbdVM);
}
void Actions::_ViewModelPropertyChangedHandler(const IInspectable& sender, const Windows::UI::Xaml::Data::PropertyChangedEventArgs& args)
{
const auto senderVM{ sender.as<Editor::KeyBindingViewModel>() };
@ -198,8 +236,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
// Ensure that...
// 1. we move focus to the edit mode controls
// 2. this is the only entry that is in edit mode
for (uint32_t i = 0; i < _KeyBindingList.Size(); ++i)
// 2. any actions that were newly added are removed
// 3. this is the only entry that is in edit mode
for (int32_t i = _KeyBindingList.Size() - 1; i >= 0; --i)
{
const auto& kbdVM{ _KeyBindingList.GetAt(i) };
if (senderVM == kbdVM)
@ -210,6 +249,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
const auto& container{ KeyBindingsListView().ContainerFromIndex(i).try_as<ListViewItem>() };
container.Focus(FocusState::Programmatic);
}
else if (kbdVM.IsNewlyAdded())
{
// Remove any actions that were newly added
_KeyBindingList.RemoveAt(i);
}
else
{
// Exit edit mode for all other containers
@ -254,13 +298,19 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void Actions::_ViewModelModifyKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const Editor::ModifyKeyBindingEventArgs& args)
{
const auto isNewAction{ !args.OldKeys() && args.OldActionName().empty() };
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())
// NOTE: we still need to update the view model if we're working with a newly added action
if (isNewAction || args.OldKeys().Modifiers() != args.NewKeys().Modifiers() || args.OldKeys().Vkey() != args.NewKeys().Vkey())
{
// update settings model
_State.Settings().ActionMap().RebindKeys(args.OldKeys(), args.NewKeys());
if (!isNewAction)
{
// update settings model
_State.Settings().ActionMap().RebindKeys(args.OldKeys(), args.NewKeys());
}
// update view model
auto senderVMImpl{ get_self<KeyBindingViewModel>(senderVM) };
@ -269,6 +319,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// If the action was changed,
// update the settings model and view model appropriately
// NOTE: no need to check for "isNewAction" here. <empty_string> != <action name> already.
if (args.OldActionName() != args.NewActionName())
{
// convert the action's name into a view model.
@ -280,13 +331,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// update view model
auto senderVMImpl{ get_self<KeyBindingViewModel>(senderVM) };
senderVMImpl->CurrentAction(args.NewActionName());
senderVMImpl->IsNewlyAdded(false);
}
};
// 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())
if (isNewAction || args.OldKeys().Modifiers() != args.NewKeys().Modifiers() || args.OldKeys().Vkey() != args.NewKeys().Vkey())
{
const auto& conflictingCmd{ _State.Settings().ActionMap().GetActionByKeyChord(args.NewKeys()) };
if (conflictingCmd)
@ -342,6 +394,19 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
senderVM.ToggleEditMode();
}
void Actions::_ViewModelDeleteNewlyAddedKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const IInspectable& /*args*/)
{
for (uint32_t i = 0; i < _KeyBindingList.Size(); ++i)
{
const auto& kbdVM{ _KeyBindingList.GetAt(i) };
if (kbdVM == senderVM)
{
_KeyBindingList.RemoveAt(i);
return;
}
}
}
// Method Description:
// - performs a search on KeyBindingList by key chord.
// Arguments:
@ -365,4 +430,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// to quickly search through the sorted list.
return std::nullopt;
}
void Actions::_RegisterEvents(com_ptr<KeyBindingViewModel>& kbdVM)
{
kbdVM->PropertyChanged({ this, &Actions::_ViewModelPropertyChangedHandler });
kbdVM->DeleteKeyBindingRequested({ this, &Actions::_ViewModelDeleteKeyBindingHandler });
kbdVM->ModifyKeyBindingRequested({ this, &Actions::_ViewModelModifyKeyBindingHandler });
kbdVM->IsAutomationPeerAttached(_AutomationPeerAttached);
}
}

7
src/cascadia/TerminalSettingsEditor/Actions.h
View file

@ -38,6 +38,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
struct KeyBindingViewModel : KeyBindingViewModelT<KeyBindingViewModel>, ViewModelHelper<KeyBindingViewModel>
{
public:
KeyBindingViewModel(const Windows::Foundation::Collections::IObservableVector<hstring>& availableActions);
KeyBindingViewModel(const Control::KeyChord& keys, const hstring& name, const Windows::Foundation::Collections::IObservableVector<hstring>& availableActions);
hstring Name() const { return _CurrentAction; }
@ -60,6 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void DisableEditMode() { IsInEditMode(false); }
void AttemptAcceptChanges();
void AttemptAcceptChanges(hstring newKeyChordText);
void CancelChanges();
void DeleteKeyBinding() { _DeleteKeyBindingRequestedHandlers(*this, _Keys); }
// ProposedAction: the entry selected by the combo box; may disagree with the settings model.
@ -81,6 +83,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
VIEW_MODEL_OBSERVABLE_PROPERTY(Control::KeyChord, Keys, nullptr);
VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsInEditMode, false);
VIEW_MODEL_OBSERVABLE_PROPERTY(bool, IsNewlyAdded, 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);
@ -89,6 +92,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
VIEW_MODEL_OBSERVABLE_PROPERTY(Windows::UI::Xaml::Media::Brush, ContainerBackground, nullptr);
TYPED_EVENT(ModifyKeyBindingRequested, Editor::KeyBindingViewModel, Editor::ModifyKeyBindingEventArgs);
TYPED_EVENT(DeleteKeyBindingRequested, Editor::KeyBindingViewModel, Terminal::Control::KeyChord);
TYPED_EVENT(DeleteNewlyAddedKeyBinding, Editor::KeyBindingViewModel, IInspectable);
private:
hstring _KeyChordText{};
@ -111,6 +115,7 @@ 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);
WINRT_PROPERTY(Editor::ActionsPageNavigationState, State, nullptr);
@ -120,8 +125,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
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 _ViewModelModifyKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const Editor::ModifyKeyBindingEventArgs& args);
void _ViewModelDeleteNewlyAddedKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const IInspectable& args);
std::optional<uint32_t> _GetContainerIndexByKeyChord(const Control::KeyChord& keys);
void _RegisterEvents(com_ptr<implementation::KeyBindingViewModel>& kbdVM);
bool _AutomationPeerAttached{ false };
Windows::Foundation::Collections::IObservableVector<hstring> _AvailableActionAndArgs;

2
src/cascadia/TerminalSettingsEditor/Actions.idl
View file

@ -22,6 +22,7 @@ namespace Microsoft.Terminal.Settings.Editor
// UI side
Boolean ShowEditButton { get; };
Boolean IsInEditMode { get; };
Boolean IsNewlyAdded { get; };
String ProposedKeys;
Object ProposedAction;
Windows.UI.Xaml.Controls.Flyout AcceptChangesFlyout;
@ -40,6 +41,7 @@ namespace Microsoft.Terminal.Settings.Editor
IObservableVector<String> AvailableActions { get; };
void ToggleEditMode();
void AttemptAcceptChanges();
void CancelChanges();
void DeleteKeyBinding();
event Windows.Foundation.TypedEventHandler<KeyBindingViewModel, ModifyKeyBindingEventArgs> ModifyKeyBindingRequested;

19
src/cascadia/TerminalSettingsEditor/Actions.xaml
View file

@ -286,7 +286,7 @@
<!-- Cancel editing the action -->
<Button x:Uid="Actions_CancelButton"
AutomationProperties.Name="{x:Bind CancelButtonName}"
Click="{x:Bind ToggleEditMode}"
Click="{x:Bind CancelChanges}"
Style="{StaticResource EditButtonStyle}">
<FontIcon FontSize="{StaticResource EditButtonIconSize}"
Glyph="&#xE711;" />
@ -307,7 +307,8 @@
<Button x:Uid="Actions_DeleteButton"
Margin="8,0,0,0"
AutomationProperties.Name="{x:Bind DeleteButtonName}"
Style="{StaticResource EditButtonStyle}">
Style="{StaticResource EditButtonStyle}"
Visibility="{x:Bind IsNewlyAdded, Mode=OneWay, Converter={StaticResource InvertedBooleanToVisibilityConverter}}">
<Button.Content>
<FontIcon FontSize="{StaticResource EditButtonIconSize}"
Glyph="&#xE74D;" />
@ -381,7 +382,21 @@
<ScrollViewer>
<StackPanel MaxWidth="600"
Spacing="8"
Style="{StaticResource SettingsStackStyle}">
<!-- Add New Button -->
<Button x:Name="AddNewButton"
Click="AddNew_Click">
<Button.Content>
<StackPanel Orientation="Horizontal">
<FontIcon FontSize="{StaticResource StandardIconSize}"
Glyph="&#xE710;" />
<TextBlock x:Uid="Actions_AddNewTextBlock"
Style="{StaticResource IconButtonTextBlockStyle}" />
</StackPanel>
</Button.Content>
</Button>
<!-- Keybindings -->
<ListView x:Name="KeyBindingsListView"
ItemTemplate="{StaticResource KeyBindingTemplate}"

4
src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
View file

@ -1154,4 +1154,8 @@
<value>Edit</value>
<comment>Text label for a button that can be used to begin making changes to a key binding entry.</comment>
</data>
<data name="Actions_AddNewTextBlock.Text" xml:space="preserve">
<value>Add new</value>
<comment>Button label that creates a new action on the actions page.</comment>
</data>
</root>