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.

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

Add support for a right-click context menu #14775

Merged
merged 36 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
eaf1a92
okay maybe I have silly free time ideas too
zadjii-msft Jul 23, 2022
7001e93
forwarded_event, string tests
zadjii-msft Jul 23, 2022
eb88cd3
practical use, look
zadjii-msft Jul 23, 2022
b535a5f
I suppose this is what happens when I have four uninterrupted hours a…
zadjii-msft Jul 23, 2022
520c89f
sad beeps
zadjii-msft Jul 23, 2022
eb7d2be
I am incapable of not making 100 tiny commits, this was already way t…
zadjii-msft Aug 13, 2022
e6b33c3
yea that's everything
zadjii-msft Aug 14, 2022
29d454b
90% plumbing, 10% bad implementation of making sure we right-clicked …
zadjii-msft Aug 28, 2022
451d8f3
move this code to interactivity. Scaling of event is wrong now
zadjii-msft Sep 19, 2022
99b84a6
Merge remote-tracking branch 'origin/main' into dev/migrie/f/3337-jus…
zadjii-msft Sep 20, 2022
dc2ebaf
use resource strings too. Last commit of stuff from before paternity …
zadjii-msft Oct 25, 2022
61a9c80
Migrate spelling-0.0.21 changes from main
DHowett Oct 25, 2022
191bca8
Merge branch 'main' into dev/migrie/f/3337-just-for-funsies
zadjii-msft Jan 29, 2023
6a60053
a commandbarflyout intsead, with a slightly more idomatic implementation
zadjii-msft Feb 1, 2023
2dd01fb
open at the mouse
zadjii-msft Feb 1, 2023
2ee4111
adapt the previous action adder for the new menu
zadjii-msft Feb 2, 2023
854335b
cleanup; don't duplicate selection entries each time; open in the rig…
zadjii-msft Feb 2, 2023
0328b4f
icons, and dismiss the menu when the action's done
zadjii-msft Feb 2, 2023
08ae0ec
cleanup for review
zadjii-msft Feb 2, 2023
d6fe520
'cleanup
zadjii-msft Feb 2, 2023
7aef3af
bommand
zadjii-msft Feb 3, 2023
c77f959
PR nits
zadjii-msft Feb 5, 2023
ab55a8d
Revert "sad beeps"
zadjii-msft Feb 5, 2023
559ecdb
Revert "I suppose this is what happens when I have four uninterrupted…
zadjii-msft Feb 5, 2023
9ff0abe
Revert "practical use, look"
zadjii-msft Feb 5, 2023
564102b
Revert "forwarded_event, string tests"
zadjii-msft Feb 5, 2023
a1d5329
Revert "okay maybe I have silly free time ideas too"
zadjii-msft Feb 5, 2023
d01cab3
Merge branch 'main' into dev/migrie/f/3337-just-for-funsies
zadjii-msft Feb 14, 2023
33b3eac
Merge remote-tracking branch 'origin/main' into dev/migrie/f/3337-jus…
zadjii-msft Mar 8, 2023
daddcf1
the easy nits
zadjii-msft Mar 8, 2023
4dcc1a3
Merge remote-tracking branch 'origin/main' into dev/migrie/f/3337-jus…
zadjii-msft Mar 16, 2023
6674df7
Use MUX in TermControl for fun and profit and faster menu opening
zadjii-msft Mar 16, 2023
35455f1
UT are important too
zadjii-msft Mar 17, 2023
263786c
Merge branch 'main' into dev/migrie/f/3337-just-for-funsies
zadjii-msft Mar 17, 2023
1a29af2
Merge remote-tracking branch 'origin/main' into dev/migrie/f/3337-jus…
zadjii-msft Mar 17, 2023
bede33c
yea these are reasonable nits
zadjii-msft Mar 17, 2023
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
3 changes: 0 additions & 3 deletions src/cascadia/TerminalApp/ColorPickupFlyout.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
#include "pch.h"
#include "ColorPickupFlyout.h"
#include "ColorPickupFlyout.g.cpp"
#include "winrt/Windows.UI.Xaml.Media.h"
#include "winrt/Windows.UI.Xaml.Shapes.h"
#include "winrt/Windows.UI.Xaml.Interop.h"
#include <LibraryResources.h>

namespace winrt::TerminalApp::implementation
Expand Down
14 changes: 10 additions & 4 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,15 @@
<data name="TabClose" xml:space="preserve">
<value>Close Tab</value>
</data>
<data name="PaneClose" xml:space="preserve">
<value>Close Pane</value>
</data>
<data name="SplitTabText" xml:space="preserve">
<value>Split Tab</value>
</data>
<data name="SplitPaneText" xml:space="preserve">
<value>Split Pane</value>
</data>
<data name="TabColorChoose" xml:space="preserve">
<value>Color...</value>
</data>
Expand Down Expand Up @@ -708,9 +717,6 @@
<data name="DropPathTabRun.Text" xml:space="preserve">
<value>Open a new tab in given starting directory</value>
</data>
<data name="SplitTabText" xml:space="preserve">
<value>Split Tab</value>
</data>
<data name="DropPathTabNewWindow.Text" xml:space="preserve">
<value>Open a new window with given starting directory</value>
</data>
Expand Down Expand Up @@ -795,4 +801,4 @@
<data name="NewTabMenuFolderEmpty" xml:space="preserve">
<value>Empty...</value>
</data>
</root>
</root>
76 changes: 76 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,9 @@ namespace winrt::TerminalApp::implementation
});

term.ShowWindowChanged({ get_weak(), &TerminalPage::_ShowWindowChangedHandler });

term.ContextMenu().Opening({ this, &TerminalPage::_ContextMenuOpened });
term.SelectionContextMenu().Opening({ this, &TerminalPage::_SelectionMenuOpened });
}

// Method Description:
Expand Down Expand Up @@ -4305,6 +4308,78 @@ namespace winrt::TerminalApp::implementation
_updateThemeColors();
}

void TerminalPage::_ContextMenuOpened(const IInspectable& sender,
const IInspectable& /*args*/)
{
_PopulateContextMenu(sender, false /*withSelection*/);
}
void TerminalPage::_SelectionMenuOpened(const IInspectable& sender,
const IInspectable& /*args*/)
{
_PopulateContextMenu(sender, true /*withSelection*/);
}

void TerminalPage::_PopulateContextMenu(const IInspectable& sender,
const bool /*withSelection*/)
{
// withSelection can be used to add actions that only appear if there's
// selected text, like "search the web". In this initial draft, it's not
// actually augmented by the TerminalPage, so it's left commented out.
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

const auto& menu{ sender.try_as<MUX::Controls::CommandBarFlyout>() };
if (!menu)
{
return;
}

// Helper lambda for dispatching an ActionAndArgs onto the
// ShortcutActionDispatch. Used below to wire up each menu entry to the
// respective action.

auto weak = get_weak();
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
auto makeCallback = [weak](const ActionAndArgs& actionAndArgs) {
return [weak, actionAndArgs](auto&&, auto&&) {
if (auto page{ weak.get() })
{
page->_actionDispatch->DoAction(actionAndArgs);
}
};
};

auto makeItem = [&menu, &makeCallback](const winrt::hstring& label,
const winrt::hstring& icon,
const auto& action) {
AppBarButton button{};

if (!icon.empty())
{
auto iconElement = IconPathConverter::IconWUX(icon);
Automation::AutomationProperties::SetAccessibilityView(iconElement, Automation::Peers::AccessibilityView::Raw);
button.Icon(iconElement);
}

button.Label(label);
button.Click(makeCallback(action));
menu.SecondaryCommands().Append(button);
};

// Wire up each item to the action that should be performed. By actually
// connecting these to actions, we ensure the implementation is
// consistent. This also leaves room for customizing this menu with
// actions in the future.

makeItem(RS_(L"SplitPaneText"), L"\xF246", ActionAndArgs{ ShortcutAction::SplitPane, SplitPaneArgs{ SplitType::Duplicate } });
makeItem(RS_(L"DuplicateTabText"), L"\xF5ED", ActionAndArgs{ ShortcutAction::DuplicateTab, nullptr });

// Only wire up "Close Pane" if there's multiple panes.
Copy link
Member

Choose a reason for hiding this comment

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

Design q: should we display this but leave it disabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Given past conversations on the repo, I reckon that most users don't even know what a Pane is, so that might be confusing. I erred on the side of "display the fewest possible valid options".

I am open to being overruled, idgaf.

if (_GetFocusedTabImpl()->GetLeafPaneCount() > 1)
{
makeItem(RS_(L"PaneClose"), L"\xE89F", ActionAndArgs{ ShortcutAction::ClosePane, nullptr });
}

makeItem(RS_(L"TabClose"), L"\xE711", ActionAndArgs{ ShortcutAction::CloseTab, CloseTabArgs{ _GetFocusedTabIndex().value() } });
}

