Prototype improved a11y buffer view#4340
Merged
meganrogge merged 22 commits intoxtermjs:masterfrom Jan 18, 2023
Merged
Conversation
meganrogge
reviewed
Jan 18, 2023
meganrogge
reviewed
Jan 18, 2023
meganrogge
reviewed
Jan 18, 2023
meganrogge
reviewed
Jan 18, 2023
meganrogge
reviewed
Jan 18, 2023
meganrogge
reviewed
Jan 18, 2023
Tyriar
commented
Jan 18, 2023
Member
Author
Tyriar
left a comment
There was a problem hiding this comment.
Some lint issues:
/home/vsts/work/1/s/src/browser/AccessibilityManager.ts
333:50 warning Strings must use singlequote @typescript-eslint/quotes
/home/vsts/work/1/s/src/browser/TestUtils.test.ts
143:110 warning Expected a comma @typescript-eslint/member-delimiter-style
143:151 warning Unexpected separator (;) @typescript-eslint/member-delimiter-style
316:110 warning Expected a comma @typescript-eslint/member-delimiter-style
316:151 warning Unexpected separator (;) @typescript-eslint/member-delimiter-style
/home/vsts/work/1/s/src/browser/Types.d.ts
147:91 warning Expected a comma @typescript-eslint/member-delimiter-style
/home/vsts/work/1/s/src/browser/Viewport.ts
283:98 warning Expected a comma @typescript-eslint/member-delimiter-style
✖ 7 problems (0 errors, 7 warnings)
0 errors and 7 warnings potentially fixable with the `--fix` option.
Tyriar
commented
Jan 18, 2023
This was referenced Jan 18, 2023
Member
|
fixed the lint issue, will add that |
|
Hi, I wonder what is the current status of this? It seems that on ToT, the old a11y tree based on list elements is gone, but the new thing (based on contexteditable element?) is also not ready? I am not able to use the screen reader on the terminal any more. |
Member
Author
|
@JasonXJ the situation is vscode should be much better now, for other xterm.js embedders we also brought back the old a11y tree. |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I got some feedback that the current navigation mode isn't very intuitive. This explores adding a different element, accessible via shift+tab (which will by default skip the terminal) that when focused fills a contenteditable element with the content of the buffer. This allows easy traversal using a screen reader and also selection.
An issue I'm currently hitting is when this element is created, NVDA can freeze for 15-60 seconds I'm guessing to update its model of the DOM. I tried caching the scrollback elements but that actually ends up worse when running
treeas it doesn't do any debouncing, even if optimized it will probably add a noticeable delay between output and announcing. Capping the output to 100 is a workaround for this, it's not ideal though. A monaco instance may be a way to fix this problem, at least within VS Code.I also thought we could leverage shell integration here in VS Code by exposing some delegate on the API that takes care of constructing the DOM elements, that way we could logically split up the buffer into command/output.
cc @meganrogge