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

Turn on accessibility in xterm.js #6359

Merged
merged 2 commits into from May 15, 2019
Merged

Turn on accessibility in xterm.js #6359

merged 2 commits into from May 15, 2019

Conversation

@yuvipanda
Copy link
Contributor

@yuvipanda yuvipanda commented May 15, 2019

This makes it work with screen readers much much better.

See xtermjs/xterm.js#1182 for more
information.

This adds extra DOM nodes that are live updated for each line
in the terminal, but I think it should be ok given the other
DOM nodes we have :)

From http://www.w4a.info/2019/hackathon/

This makes it work with screen readers much much better.

See xtermjs/xterm.js#1182 for more
information
@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented May 15, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 15, 2019

Thanks @yuvipanda! Is there any reason that we might want to let this option be configurable? Are there any performance or browser compatibility issues? If so, we may want to add this to the ITerminal.IOptions interface. Otherwise, I think this can go in as is.

@yuvipanda
Copy link
Contributor Author

@yuvipanda yuvipanda commented May 15, 2019

@ian-r-rose from reading the PR, there's no reason (other than if you're worried about the DOM node overhead) for it to be configurable at the JupyterLab user level. Adding it to ITerminal.IOptions (with default to True) might make sense so other components can turn it off if needed. I'll do that now.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 15, 2019

Yeah, I agree, there shouldn't need to be much reason for this to be configurable at the user level. Might be worthwhile at the component level.

@yuvipanda
Copy link
Contributor Author

@yuvipanda yuvipanda commented May 15, 2019

@ian-r-rose done.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented May 15, 2019

Awesome, thanks @yuvipanda!

@ian-r-rose ian-r-rose merged commit 1b7b564 into jupyterlab:master May 15, 2019
9 checks passed
@yuvipanda yuvipanda deleted the xterm-screenreader branch May 15, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants