Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
3 commits merged into from
Jul 7, 2021
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1209ae
Add the 'Add new' button to the Actions page
carlos-zamora Jul 2, 2021
d860085
apply PR feedback
carlos-zamora Jul 6, 2021
2664e86
make AttemptAcceptChanges more clear
carlos-zamora Jul 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 81 additions & 12 deletions src/cascadia/TerminalSettingsEditor/Actions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) },
Expand Down Expand Up @@ -84,7 +87,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

void KeyBindingViewModel::AttemptAcceptChanges(hstring newKeyChordText)
{
auto args{ make_self<ModifyKeyBindingEventArgs>(_Keys, _Keys, _CurrentAction, unbox_value<hstring>(_ProposedAction)) };
// NewlyAdded: introduce a new key binding
// Otherwise: include the old and new key binding data
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
auto args = _IsNewlyAdded ? make_self<ModifyKeyBindingEventArgs>(_Keys, unbox_value<hstring>(_ProposedAction)) :
make_self<ModifyKeyBindingEventArgs>(_Keys, _Keys, _CurrentAction, unbox_value<hstring>(_ProposedAction));

// Key Chord Text
try
Expand All @@ -108,9 +114,23 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_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()
Expand Down Expand Up @@ -149,10 +169,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);
}

Expand Down Expand Up @@ -183,11 +200,29 @@ 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 _before_ adding the VM to the list. Otherwise, it'll get deleted immediately.
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
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>() };
Expand All @@ -198,8 +233,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)
miniksa marked this conversation as resolved.
Show resolved Hide resolved
{
const auto& kbdVM{ _KeyBindingList.GetAt(i) };
if (senderVM == kbdVM)
Expand All @@ -210,6 +246,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
Expand Down Expand Up @@ -254,13 +295,18 @@ 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())
if (!args.OldKeys() || 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) };
Expand All @@ -269,6 +315,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.
Expand All @@ -280,13 +327,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)
Expand Down Expand Up @@ -342,6 +390,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:
Expand All @@ -365,4 +426,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);
}
}
13 changes: 13 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Actions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
struct ModifyKeyBindingEventArgs : ModifyKeyBindingEventArgsT<ModifyKeyBindingEventArgs>
{
public:
// used for new key bindings
ModifyKeyBindingEventArgs(const Control::KeyChord& keys, const hstring actionName) :
_NewKeys{ keys },
_NewActionName{ std::move(actionName) } {}

// used for modifying existing key bindings
ModifyKeyBindingEventArgs(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys, const hstring oldActionName, const hstring newActionName) :
_OldKeys{ oldKeys },
_NewKeys{ newKeys },
Expand All @@ -38,6 +44,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; }
Expand All @@ -60,6 +67,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.
Expand All @@ -81,6 +89,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);
Expand All @@ -89,6 +98,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{};
Expand All @@ -111,6 +121,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);
Expand All @@ -120,8 +131,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);
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

bool _AutomationPeerAttached{ false };
Windows::Foundation::Collections::IObservableVector<hstring> _AvailableActionAndArgs;
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Actions.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
19 changes: 17 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Actions.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;" />
Expand All @@ -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;" />
Expand Down Expand Up @@ -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}"
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should be "Add new..."? eh, maybe not though. idc.

<comment>Button label that creates a new action on the actions page.</comment>
</data>
</root>