// Handler for our WindowProperties's PropertyChanged event. We'll use this
// to pop the "Identify Window" toast when the user renames our window.
winrt::fire_and_forget TerminalPage::_windowPropertyChanged(const IInspectable& /*sender*/,
Expand All @@ -4328,4 +4403,5 @@ namespace winrt::TerminalApp::implementation
}
}
}

}
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,10 @@ namespace winrt::TerminalApp::implementation
winrt::fire_and_forget _ShowWindowChangedHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::ShowWindowArgs args);
winrt::fire_and_forget _windowPropertyChanged(const IInspectable& sender, const winrt::Windows::UI::Xaml::Data::PropertyChangedEventArgs& args);

void _ContextMenuOpened(const IInspectable& sender, const IInspectable& args);
void _SelectionMenuOpened(const IInspectable& sender, const IInspectable& args);
void _PopulateContextMenu(const IInspectable& sender, const bool withSelection);

#pragma region ActionHandlers
// These are all defined in AppActionHandlers.cpp
#define ON_ALL_ACTIONS(action) DECLARE_ACTION_HANDLER(action);
Expand Down
25 changes: 17 additions & 8 deletions src/cascadia/TerminalControl/ControlInteractivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// GH#9396: we prioritize hyper-link over VT mouse events
auto hyperlink = _core->GetHyperlink(terminalPosition.to_core_point());
if (WI_IsFlagSet(buttonState, MouseButtonState::IsLeftButtonDown) &&
ctrlEnabled && !hyperlink.empty())
ctrlEnabled &&
!hyperlink.empty())
Comment on lines +208 to +209
Copy link
Member

Choose a reason for hiding this comment

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

nit: change req'd?

{
const auto clickCount = _numberOfClicks(pixelPosition, timestamp);
// Handle hyper-link only on the first click to prevent multiple activations
Expand Down Expand Up @@ -255,14 +256,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown))
{
// Try to copy the text and clear the selection
const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr);
_core->ClearSelection();
if (_core->CopyOnSelect() || !successfulCopy)
if (_core->Settings().RightClickContextMenu())
{
// CopyOnSelect: right click always pastes!
// Otherwise: no selection --> paste
RequestPasteTextFromClipboard();
auto contextArgs = winrt::make<ContextMenuRequestedEventArgs>(til::point{ pixelPosition }.to_winrt_point());
_ContextMenuRequestedHandlers(*this, contextArgs);
}
else
{
// Try to copy the text and clear the selection
const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr);
_core->ClearSelection();
if (_core->CopyOnSelect() || !successfulCopy)
{
// CopyOnSelect: right click always pastes!
// Otherwise: no selection --> paste
RequestPasteTextFromClipboard();
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlInteractivity.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TYPED_EVENT(OpenHyperlink, IInspectable, Control::OpenHyperlinkEventArgs);
TYPED_EVENT(PasteFromClipboard, IInspectable, Control::PasteFromClipboardEventArgs);
TYPED_EVENT(ScrollPositionChanged, IInspectable, Control::ScrollPositionChangedArgs);
TYPED_EVENT(ContextMenuRequested, IInspectable, Control::ContextMenuRequestedEventArgs);

private:
// NOTE: _uiaEngine must be ordered before _core.
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/ControlInteractivity.idl
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, ScrollPositionChangedArgs> ScrollPositionChanged;
event Windows.Foundation.TypedEventHandler<Object, PasteFromClipboardEventArgs> PasteFromClipboard;

// Used to communicate to the TermControl, but not necessarily higher up in the stack
event Windows.Foundation.TypedEventHandler<Object, ContextMenuRequestedEventArgs> ContextMenuRequested;


};
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/EventArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "EventArgs.h"
#include "TitleChangedEventArgs.g.cpp"
#include "CopyToClipboardEventArgs.g.cpp"
#include "ContextMenuRequestedEventArgs.g.cpp"
#include "PasteFromClipboardEventArgs.g.cpp"
#include "OpenHyperlinkEventArgs.g.cpp"
#include "NoticeEventArgs.g.cpp"
Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/TerminalControl/EventArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "TitleChangedEventArgs.g.h"
#include "CopyToClipboardEventArgs.g.h"
#include "ContextMenuRequestedEventArgs.g.h"
#include "PasteFromClipboardEventArgs.g.h"
#include "OpenHyperlinkEventArgs.g.h"
#include "NoticeEventArgs.g.h"
Expand Down Expand Up @@ -53,6 +54,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
Windows::Foundation::IReference<CopyFormat> _formats;
};

