From 06373748d910d374c9a3249ee523185c073b86b0 Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Fri, 7 Feb 2020 14:49:42 -0800 Subject: [PATCH 1/5] inconsistencies seem to be consistent --- src/cascadia/TerminalControl/TermControl.cpp | 40 +++++++++++++++----- src/cascadia/TerminalControl/TermControl.h | 1 + 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index d4bb8dc8c69..af21f089513 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -895,6 +895,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void TermControl::_PointerPressedHandler(Windows::Foundation::IInspectable const& sender, Input::PointerRoutedEventArgs const& args) { + Focus(FocusState::Pointer); + _CapturePointer(sender, args); const auto ptr = args.Pointer(); @@ -902,15 +904,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen) { - // Ignore mouse events while the terminal does not have focus. - // This prevents the user from selecting and copying text if they - // click inside the current tab to refocus the terminal window. - if (!_focused) - { - args.Handled(true); - return; - } - const auto modifiers = static_cast(args.KeyModifiers()); // static_cast to a uint32_t because we can't use the WI_IsFlagSet // macro directly with a VirtualKeyModifiers @@ -922,6 +915,21 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto cursorPosition = point.Position(); const auto terminalPosition = _GetTerminalPosition(cursorPosition); + // Ignore most mouse events while the terminal does not have focus. + // This prevents the user from selecting and copying text if they + // single click inside the current tab to refocus the terminal window. + // The one case we don't want to ignore is when the user tries to + // click-drag select. + if (!_focused) + { + // Save this click position in case the user is performing a click-drag. + // The PointerMovedHandler will use this position to set the selection anchor. + _clickDragStartPos = terminalPosition; + + args.Handled(true); + return; + } + // handle ALT key _terminal->SetBoxSelection(altEnabled); @@ -948,7 +956,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation } else { - // save location before rendering _terminal->SetSelectionAnchor(terminalPosition); } @@ -995,6 +1002,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { if (point.Properties().IsLeftButtonPressed()) { + if (_clickDragStartPos) + { + _terminal->SetSelectionAnchor(*_clickDragStartPos); + } + const auto cursorPosition = point.Position(); _SetEndSelectionPointAtCursor(cursorPosition); @@ -1070,6 +1082,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto ptr = args.Pointer(); + _clickDragStartPos = std::nullopt; + if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen) { const auto modifiers = static_cast(args.KeyModifiers()); @@ -1395,6 +1409,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { return; } + + if (this->FocusState() == FocusState::Unfocused) + { + _focused = false; + } + _focused = false; if (_uiaEngine.get()) diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 3bcd654c831..831ce10b55f 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -162,6 +162,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation Timestamp _lastMouseClick; unsigned int _multiClickCounter; std::optional _lastMouseClickPos; + std::optional _clickDragStartPos{ std::nullopt }; // Event revokers -- we need to deregister ourselves before we die, // lest we get callbacks afterwards. From 85cc0710aeaf8373110ec0f58e3b9fa1d4fdd105 Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Fri, 7 Feb 2020 15:04:04 -0800 Subject: [PATCH 2/5] removing a debugging code oops --- src/cascadia/TerminalControl/TermControl.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index af21f089513..2ad41cd28d6 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1409,12 +1409,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { return; } - - if (this->FocusState() == FocusState::Unfocused) - { - _focused = false; - } - _focused = false; if (_uiaEngine.get()) From 6907396fbed2bb914b4db5e82374d285464f6969 Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Fri, 7 Feb 2020 15:21:21 -0800 Subject: [PATCH 3/5] making the focus setting more local --- src/cascadia/TerminalControl/TermControl.cpp | 10 ++++++---- src/cascadia/TerminalControl/TermControl.h | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 2ad41cd28d6..108d0f18121 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -895,8 +895,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void TermControl::_PointerPressedHandler(Windows::Foundation::IInspectable const& sender, Input::PointerRoutedEventArgs const& args) { - Focus(FocusState::Pointer); - _CapturePointer(sender, args); const auto ptr = args.Pointer(); @@ -922,9 +920,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // click-drag select. if (!_focused) { + Focus(FocusState::Pointer); + // Save this click position in case the user is performing a click-drag. // The PointerMovedHandler will use this position to set the selection anchor. - _clickDragStartPos = terminalPosition; + _clickDragStartPos = point.Position(); args.Handled(true); return; @@ -966,6 +966,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation } else if (point.Properties().IsRightButtonPressed()) { + Focus(FocusState::Pointer); + // copyOnSelect causes right-click to always paste if (_terminal->IsCopyOnSelectActive() || !_terminal->IsSelectionActive()) { @@ -1004,7 +1006,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { if (_clickDragStartPos) { - _terminal->SetSelectionAnchor(*_clickDragStartPos); + _terminal->SetSelectionAnchor(_GetTerminalPosition(*_clickDragStartPos)); } const auto cursorPosition = point.Position(); diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 831ce10b55f..708baeb164c 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -162,7 +162,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation Timestamp _lastMouseClick; unsigned int _multiClickCounter; std::optional _lastMouseClickPos; - std::optional _clickDragStartPos{ std::nullopt }; + std::optional _clickDragStartPos{ std::nullopt }; // Event revokers -- we need to deregister ourselves before we die, // lest we get callbacks afterwards. From ae33440721a1b1d55fddadba1c5928b2da715393 Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Fri, 7 Feb 2020 15:24:07 -0800 Subject: [PATCH 4/5] simplifying --- src/cascadia/TerminalControl/TermControl.cpp | 4 ++-- src/cascadia/TerminalControl/TermControl.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 108d0f18121..54742f5d2f3 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -924,7 +924,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Save this click position in case the user is performing a click-drag. // The PointerMovedHandler will use this position to set the selection anchor. - _clickDragStartPos = point.Position(); + _clickDragStartPos = terminalPosition; args.Handled(true); return; @@ -1006,7 +1006,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { if (_clickDragStartPos) { - _terminal->SetSelectionAnchor(_GetTerminalPosition(*_clickDragStartPos)); + _terminal->SetSelectionAnchor(*_clickDragStartPos); } const auto cursorPosition = point.Position(); diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 708baeb164c..831ce10b55f 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -162,7 +162,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation Timestamp _lastMouseClick; unsigned int _multiClickCounter; std::optional _lastMouseClickPos; - std::optional _clickDragStartPos{ std::nullopt }; + std::optional _clickDragStartPos{ std::nullopt }; // Event revokers -- we need to deregister ourselves before we die, // lest we get callbacks afterwards. From 55593c6e8a290148d927e16f169b8fa0ccfd7d41 Mon Sep 17 00:00:00 2001 From: Leon Liang Date: Tue, 11 Feb 2020 10:38:03 -0800 Subject: [PATCH 5/5] adding comments and making things a little cleaner --- src/cascadia/TerminalControl/TermControl.cpp | 43 ++++++++++++-------- src/cascadia/TerminalControl/TermControl.h | 2 +- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 54742f5d2f3..8236bf10b23 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -900,6 +900,18 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto ptr = args.Pointer(); const auto point = args.GetCurrentPoint(_root); + if (!_focused) + { + Focus(FocusState::Pointer); + + // Save the click position here when the terminal does not have focus + // because they might be performing a click-drag selection. Since we + // only want to start the selection when the user moves the pointer with + // the left mouse button held down, the PointerMovedHandler will use + // this saved position to set the SelectionAnchor. + _clickDragStartPos = point.Position(); + } + if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse || ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Pen) { const auto modifiers = static_cast(args.KeyModifiers()); @@ -910,26 +922,19 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (point.Properties().IsLeftButtonPressed()) { - const auto cursorPosition = point.Position(); - const auto terminalPosition = _GetTerminalPosition(cursorPosition); - - // Ignore most mouse events while the terminal does not have focus. - // This prevents the user from selecting and copying text if they - // single click inside the current tab to refocus the terminal window. - // The one case we don't want to ignore is when the user tries to - // click-drag select. - if (!_focused) + // _clickDragStartPos having a value signifies to us that + // the user clicked on an unfocused terminal. We don't want + // a single left click from out of focus to start a selection, + // so we return fast here. + if (_clickDragStartPos) { - Focus(FocusState::Pointer); - - // Save this click position in case the user is performing a click-drag. - // The PointerMovedHandler will use this position to set the selection anchor. - _clickDragStartPos = terminalPosition; - args.Handled(true); return; } + const auto cursorPosition = point.Position(); + const auto terminalPosition = _GetTerminalPosition(cursorPosition); + // handle ALT key _terminal->SetBoxSelection(altEnabled); @@ -966,8 +971,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation } else if (point.Properties().IsRightButtonPressed()) { - Focus(FocusState::Pointer); - // copyOnSelect causes right-click to always paste if (_terminal->IsCopyOnSelectActive() || !_terminal->IsSelectionActive()) { @@ -1004,9 +1007,13 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { if (point.Properties().IsLeftButtonPressed()) { + // If this does not have a value, it means that PointerPressedHandler already + // set the SelectionAnchor. If it does have a value, that means the user is + // performing a click-drag selection on an unfocused terminal, so + // a SelectionAnchor isn't set yet. We'll have to set it here. if (_clickDragStartPos) { - _terminal->SetSelectionAnchor(*_clickDragStartPos); + _terminal->SetSelectionAnchor(_GetTerminalPosition(*_clickDragStartPos)); } const auto cursorPosition = point.Position(); diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 831ce10b55f..708baeb164c 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -162,7 +162,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation Timestamp _lastMouseClick; unsigned int _multiClickCounter; std::optional _lastMouseClickPos; - std::optional _clickDragStartPos{ std::nullopt }; + std::optional _clickDragStartPos{ std::nullopt }; // Event revokers -- we need to deregister ourselves before we die, // lest we get callbacks afterwards.