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

Support terminal link keyboard navigation #95570

Closed
Tyriar opened this issue Apr 17, 2020 · 30 comments · Fixed by #139620 or #140121
Closed

Support terminal link keyboard navigation #95570

Tyriar opened this issue Apr 17, 2020 · 30 comments · Fixed by #139620 or #140121
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan terminal Integrated terminal issues terminal-links
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Apr 17, 2020

Right now there is navigation mode which is similar to this but only with the text in the line. Where this gets really interesting is navigating types of links (eg. web links only, local files only, all links). Example mockup:

image

It could feature up, down, left, right as well as previous and next navigation.

@Tyriar Tyriar added feature-request Request for new features or functionality accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues terminal Integrated terminal issues terminal-links labels Apr 17, 2020
@Tyriar Tyriar added this to the May 2020 milestone Apr 17, 2020
@Tyriar Tyriar self-assigned this Apr 17, 2020
@Tyriar
Copy link
Member Author

Tyriar commented Apr 20, 2020

Idea: Show the hover while navigating the link with the keyboard so you can still see what will happen:

image

@Tyriar
Copy link
Member Author

Tyriar commented Apr 21, 2020

Another idea: Announce to screen readers how many rows were travelled.

@zersiax
Copy link

zersiax commented May 6, 2020

Going to admit, I have no idea what exactly you are trying to do here :) Can you, or anyone else, tell me:

  • What problem are we solving?
  • What is the currently proposed solution?

Thanks :)

@Tyriar
Copy link
Member Author

Tyriar commented May 6, 2020

@zersiax sorry, you're too quick 😃 I'll post a better summary of the problem soon, followed by a proposal.

@Tyriar
Copy link
Member Author

Tyriar commented May 7, 2020

Ok so here's the problem:

Links in the terminal have never been accessible, this is a feature where hovering your mouse over web links or local links that we've detected (eg. ./README.md, foo/bar.md, /foo/bar, etc.) allow you to ctrl/cmd+click them to open the link either in the editor or a browser.

In 1.45 this system has overgone a major overhaul and now features:

  • Link detection logic shared with the editor, meaning better web link detection, file:// link support, better unicode support, etc.
  • Folders are now supported, opening the explorer when the file is within the workspace or opening a new VS Code window if it's outside.
  • A new "word link" concept linkifies basically everything else based on the terminal.integrated.wordSeparators setting and searches the workspace for it. Using this will open the file if the workspace contains exactly 1 matching file, otherwise it will open quick access, basically a shortcut for ctrl+p and typing out the link text.
  • A new hover when you mouse over the link that describes exactly what will happen, rather than just "Follow link" as it was before.

The problem is about how to make this keyboard and screen reader accessible.

@zersiax
Copy link

zersiax commented May 7, 2020

Ok ...so this is actually a difficult one :)

Some ideas:

  • My thinking is that , when a link gets detected like that, some CSS is triggered to indicate this is a link visually. That tells me there's a hook that triggers that CSS to apear, which we can use. My thought is that for folder links, file links etc in the terminal, we can either wrap the text in a tag, or give it role="button" or role="link" to make it clickable. That would mean that a screen reader user will have to hop into browse mode to find these, but I don't think that can be helped in this case.
  • An alternative is to make a hotkey that cycles between the links found in the currently visible output, back and forward, and another one to activate the currently selected link in that cycling list. This is similar to how the VoiceOver rotor actions work on iOS.
  • As for the word link ... we need to do something else here. In this sccenario, the user is in the editor (for the most part) and therefore shouldn't be in browse mode. My thinking is to create an alternative that triggers the word-linking feature using a hotkey, taking as input the currently highlighted text in the editor.
    Does that cover everything?

@isidorn , perhaps @MarcoZehe , thoughts?

@Tyriar
Copy link
Member Author

Tyriar commented May 7, 2020

I was writing this up before I knew @zersiax posted, I'll read that now and follow up on it.


Now on the solution:

For keyboard accessibility I had the idea of there being a generic command to navigate the terminal buffer based on the link type, say workbench.action.terminal.previousLink and workbench.action.terminal.nextLink. When these commands are used without arguments, it would just navigate up and down the buffer finding any type of link, but I would also like to enable the ability of navigating based on the "link provider" type (word, web, file mentioned above). So take this keybinding as an example that could be build in:

