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

Keyboard navigation of clickable link #6649

Closed
Tracked by #5001
mithrandir opened this issue Jun 24, 2020 · 3 comments · Fixed by #13405
Closed
Tracked by #5001

Keyboard navigation of clickable link #6649

mithrandir opened this issue Jun 24, 2020 · 3 comments · Fixed by #13405
Labels
A11yMAS Accessibility tracking Area-Input Related to input processing (key presses, mouse, etc.) Disability-Mobility Accessibility tracking InclusionBacklog Accessibility tracking InclusionBacklog-Windows TerminalWin32 Accessibility tracking Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Backlog

Comments

@mithrandir
Copy link

Description of the new feature/enhancement

Keyboard navigation of clickable link in terminal text. Terminal Roadmap v2.0 is describing about clickable link

| Hyperlinking any links that appear in the text buffer. When clicking on the link, the link will open in your default browser.

However, this involves pointing device such as mouse or trackball. I believe many of terminal users love keyboard and it is important to keep using keyboard without switching to pointing device as much as possble.

Proposed technical implementation details (optional)

We may have a command like "NavigateToPreviousClickableLink" or "NavtigateToNextClickableLink" and "InvokeFocusedClickableLink", then we can reassign key binding those command.

bentaudun commented as #204 (comment) but wasn't mentioned in #574
@mithrandir mithrandir added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Jun 24, 2020
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 24, 2020
@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. labels Jun 24, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 24, 2020
@zadjii-msft
Copy link
Member

I agree this would be super useful. I'd think we could probably leave out InvokeFocusedClickableLink, and just leave that as space or enter, but now as I'm typing this up, maybe people would want to navigate to the previous link, then copy it, then return to the input line and paste it? So maybe another action for that makes sense? Though, we can't have multiple actions bound to the same key, so that wouldn't work with "enter to copy" people...

I'm maybe rambling a bit.

This is a good idea for inclusion once #574/#204 lands.

@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Jun 24, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 25, 2020
@DHowett DHowett mentioned this issue Jul 2, 2020
Closed
@zadjii-msft zadjii-msft mentioned this issue Feb 25, 2021
Open
22 tasks
@zadjii-msft zadjii-msft removed the Priority-1 A description (P1) label May 6, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@zadjii-msft zadjii-msft mentioned this issue May 16, 2022
Closed
@carlos-zamora
Copy link
Member

Copying some ideas from the dup I accidentally made...

Description of the new feature/enhancement

Windows Terminal supports automatic detection of hyperlinks and allows you to open the hyperlink using your mouse (specifically ctrl+clicking the link). I've realized that there's no way to do this using only the keyboard.

Once mark mode lands, the solution is a little better, but still far from perfect and on par. The user would be able to navigate to the text with the hyperlink, create a selection that encompasses it, copy the URL, then manually open it in their preferred web browser.

What if we could make this a nicer experience? Consider this...

  1. enter mark mode
  2. tab (or shift+tab) to navigate to these hyperlinks
  3. when you've navigated to the hyperlink, a selection is created that encompasses the relevant hyperlink.
  4. enter can be used to invoke the hyperlink directly

Some corner cases...

  • what if enter is bound to an action (like "copy")? --> prefer the action
  • what if we add more bookmark-like things like "prompt position" or user-configured bookmarks --> might as well navigate to them too? I'm thinking these can be tab stops.

From Mike...

Oh man do I have thoughts in #11000.

@carlos-zamora carlos-zamora added InclusionBacklog Accessibility tracking InclusionBacklog-Windows TerminalWin32 Accessibility tracking A11yMAS Accessibility tracking Disability-Mobility Accessibility tracking labels Jun 6, 2022
@zadjii-msft zadjii-msft mentioned this issue Jun 22, 2022
Open
11 tasks
This was referenced Jun 28, 2022
Merged
Merged
@ghost ghost added In-PR This issue has a related PR and removed In-PR This issue has a related PR labels Jun 30, 2022
@ghost ghost closed this as completed in #13405 Jul 12, 2022
ghost pushed a commit that referenced this issue Jul 12, 2022
@carlos-zamora
Add keyboard navigation to hyperlinks (#13405)
7abed9f 
## 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
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 12, 2022
@carlos-zamora carlos-zamora mentioned this issue Jul 13, 2022
Merged
6 tasks
ghost pushed a commit that referenced this issue Aug 25, 2022
@carlos-zamora
[Follow-up] Polish keyboard navigation to hyperlinks (#13494)
4488a25 
## Summary of the Pull Request
Polishes #13405 with some additional feedback found in testing and post-mortem reviews. Such feedback includes:
- [x] ControlInteractivity vs ControlCore split ([link](#13405 (comment)))
- [x] clearing the selection should be under lock when copying text via "Enter"
- [x] move mark mode keybindings into a helper function
- [x] decide if "Enter" should be configurable or non-configurable ([link](#13405 (comment)))
- [x] rename `_isTargetingUrl`
- [x] bugfix: ctrl+enter when the link is outside of the viewport

## References
Original PR: #13405
Relevant issue: #6649
Epic: #4993
@ghost
Copy link

ghost commented Sep 13, 2022

🎉This issue was addressed in #13405, which has now been successfully released as Windows Terminal Preview v1.16.252.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A11yMAS Accessibility tracking Area-Input Related to input processing (key presses, mouse, etc.) Disability-Mobility Accessibility tracking InclusionBacklog Accessibility tracking InclusionBacklog-Windows TerminalWin32 Accessibility tracking Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Add keyboard navigation to hyperlinks microsoft/terminal
4 participants
@DHowett @mithrandir @carlos-zamora @zadjii-msft