Skip to content

Commit

Permalink
Add keyboard navigation to hyperlinks (#13405)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Adds support to navigate to clickable hyperlinks using only the keyboard. When in mark mode, the user can press [shift+]tab to go the previous/next hyperlink in the text buffer. Once a hyperlink is selected, the user can press <kbd>Ctrl+Enter</kbd> to open the hyperlink.

## References
#4993 

## PR Checklist
* [x] Closes #6649
* [x] Documentation updated at MicrosoftDocs/terminal#558

## Detailed Description of the Pull Request / Additional comments
- Main change
   - The `OpenHyperlink` event needs to be piped down to `ControlCore` now so that we can open a hyperlink at that layer.
   - `SelectHyperlink(dir)` searches the buffer in a given direction and finds the first hyperlink, then selects it.
   - "finding the hyperlink" is the tough part because the pattern tree takes in buffer coordinates, searches through the buffer in that given space, then stores everything relative to the defined space. Normally, the given buffer coordinates would align with the viewport's start and end. However, we're now trying to search outside of the viewport (sometimes), so we need to manage two coordinate systems at the same time.
   - `convertToSearchArea()` lambda was used to convert a given coordinate into the search area coordinate system. So if the search area is the visible viewport, we spit out a viewport position. If the search area is the _next_ viewport, we spit out a position relative to that.
   - `extractResultFromList()` lambda takes the list of patterns from the pattern tree and spits out the hyperlink we want. If we're searching forwards, we get the next one. Otherwise, we get the previous one. We explicitly ignore the one we're already on. If we don't find any, we return `nullopt`.
   - Now that we have all these cool tools, we use them to progressively search through the buffer to find the next/previous hyperlink. Start by searching the visible viewport _after_ (or _before_) the current selection. If we can't find anything, go to the next "page" (viewport scrolled up/down). Repeat this process until something comes up.
   - If we can't find anything, nothing happens. We don't wrap around.
- Other relevant changes
   - the `copy` action is no longer bound to `Enter`. Instead, we manually handle it in `ControlCore.cpp`. This also lets us handle <kbd>Shift+Enter</kbd> appropriately without having to take another key binding.
   - `_ScrollToPoint` was added. It's a simple function that just scrolls the viewport such that the provided buffer position is in view. This was used to de-duplicate code.
   - `_ScrollToPoint` was added into the `ToggleMarkMode()` function. Turns out, we don't scroll to the new selection when we enter mark mode (whoops!). We _should_ do that and we should backport this part of the change too. I'll handle that.
   - add some clarity when some functions are using the viewport position vs the buffer position. This is important because the pattern interval tree works in the viewport space.

## Validation Steps Performed
- case: all hyperlinks are in the view
   - ✅ get next link
   - ✅ get prev link
- ✅ case: need to scroll down for next link
- ✅ case: need to scroll up for next link
  • Loading branch information
carlos-zamora authored Jul 12, 2022
1 parent b4a52c8 commit 7abed9f
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 42 deletions.
52 changes: 41 additions & 11 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,13 +417,43 @@ namespace winrt::Microsoft::Terminal::Control::implementation
vkey != VK_SNAPSHOT &&
keyDown)
{
const auto isInMarkMode = _terminal->SelectionMode() == ::Microsoft::Terminal::Core::Terminal::SelectionInteractionMode::Mark;
if (isInMarkMode && modifiers.IsCtrlPressed() && vkey == 'A')
const auto isInMarkMode = _terminal->SelectionMode() == ::Terminal::SelectionInteractionMode::Mark;
if (isInMarkMode)
{
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_updateSelectionUI();
return true;
if (modifiers.IsCtrlPressed() && vkey == 'A')
{
// Ctrl + A --> Select all
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_updateSelectionUI();
return true;
}
else if (vkey == VK_TAB && _settings->DetectURLs())
{
// [Shift +] Tab --> next/previous hyperlink
auto lock = _terminal->LockForWriting();
const auto direction = modifiers.IsShiftPressed() ? ::Terminal::SearchDirection::Backward : ::Terminal::SearchDirection::Forward;
_terminal->SelectHyperlink(direction);
_updateSelectionUI();
return true;
}
else if (vkey == VK_RETURN && modifiers.IsCtrlPressed() && _terminal->IsTargetingUrl())
{
// Ctrl + Enter --> Open URL
auto lock = _terminal->LockForReading();
const auto uri = _terminal->GetHyperlinkAtBufferPosition(_terminal->GetSelectionAnchor());
_OpenHyperlinkHandlers(*this, winrt::make<OpenHyperlinkEventArgs>(winrt::hstring{ uri }));
return true;
}
else if (vkey == VK_RETURN)
{
// [Shift +] Enter --> copy text
// Don't lock here! CopySelectionToClipboard already locks for you!
CopySelectionToClipboard(modifiers.IsShiftPressed(), nullptr);
_terminal->ClearSelection();
_updateSelectionUI();
return true;
}
}

// try to update the selection
Expand Down Expand Up @@ -591,12 +621,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_lastHoveredCell = terminalPosition;
uint16_t newId{ 0u };
// we can't use auto here because we're pre-declaring newInterval.
decltype(_terminal->GetHyperlinkIntervalFromPosition({})) newInterval{ std::nullopt };
decltype(_terminal->GetHyperlinkIntervalFromViewportPosition({})) newInterval{ std::nullopt };
if (terminalPosition.has_value())
{
auto lock = _terminal->LockForReading(); // Lock for the duration of our reads.
newId = _terminal->GetHyperlinkIdAtPosition(*terminalPosition);
newInterval = _terminal->GetHyperlinkIntervalFromPosition(*terminalPosition);
newId = _terminal->GetHyperlinkIdAtViewportPosition(*terminalPosition);
newInterval = _terminal->GetHyperlinkIntervalFromViewportPosition(*terminalPosition);
}

// If the hyperlink ID changed or the interval changed, trigger a redraw all
Expand Down Expand Up @@ -626,15 +656,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
// Lock for the duration of our reads.
auto lock = _terminal->LockForReading();
return winrt::hstring{ _terminal->GetHyperlinkAtPosition(til::point{ pos }) };
return winrt::hstring{ _terminal->GetHyperlinkAtViewportPosition(til::point{ pos }) };
}

winrt::hstring ControlCore::HoveredUriText() const
{
auto lock = _terminal->LockForReading(); // Lock for the duration of our reads.
if (_lastHoveredCell.has_value())
{
return winrt::hstring{ _terminal->GetHyperlinkAtPosition(*_lastHoveredCell) };
return winrt::hstring{ _terminal->GetHyperlinkAtViewportPosition(*_lastHoveredCell) };
}
return {};
}
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TYPED_EVENT(FoundMatch, IInspectable, Control::FoundResultsArgs);
TYPED_EVENT(ShowWindowChanged, IInspectable, Control::ShowWindowArgs);
TYPED_EVENT(UpdateSelectionMarkers, IInspectable, Control::UpdateSelectionMarkersEventArgs);
TYPED_EVENT(OpenHyperlink, IInspectable, Control::OpenHyperlinkEventArgs);
// clang-format on

private:
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,6 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, FoundResultsArgs> FoundMatch;
event Windows.Foundation.TypedEventHandler<Object, ShowWindowArgs> ShowWindowChanged;
event Windows.Foundation.TypedEventHandler<Object, UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;

event Windows.Foundation.TypedEventHandler<Object, OpenHyperlinkEventArgs> OpenHyperlink;
};
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_core.HoveredHyperlinkChanged({ this, &TermControl::_hoveredHyperlinkChanged });
_core.FoundMatch({ this, &TermControl::_coreFoundMatch });
_core.UpdateSelectionMarkers({ this, &TermControl::_updateSelectionMarkers });
_core.OpenHyperlink({ this, &TermControl::_HyperlinkHandler });
_interactivity.OpenHyperlink({ this, &TermControl::_HyperlinkHandler });
_interactivity.ScrollPositionChanged({ this, &TermControl::_ScrollPositionChanged });

Expand Down
28 changes: 18 additions & 10 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Terminal::Terminal() :
_altGrAliasing{ true },
_blockSelection{ false },
_selectionMode{ SelectionInteractionMode::None },
_isTargetingUrl{ false },
_selection{ std::nullopt },
_selectionEndpoint{ static_cast<SelectionEndpoint>(0) },
_anchorInactiveSelectionEndpoint{ false },
Expand Down Expand Up @@ -563,17 +564,24 @@ bool Terminal::ShouldSendAlternateScroll(const unsigned int uiButton,
// Method Description:
// - Given a coord, get the URI at that location
// Arguments:
// - The position
std::wstring Terminal::GetHyperlinkAtPosition(const til::point position)
// - The position relative to the viewport
std::wstring Terminal::GetHyperlinkAtViewportPosition(const til::point viewportPos)
{
auto attr = _activeBuffer().GetCellDataAt(_ConvertToBufferCell(position))->TextAttr();
return GetHyperlinkAtBufferPosition(_ConvertToBufferCell(viewportPos));
}

std::wstring Terminal::GetHyperlinkAtBufferPosition(const til::point bufferPos)
{
auto attr = _activeBuffer().GetCellDataAt(bufferPos)->TextAttr();
if (attr.IsHyperlink())
{
auto uri = _activeBuffer().GetHyperlinkUriFromId(attr.GetHyperlinkId());
return uri;
}
// also look through our known pattern locations in our pattern interval tree
const auto result = GetHyperlinkIntervalFromPosition(position);
auto viewportPos = bufferPos;
_GetVisibleViewport().ConvertToOrigin(&viewportPos);
const auto result = GetHyperlinkIntervalFromViewportPosition(viewportPos);
if (result.has_value() && result->value == _hyperlinkPatternId)
{
const auto start = result->start;
Expand All @@ -594,23 +602,23 @@ std::wstring Terminal::GetHyperlinkAtPosition(const til::point position)
// Method Description:
// - Gets the hyperlink ID of the text at the given terminal position
// Arguments:
// - The position of the text
// - The position of the text relative to the viewport
// Return value:
// - The hyperlink ID
uint16_t Terminal::GetHyperlinkIdAtPosition(const til::point position)
uint16_t Terminal::GetHyperlinkIdAtViewportPosition(const til::point viewportPos)
{
return _activeBuffer().GetCellDataAt(_ConvertToBufferCell(position))->TextAttr().GetHyperlinkId();
return _activeBuffer().GetCellDataAt(_ConvertToBufferCell(viewportPos))->TextAttr().GetHyperlinkId();
}

// Method description:
// - Given a position in a URI pattern, gets the start and end coordinates of the URI
// Arguments:
// - The position
// - The position relative to the viewport
// Return value:
// - The interval representing the start and end coordinates
std::optional<PointTree::interval> Terminal::GetHyperlinkIntervalFromPosition(const til::point position)
std::optional<PointTree::interval> Terminal::GetHyperlinkIntervalFromViewportPosition(const til::point viewportPos)
{
const auto results = _patternIntervalTree.findOverlapping({ position.X + 1, position.Y }, position);
const auto results = _patternIntervalTree.findOverlapping({ viewportPos.X + 1, viewportPos.Y }, viewportPos);
if (results.size() > 0)
{
for (const auto& result : results)
Expand Down
17 changes: 14 additions & 3 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,10 @@ class Microsoft::Terminal::Core::Terminal final :

void FocusChanged(const bool focused) noexcept override;

std::wstring GetHyperlinkAtPosition(const til::point position);
uint16_t GetHyperlinkIdAtPosition(const til::point position);
std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> GetHyperlinkIntervalFromPosition(const til::point position);
std::wstring GetHyperlinkAtViewportPosition(const til::point viewportPos);
std::wstring GetHyperlinkAtBufferPosition(const til::point bufferPos);
uint16_t GetHyperlinkIdAtViewportPosition(const til::point viewportPos);
std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> GetHyperlinkIntervalFromViewportPosition(const til::point viewportPos);
#pragma endregion

#pragma region IBaseData(base to IRenderData and IUiaData)
Expand Down Expand Up @@ -249,6 +250,12 @@ class Microsoft::Terminal::Core::Terminal final :
Down
};

enum class SearchDirection
{
Forward,
Backward
};

enum class SelectionExpansion
{
Char,
Expand All @@ -273,6 +280,8 @@ class Microsoft::Terminal::Core::Terminal final :
SelectionInteractionMode SelectionMode() const noexcept;
void SwitchSelectionEndpoint();
void ToggleMarkMode();
void SelectHyperlink(const SearchDirection dir);
bool IsTargetingUrl() const noexcept;

using UpdateSelectionParams = std::optional<std::pair<SelectionDirection, SelectionExpansion>>;
UpdateSelectionParams ConvertKeyEventToUpdateSelectionParams(const ControlKeyStates mods, const WORD vkey) const;
Expand Down Expand Up @@ -349,6 +358,7 @@ class Microsoft::Terminal::Core::Terminal final :
std::wstring _wordDelimiters;
SelectionExpansion _multiClickSelectionMode;
SelectionInteractionMode _selectionMode;
bool _isTargetingUrl;
SelectionEndpoint _selectionEndpoint;
bool _anchorInactiveSelectionEndpoint;
#pragma endregion
Expand Down Expand Up @@ -424,6 +434,7 @@ class Microsoft::Terminal::Core::Terminal final :
std::pair<til::point, til::point> _PivotSelection(const til::point targetPos, bool& targetStart) const;
std::pair<til::point, til::point> _ExpandSelectionAnchors(std::pair<til::point, til::point> anchors) const;
til::point _ConvertToBufferCell(const til::point viewportPos) const;
void _ScrollToPoint(const til::point pos);
void _MoveByChar(SelectionDirection direction, til::point& pos);
void _MoveByWord(SelectionDirection direction, til::point& pos);
void _MoveByViewport(SelectionDirection direction, til::point& pos);
Expand Down
Loading

0 comments on commit 7abed9f

Please sign in to comment.