{
    "key": "ctrl+[",
    "command": "workbench.action.terminal.previousLink",
    "when": "terminalFocus",
    "args": { "type": "web" }
}

Triggering this would navigate up to the closest link detected by the weblink provider. Meaning you could run jekyll serve, npm start, etc. and then hit ctrl+[ and enter to launch the browser. This idea will be refined, for example maybe regex would be better than link type, but that's the basic idea. While focused the hover would be visible so you would know visually what is going to happen.

On the screen reader accessibility side, at the time the user presses ctrl+[, the terminal would create an <a> tag to focus. When focused my thinking is it would read out what will happen as in the hover, something like "https://microsoft.com, follow link" or "package.json, search workspace".

You might be thinking that we also have navigation mode in the terminal and while it seems like it would be relatively easy on our side to inject the links there as well, I worry that it would be too spammy as almost everything in the terminal is a link now and it would make it difficult to both navigate to the link you're after while at the same time making it harder to read line output which is the main point of navigation mode. Perhaps this could work but just include web and file links in navigation mode?

Something also to consider is that these links will eventually be extensible by extensions (#91290) so there will be more than just web and file links appearing. Some examples: git commits/branches linking to github, localhost links opening debug browsers.

@Tyriar
Copy link
Member Author

Tyriar commented May 7, 2020

Ok it looks like my proposal mostly maps to @zersiax's second point.

That would mean that a screen reader user will have to hop into browse mode to find these, but I don't think that can be helped in this case.

I'm not sure browse mode will end up working as the terminal buffer is a virtual list and also because detecting links is an asynchronous action that could end up going to the extension host and having additional validation, meaning it could take a second in a pretty bad case for all we know.

As for the word link ... we need to do something else here. In this sccenario, the user is in the editor (for the most part) and therefore shouldn't be in browse mode.

I think I might have been misunderstood here, the word link isn't in the editor but clicking on it will open quick access or open the editor. Take this terminal output of running ls as an example:

out-test      Dockerfile           yarn.lock

There are no web or file links here as we don't attempt to match Dockerfile for example as it doesn't contain any path separators like ./, \, etc. All 3 of these however are word links which will do the following:

  • out-test: Open "Go to File..." (ctrl+p) and search "out-test", as there was no file that matched it.
  • Dockerfile:OpenDockerfile` as there was only one file matching this in the workspace.
  • yarn.lock: Open"Go to File..." (ctrl+p) and search "yarn.lock", as there were many files that matched it.

@zersiax
Copy link

zersiax commented May 7, 2020

Ok, given we can't really do anything on a per-line basis very easily at present, I think an idea to walk through the link, like proposed above by both me and @Tyriar is the best thin we can do here.
When moving to a new link, we would need to have that link announced, perhaps followed by what activating it would do exactly in a short bit of extra text. This could be toggleable, like hitting ctrl+space in autocomplete suggestions to either show just the suggestions, or it's docstring as well.
What I am a little worriedabout is how users would discover this mechanism. Screen readers currently have no idea this is a thing at all, when I saw this issue I had no idea what it was referring to initially. Any thoughts for that? :)

@Tyriar
Copy link
Member Author

Tyriar commented May 7, 2020

This could be toggleable, like hitting ctrl+space in autocomplete suggestions to either show just the suggestions, or it's docstring as well.

I hadn't thought to use ctrl+space, I was thinking maybe tab instead? For this we need to keep in mind that eventually multiple actions will be possible, based on early discussions in the UX chat the hover will probably end up having the default action like "Follow link (ctrl + click)" and then in the hover's "status bar", a button like "Change default action".

What I am a little worriedabout is how users would discover this mechanism. Screen readers currently have no idea this is a thing at all, when I saw this issue I had no idea what it was referring to initially. Any thoughts for that? :)

Discoverability is always an issue for these sorts of things, the command to navigate to web links will be super powerful but I'm sure not that many people will end up finding it, this applies to all users not just those using a screen reader. I'll probably write up a doc page about it and hope bloggers/influencers pick up on it.

We have this section in the docs which could be expanded https://code.visualstudio.com/docs/editor/accessibility#_terminal-accessibility, not sure what else we can do on the discoverability end.

@marlon-sousa
Copy link

Folks,

Being able to access these links using a screen reader would be really helpful.

I see that this issue has been closed, but don't know whether this is implemented, won't be implemented or is being implemented at all. What can I do to first discover if this is being worked out or if this is stopped at all?

As for the discussion, must screen readers would follow listings in terminal using the browse mode (JAWS) or navigation mode (NVDA), these implementations are similar, since we can arrow through the lines and read through. Making these elements clickable seems to be enough, because then screen readers would be able to send the enter key on them and click events would be generated.

Thanks for the great work!

@isidorn
Copy link
Contributor

isidorn commented Oct 19, 2020

@marlon-sousa this issue is currently assigned to Backlog, it is not closed. Which means we acknowledge the issue but no current plans to fix. @Tyriar can maybe provide more details if he has any plans to tackle this in the future

@marlon-sousa
Copy link

@Tyriar hello, I'd like to try to solve this issue.

I recognize that for a beginner on VSCode project this can be hard, but I'd like to try anyways and if that works may be submit a pr. Would you be available to provide me a basic mentoring, basically giving me typs about where in the code should I start looking at?

@Tyriar
Copy link
Member Author

Tyriar commented Oct 28, 2020

@marlon-sousa yes this is a pretty involved task, but you could have a look if you're game. It's basically about creating <a> elements for links and focusing them when navigating using this special command/keybinding. There are complications though in that the links don't exist in the DOM normally as the terminal is virtualized (the whole content isn't put in the DOM) and also the terminal typically renders using a <canvas>.

Pointers to start looking into it:

  • The class that owns all the links provider from both internal link providers and from extensions
    private _standardLinkProviders: ILinkProvider[] = [];
    . Note that a list of links is not maintained, we go to each link provider when we need to evaluate whether links are present on the line.
  • The navigation mode addon to xterm.js (the terminal component) https://github.com/microsoft/vscode/blob/master/src/vs/workbench/contrib/terminal/browser/addons/navigationModeAddon.ts, the link solution needed is similar to this in that is needs to manipulate focus on the terminal
  • How to contribute a command/keybinding
    registerAction2(class extends Action2 {
    constructor() {
    super({
    id: TERMINAL_COMMAND_ID.NAVIGATION_MODE_FOCUS_NEXT,
    title: { value: localize('workbench.action.terminal.navigationModeFocusNext', "Focus Next Line (Navigation Mode)"), original: 'Focus Next Line (Navigation Mode)' },
    f1: true,
    category,
    keybinding: {
    primary: KeyMod.CtrlCmd | KeyCode.DownArrow,
    when: ContextKeyExpr.or(
    ContextKeyExpr.and(KEYBINDING_CONTEXT_TERMINAL_A11Y_TREE_FOCUS, CONTEXT_ACCESSIBILITY_MODE_ENABLED),
    ContextKeyExpr.and(KEYBINDING_CONTEXT_TERMINAL_FOCUS, CONTEXT_ACCESSIBILITY_MODE_ENABLED)
    ),
    weight: KeybindingWeight.WorkbenchContrib
    },
    precondition: KEYBINDING_CONTEXT_TERMINAL_PROCESS_SUPPORTED
    });
    }
    run(accessor: ServicesAccessor) {
    accessor.get(ITerminalService).getActiveInstance()?.navigationMode?.focusNextLine();
    }
    });

@Tyriar
Copy link
Member Author

Tyriar commented Oct 11, 2021

Related: #69795

@Tyriar Tyriar removed this from the Backlog milestone Dec 8, 2021
@isidorn
Copy link
Contributor

isidorn commented Jan 3, 2022

@zersiax @marlon-sousa give it a try in latest insiders and let us know how it behaves. Thanks!

@Tyriar
Copy link
Member Author

Tyriar commented Jan 4, 2022

You can try it out by running the Terminal: Show ... Link Quick Pick commands. I'll re-open this to give the command names/format a polish pass, might also be good to for a UX sync discussion.

@meganrogge
Copy link
Contributor

meganrogge commented Jan 5, 2022

Seeking accessibility feedback related to terminal link keyboard navigation:

terminal link types include

  • web links, which open in the browser
  • file links, which open the file in vscode
  • word links, which are low confidence links separated by spaces. clicking these will search for files containing the word.

Option 1:
display all links, categorized by their type

have 4 different commands - one which shows all types and one for each of the types
downsides: could be confusing especially because link types aren't always known to the user or discoverable
Screen Shot 2022-01-05 at 1 43 32 PM

Option 2:
display link types (word, web, file) and upon choice, display only links of that type
Screen Shot 2022-01-05 at 3 13 31 PM

@meganrogge
Copy link
Contributor

1st pass merged with option 1.

@isidorn
Copy link
Contributor

isidorn commented Jan 6, 2022

Let me share here what I wrote in our chat already (so we have everything in one place):

I think it should all be under one list. That is the upside of quickPick that it is good at filtering and dealing with large amounts if entries.
Having 4 different commands for each type will be hard to understand, same as option 2. So I suggest just one command that shows all. You do good sorting, since the Word category is at the end.
Can we do some smart detection and automatically filter out the 1 word entries from that list?

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2022

@isidorn smart detection as in duplicate removal?

@isidorn
Copy link
Contributor

isidorn commented Jan 6, 2022

@meganrogge I just tried this out. Works nicely. Some feedback which also address @Tyriar questions:

  1. Yes we should remove duplicates
  2. Can we ignore the words on the input line? In my use case the words are filled with useless X, -> which all come from the input line, which is highly unlikely to contain a link I want to click on (example image below)
  3. "Select Detected Link" this does not really Select it. Is "Open Detected Link" a better name for the command?

@jooyoungseo I know you are a heavy terminal user. Would love to hear your thoughts. To try it out, open terminal, have some text and links, F1 > Terminal Select Detected Link

Screenshot 2022-01-06 at 15 07 47

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2022

Can we ignore the words on the input line? In my use case the words are filled with useless X, -> which all come from the input line, which is highly unlikely to contain a link I want to click on (example image below)

The trouble is defining what gets ignored, how are we to say what is not a valid link as filtering out any of them is removing access to a feature available via the mouse (you can ctrl+click anything). Perhaps filtering out these high code point symbols/emoji only is the way to go?

The input line can have valid links on it like folders/urls/files as well.

"Select Detected Link" this does not really Select it. Is "Open Detected Link" a better name for the command?

👍

@isidorn
Copy link
Contributor

isidorn commented Jan 6, 2022

Perhaps filtering out these high code point symbols/emoji only is the way to go?

Sounds like a good approach to me.

@zersiax
Copy link

zersiax commented Jan 6, 2022

I'd be inclined to agree although I don't quite know what these emoji/code points are for? As in ...what would happen if you were to click them?

@isidorn
Copy link
Contributor

isidorn commented Jan 6, 2022

The same thing as if you were to select them via the new command: we open the Quick Pick and try to find a file with that name -> so not useful.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2022

@zersiax yeah nothing useful, unless you have a file whose name contains one of the symbols.

@zersiax
Copy link

zersiax commented Jan 6, 2022

I'd say filtering those out is a good strategy in that case

@vistuleB
Copy link

vistuleB commented Jan 15, 2022

I was interested in this feature and I dowloaded Code Insiders to take a look.

In my case I'm interested in keyboard navigation of crash stack traces, such as:

Screen Shot 2022-01-15 at 12 23 13 PM

It would be great to have commands "terminal: open previous link" and "terminal: open next link". When there's no history or when a command has just been run in the terminal, "open previous link" would open the first link found from the bottom of the terminal, namely the one that is already selected by default here:

Screen Shot 2022-01-15 at 12 27 39 PM

From there, running "terminal: open previous link" over and over again would walk down the list of links as they appear in the above list; until, that is, some command occurs in the terminal, that would erase the history of link visitations. (And "terminal: open next link" walks back up the same list, but which is downward inside the terminal window.)

@Tyriar
Copy link
Member Author

Tyriar commented Jan 18, 2022

Thanks @vistuleB, it was my original plan to have a command that opens the most recent link of a particular type, but I didn't consider your idea of keeping track of that last link to open multiple in the editor in quick succession. Tracking in #140923

@meganrogge meganrogge added the on-release-notes Issue/pull request mentioned in release notes label Jan 28, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan terminal Integrated terminal issues terminal-links
Projects
None yet
8 participants
@isidorn @Tyriar @hediet @zersiax @vistuleB @marlon-sousa @meganrogge and others