struct ContextMenuRequestedEventArgs : public ContextMenuRequestedEventArgsT<ContextMenuRequestedEventArgs>
{
public:
ContextMenuRequestedEventArgs(winrt::Windows::Foundation::Point pos) :
_Position(pos) {}

WINRT_PROPERTY(winrt::Windows::Foundation::Point, Position);
};

struct PasteFromClipboardEventArgs : public PasteFromClipboardEventArgsT<PasteFromClipboardEventArgs>
{
public:
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalControl/EventArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ namespace Microsoft.Terminal.Control
Windows.Foundation.IReference<CopyFormat> Formats { get; };
}

runtimeclass ContextMenuRequestedEventArgs
{
Windows.Foundation.Point Position { get; };
}

runtimeclass TitleChangedEventArgs
{
String Title;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/IControlSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,6 @@ namespace Microsoft.Terminal.Control
Boolean SoftwareRendering { get; };
Boolean ShowMarks { get; };
Boolean UseBackgroundImageForWindow { get; };
Boolean RightClickContextMenu { get; };
};
}
34 changes: 33 additions & 1 deletion src/cascadia/TerminalControl/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,36 @@ Please either install the missing font or choose another one.</value>
<value>No results found</value>
<comment>Announced to a screen reader when the user searches for some text and there are no matches for that text in the terminal.</comment>
</data>
</root>
<data name="PasteCommandButton.Label" xml:space="preserve">
<value>Paste</value>
<comment>The label of a button for pasting the contents of the clipboard.</comment>
</data>
<data name="PasteCommandButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Paste</value>
<comment>The tooltip for a paste button</comment>
</data>
<data name="CopyCommandButton.Label" xml:space="preserve">
<value>Copy</value>
<comment>The label of a button for copying the selected text to the clipboard.</comment>
</data>
<data name="CopyCommandButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Copy</value>
<comment>The tooltip for a copy button</comment>
</data>
<data name="PasteWithSelectionCommandButton.Label" xml:space="preserve">
<value>Paste</value>
<comment>The label of a button for pasting the contents of the clipboard.</comment>
</data>
<data name="PasteWithSelectionCommandButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Paste</value>
<comment>The tooltip for a paste button</comment>
</data>
<data name="SearchCommandButton.Label" xml:space="preserve">
<value>Find...</value>
<comment>The label of a button for searching for the selected text</comment>
</data>
<data name="SearchCommandButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Find</value>
<comment>The tooltip for a button for searching for the selected text</comment>
</data>
</root>
2 changes: 0 additions & 2 deletions src/cascadia/TerminalControl/SearchBoxControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ Author(s):
--*/

#pragma once
#include "winrt/Windows.UI.Xaml.h"
#include "winrt/Windows.UI.Xaml.Controls.h"

#include "SearchBoxControl.g.h"

Expand Down
Loading