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

eframe web: ignore keyboard events unless canvas has focus #4718

Merged
merged 19 commits into from
Jun 28, 2024

Conversation

emilk
Copy link
Owner

@emilk emilk commented Jun 27, 2024

This improves how an eframe canvas works inside of a larger web page, and how it works when there are multiple eframe apps in the same page.

eframe will set tabindex="0" on the canvas automatically, making it focusable.
It will also set outline: none on the CSS, so the focused canvas won't have an ugly outline.

Breaking changes

You may wanna add this to your index.html to give the canvas focus on startup:

document.getElementById("the_canvas_id").focus();

Test setup

./scripts/build_demo_web.sh
./scripts/start_server.sh
open http://localhost:8888/multiple_apps.html

Then open the "Input Event History" and "Text Edit" windows

Tested

  • Chromium
    • drag-and-drop of files
    • Test both when a TextEdit is focused and when it is not:
      • Event::Key
      • Event::Text
      • copy-cut-paste
      • Wheel scroll
    • Event::PointerGone
    • Mouse drag
    • Mouse click
    • Mouse right-click
    • Defocus all eframe canvas, and then start typing text
  • Firefox (all of the above)
  • Desktop Safari (all of the above)
  • Mobile Safari

Future work (pre-existing issues)

@emilk emilk added web Related to running Egui on the web eframe Relates to epi and eframe labels Jun 27, 2024
@emilk emilk force-pushed the emilk/only-get-canvas-events-on-focus branch from 87ec7c8 to c6980af Compare June 27, 2024 14:58
@emilk emilk force-pushed the emilk/only-get-canvas-events-on-focus branch from c6980af to 62c8995 Compare June 27, 2024 15:07
@jleibs
Copy link
Contributor

jleibs commented Jun 27, 2024

This solves most of rerun-io/rerun#6638

In a notebook context, Rerun no longer gets events when typing in notebook cells.

However, the outer notebook still also receives keyboard events, even if they are supposed to be handled by Rerun and it's in focus.

rerun-io/rerun#6674

@emilk emilk marked this pull request as ready for review June 28, 2024 06:48
@jprochazk
Copy link
Collaborator

Right click does not work in a notebook context (both classic and lab). For example, right click on a view in the blueprint panel should bring up its context menu, but instead it always opens the notebook's context menu

@emilk
Copy link
Owner Author

emilk commented Jun 28, 2024

@jprochazk please see if the latest commit (616fa2b) fixes it.

Otherwise this might be another case of Jupyter Notebook Classic install event handlers on document (rerun-io/rerun#6638 (comment))

@jprochazk
Copy link
Collaborator

I can confirm it's fixed with that commit.

Copy link
Collaborator

@jprochazk jprochazk left a comment

Choose a reason for hiding this comment

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

LGTM

@emilk emilk changed the title eframe web: Only forward keyboard events if canvas has focus eframe web: ignore keyboard events unless canvas has focus Jun 28, 2024
@emilk emilk merged commit 3b9f964 into master Jun 28, 2024
35 checks passed
@emilk emilk deleted the emilk/only-get-canvas-events-on-focus branch June 28, 2024 13:01
emilk added a commit to rerun-io/rerun that referenced this pull request Jun 28, 2024
### What
* Closes #6638
* Uses emilk/egui#4718

Instructions:
```
pixi run -e examples py-build-notebook
pixi run -e examples jupyter notebook examples/python/notebook/cube.ipynb
```
Run the first few 3 cells.

### Known issues:
- Hitting 'a' when the viewer selected still bypasses Rerun and adds a
new preceding cell in the notebook. This is because of how Jupyter
Notebook Classic does things.


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6674?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6674?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6674)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

---------

Co-authored-by: Emil Ernerfeldt <[email protected]>
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
* Fixes rerun-io/rerun#6638
* Related? emilk#4563

This improves how an eframe canvas works inside of a larger web page,
and how it works when there are multiple eframe apps in the same page.

`eframe` will set `tabindex="0"` on the canvas automatically, making it
focusable.
It will also set `outline: none` on the CSS, so the focused canvas won't
have an ugly outline.


## Breaking changes
You may wanna add this to your `index.html` to give the canvas focus on
startup:
```js
document.getElementById("the_canvas_id").focus();
```

## Test setup
```sh
./scripts/build_demo_web.sh
./scripts/start_server.sh
open http://localhost:8888/multiple_apps.html
```
Then open the "Input Event History" and "Text Edit" windows

## Tested
* Chromium
	* [x] drag-and-drop of files
	* Test both when a `TextEdit` is focused and when it is not:
	  * [x] `Event::Key`
	  * [x] `Event::Text`
	  * [x] copy-cut-paste
	  * [x] Wheel scroll
	* [x] `Event::PointerGone`
	* [x] Mouse drag
	* [x] Mouse click
	* [x] Mouse right-click
	* [x] Defocus all eframe canvas, and then start typing text
* [x] Firefox (all of the above)
* [x] Desktop Safari (all of the above)
* [x] Mobile Safari


## Future work (pre-existing issues)
* emilk#4723
* emilk#4724
* emilk#4725
* emilk#4726
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe web Related to running Egui on the web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rerun notebook widget eavesdrops on keyboard events while typing in another cell
3 participants