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

elements respond to enter or spacebar #590

Merged

Conversation

g547315
Copy link
Contributor

@g547315 g547315 commented May 30, 2023

References
jupyterlab/jupyterlab#9686

Code changes
Hard part of #9686: added functionality so elements now respond to "enters" or "spaces"on a keyboard like a link or button .

@krassowski Please review and comment

@welcome
Copy link

welcome bot commented May 30, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@g547315
Copy link
Contributor Author

g547315 commented May 30, 2023

Addressing the failing test: test:firefox-headless

  • The changes do not affect this test as it fails with or without the changes
  • Error Message: 30 05 2023 14:27:53.287:ERROR [launcher]: Cannot start FirefoxHeadless XPCOMGlueLoad error for file /home/xxx-xxxx/.cache/ms-playwright/firefox-1369/firefox/libmozgtk.so: libgtk-3.so.0: cannot open shared object file: No such file or directory Couldn't load XPCOM.

As for Enforce PR label / enforce-label

  • Need a maintainer to label the PR (don't have permissions )

@krassowski krassowski added enhancement New feature or request accessibility Addresses accessibility needs labels May 31, 2023
@g547315
Copy link
Contributor Author

g547315 commented Jun 28, 2023

Code changes
Removed synthetic events creation
Added helper method to find all adjacent child nodes
Added helper method to find index of the focusedElement from adjacent child nodes
Added functionality to update current Index

User-facing changes
Left, right and docker panel tab bar now respond space bar and enter

Backwards-incompatible changes
None

@g547315
Copy link
Contributor Author

g547315 commented Jun 28, 2023

To fix the "Does PR have API changes" test I ran the yarn run api and got this error

  • ERROR: Error parsing ...lumino/packages/widgets/api-extractor.json: The "mainEntryPointFilePath" path does not exist: ...lumino/packages/widgets/types/index.d.ts

  • I manually added the changes the command should have but the test still fails. I think it just needs someone to run the command as I am unable to

Addressing the failing test: Tests / JS (macos-latest, webkit-headless)

  • The Error: WebkitHeadless stderr: /home/ec2-user/.cache/ms-playwright/webkit-1751/minibrowser-wpe/bin/MiniBrowser: error while loading shared libraries: libgstreamer-1.0.so.0: cannot open shared object file: No such file or directory

  • The changes do not affect this test as it fails with or without the changes

g547315 and others added 3 commits July 12, 2023 13:30
Don't handle keydown dynamically.
Undo removal of listeners as they are use at capturing phase
@fcollonval
Copy link
Member

@g547315 thanks for pushing this. I took the liberty to push some simplification to push this PR through the finish line. Would you mind having a look to my changes?

@brichet
Copy link
Contributor

brichet commented Jul 19, 2023

Thanks @g547315 for working on this, this look good to me.

(Probably for a follow up PR)
I was thinking about the same issue before seeing this PR, and I'm wondering if having the tabindex="0" on each tab is the right approach.
It should be better to switch from one tab to another using the arrow keys instead of using tabulation.
If you have a lot of tabs opened, it can be very annoying to move out of the tabbar.

Maybe a better solution could be:

  • to have the tabindex="0" only to the selected tab, and tabindex="-1" on the other ones
  • to catch the left/right/top/down arrow to switch the selected tabs (and the tabindex values)

Like that, using the tabulation will move the focus on another widget than the tabbar.

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brichet has time to improving further keyboard navigation on tabs. So I'll merge this to allow him to built on top of it.

@fcollonval fcollonval merged commit af2ae00 into jupyterlab:main Jul 19, 2023
@welcome
Copy link

welcome bot commented Jul 19, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Addresses accessibility needs enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants