search

jupyterlab-contrib / rise

RISE: "Live" Reveal.js JupyterLab Slideshow Extension
BSD 3-Clause "New" or "Revised" License
157 stars 16 forks source link

Fix keys being captured when in edit mode #58

Closed briank-git closed 7 months ago

briank-git commented 8 months ago

Hello,

This PR fixes #33.

What I found was that shortcuts listed in packages/lab/schema/plugin.json have selectors that target .lm-Widget.reveal which doesn't account for whether or not a slide is in edit or command mode, meaning the keys are captured even when the cell is being actively edited.

The selectors now target .lm-Widget.slides.jp-mod-commandMode which only exists when the slide is in command mode.

I have tested this on JupyterLab 4.0.5 and keys (= , s t ...) are no longer captured in edit mode and are only usable in command mode.

This might affect the 3.x branch and could be worth investigating, as when I tested with 0.3.0 and JupyterLab 3.6.6 the same issue was present.

Thank you.

github-actions[bot] commented 8 months ago

Binder :point_left: Launch a Binder on branch briank-git/rise/key-binding-fix

brichet commented 7 months ago

@briank-git thanks for opening this.

Indeed the keys are no longer captured in edit mode, but I'm also not able to run the shortcuts in command mode.

Is that the same on your side ?

briank-git commented 7 months ago

Thanks for taking a look @brichet

I'm able to invoke the shortcuts, but have to either click a cell's output or a markdown cell once to get the right element in focus. Essentially the cursor has to have clicked part of a cell or cell output without entering edit mode.

When I click outside of a cell or cell output, it seems the focus is on an element with class slide-background-content which does not have any elements with classes .lm-Widget.slides.jp-mod-commandMode as an ancestor or descendant. As a result the shortcuts won't function.

I'll have to see if there's a way to bring the correct element forward or find a different selector.

brichet commented 7 months ago

@briank-git, before seeing your PR, I was about to open a PR with this changes, which should fix both: ignoring the shortcut in edit mode and focusing properly on the notebook widget in command mode. Feel free to pickup anything there.

briank-git commented 7 months ago

@brichet That's great, your changes fix both issues! I cherry-picked it and tested on my branch.

I would just be overwriting my own changes with yours, so feel free to make your own PR and I will just close this one.

Thanks!

brichet commented 7 months ago

OK, I didn't want to supersede your PR as you were first working on this.