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: only capture clicks/touches when actually over canvas #4775

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

lucasmerlin
Copy link
Collaborator

To test, start the web demo and follow the repro steps from #4752

@lucasmerlin lucasmerlin force-pushed the check-touch-above-canvas branch from ebc5f0f to 5769491 Compare July 3, 2024 17:04
@lucasmerlin lucasmerlin force-pushed the check-touch-above-canvas branch from 5769491 to 8a9619a Compare July 3, 2024 17:13
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Have you tested this with scrolling, to make sure we're not confusing coordinate systems? (page_x vs client_x)

@emilk emilk added bug Something is broken web Related to running Egui on the web eframe Relates to epi and eframe labels Jul 4, 2024
@lucasmerlin
Copy link
Collaborator Author

lucasmerlin commented Jul 4, 2024

Have you tested this with scrolling, to make sure we're not confusing coordinate systems? (page_x vs client_x)

Seems to work as expected:

Bildschirmaufnahme.2024-07-04.um.11.45.07.mov

I also checked the clientX/Y and elementFromPoint documentation which both state that they take coordinates relative to the viewport.

@emilk emilk added this to the Next Patch Release milestone Jul 4, 2024
Comment on lines 426 to 434
window()
.unwrap()
.document()
.unwrap()
.element_from_point(pos.x, pos.y)
.is_some_and(|element| element.eq(runner.canvas()))
|| runner
.egui_ctx()
.input(|i| i.pointer.any_down() || i.any_touches())
Copy link
Owner

Choose a reason for hiding this comment

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

This is very difficult to parse. Naming intermediates helps

Suggested change
window()
.unwrap()
.document()
.unwrap()
.element_from_point(pos.x, pos.y)
.is_some_and(|element| element.eq(runner.canvas()))
|| runner
.egui_ctx()
.input(|i| i.pointer.any_down() || i.any_touches())
let document = web_sys::window().unwrap().document().unwrap();
let hovering_canvas = document
.element_from_point(pos.x, pos.y)
.is_some_and(|element| element.eq(runner.canvas()));
let pointer_down = runner
.egui_ctx()
.input(|i| i.pointer.any_down() || i.any_touches())
is_hovering_canvas || pointer_down

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Much better!

@emilk emilk merged commit 9a4c462 into emilk:master Jul 5, 2024
19 checks passed
@emilk emilk changed the title Check if the mouse / touch event is above the canvas via element_from_point eframe web: only capture clicks/touches when actually over canvas Jul 5, 2024
@emilk emilk mentioned this pull request Jul 5, 2024
5 tasks
emilk added a commit to rerun-io/rerun that referenced this pull request Jul 5, 2024
## What
* Closes #5315

## egui changelog

### ⭐ Added
* Add `Image::uri()` [#4720](emilk/egui#4720) by
[@rustbasic](https://github.com/rustbasic)

### 🔧 Changed
* Better documentation for `Event::Zoom`
[#4778](emilk/egui#4778) by
[@emilk](https://github.com/emilk)
* Hide tooltips when scrolling
[#4784](emilk/egui#4784) by
[@emilk](https://github.com/emilk)
* Smoother animations [#4787](emilk/egui#4787)
by [@emilk](https://github.com/emilk)
* Hide tooltip on click [#4789](emilk/egui#4789)
by [@emilk](https://github.com/emilk)

### 🐛 Fixed
* Fix default height of top/bottom panels
[#4779](emilk/egui#4779) by
[@emilk](https://github.com/emilk)
* Show the innermost debug rectangle when pressing all modifier keys
[#4782](emilk/egui#4782) by
[@emilk](https://github.com/emilk)
* Fix occasional flickering of pointer-tooltips
[#4788](emilk/egui#4788) by
[@emilk](https://github.com/emilk)

## eframe changelog
* Web: only capture clicks/touches when actually over canvas
[#4775](emilk/egui#4775) by
[@lucasmerlin](https://github.com/lucasmerlin)

## 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/6785?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/6785?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/6785)
- [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`.
emilk added a commit to rerun-io/rerun that referenced this pull request Jul 5, 2024
## What
* Closes #5315

## egui changelog

### ⭐ Added
* Add `Image::uri()` [#4720](emilk/egui#4720) by
[@rustbasic](https://github.com/rustbasic)

### 🔧 Changed
* Better documentation for `Event::Zoom`
[#4778](emilk/egui#4778) by
[@emilk](https://github.com/emilk)
* Hide tooltips when scrolling
[#4784](emilk/egui#4784) by
[@emilk](https://github.com/emilk)
* Smoother animations [#4787](emilk/egui#4787)
by [@emilk](https://github.com/emilk)
* Hide tooltip on click [#4789](emilk/egui#4789)
by [@emilk](https://github.com/emilk)

### 🐛 Fixed
* Fix default height of top/bottom panels
[#4779](emilk/egui#4779) by
[@emilk](https://github.com/emilk)
* Show the innermost debug rectangle when pressing all modifier keys
[#4782](emilk/egui#4782) by
[@emilk](https://github.com/emilk)
* Fix occasional flickering of pointer-tooltips
[#4788](emilk/egui#4788) by
[@emilk](https://github.com/emilk)

## eframe changelog
* Web: only capture clicks/touches when actually over canvas
[#4775](emilk/egui#4775) by
[@lucasmerlin](https://github.com/lucasmerlin)

## 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/6785?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/6785?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/6785)
- [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`.
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…_point (emilk#4775)

* Closes emilk#4752 

To test, start the web demo and follow the repro steps from emilk#4752
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken 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.

touchend listener on document prevents listening for clicks on dom elements (web)
2 participants