jupyterlab / lumino

Lumino is a library for building interactive web applications
https://lumino.readthedocs.io/
Other
635 stars 126 forks source link

elements respond to enter or spacebar #590

Closed g547315 closed 1 year ago

g547315 commented 1 year ago

References https://github.com/jupyterlab/jupyterlab/issues/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[bot] commented 1 year ago

Thanks for submitting your first pull request! You are awesome! :hugs:
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! :wave:
Welcome to the Jupyter community! :tada:

g547315 commented 1 year ago

Addressing the failing test: test:firefox-headless

As for Enforce PR label / enforce-label

g547315 commented 1 year ago

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 commented 1 year ago

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

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

fcollonval commented 1 year ago

@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 commented 1 year ago

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:

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

welcome[bot] commented 1 year ago

Congrats on your first merged pull request in this project! :tada: congrats Thank you for contributing, we are very proud of you! :heart: