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

Conditionally propagate web events using a filter in WebOptions #5056

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

liamrosenfeld
Copy link
Contributor

@liamrosenfeld liamrosenfeld commented Sep 1, 2024

Currently egui will prevent all web events from propagating. This causes issues in contexts where you are using egui in a larger web context that wants to receive events that egui does not directly respond to. For example, currently using egui in a VSCode extension will block all app hotkeys, such as saving and opening the panel.

This adds a closure to WebOptions that takes in a reference to the egui event that is generated from a web event and returns if the corresponding web event should be propagated or not. The default for it is to always return false.

Alternatives I considered were:

  1. Having the propagation filter be a property of the focus in memory. That way it could be configured by the view currently selected. I opted away from that because I wanted to avoid lowering eframe implementation specific stuff into egui.
  2. Having events contain a web_propagate flag that could be set when handling them. However, that would not be compatible with the current system of egui events being handled outside of the web event handler.

I just recently started using egui so I am not sure how idiomatic my approach here is. I would be happy to switch this over to a different architecture if there are suggestions.

  • I have followed the instructions in the PR template

Currently egui will prevent all web events from propagating. This causes issues in contexts where you are using egui embedded in a larger context that wants to receive events that egui does not directly respond to. For example, currently using egui in a VSCode extension will block all app hotkeys, such as saving and opening the panel.

This adds a closure to `WebConfig` that takes in a reference to the egui event that is generated from a web event and returns if the corresponding web event should be propagated or not. The default for it is to always return false.

Alternatives I considered were:
1) Having the propagation filter be a property of the focus in memory. That way it could be configured by the view currently selected. I opted away from that because I wanted to avoid lowering eframe implementation specific stuff into egui.
2) Having events contain a `web_propagate` flag that could be set when handling them. However, that would not be compatible with the current system of egui events being handled outside of the web event handler.
crates/eframe/src/web/events.rs Outdated Show resolved Hide resolved
crates/eframe/src/epi.rs Show resolved Hide resolved
@emilk emilk added feature New feature or request web Related to running Egui on the web labels Sep 2, 2024
@liamrosenfeld
Copy link
Contributor Author

I updated the comments. I also added the same comments to other occurrences of the check for consistency.

@emilk emilk added the eframe Relates to epi and eframe label Sep 3, 2024
@emilk emilk merged commit df9cd21 into emilk:master Sep 5, 2024
20 of 21 checks passed
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…k#5056)

Currently egui will prevent all web events from propagating. This causes
issues in contexts where you are using egui in a larger web context that
wants to receive events that egui does not directly respond to. For
example, currently using egui in a VSCode extension will block all app
hotkeys, such as saving and opening the panel.

This adds a closure to `WebOptions` that takes in a reference to the
egui event that is generated from a web event and returns if the
corresponding web event should be propagated or not. The default for it
is to always return false.

Alternatives I considered were:
1. Having the propagation filter be a property of the focus in memory.
That way it could be configured by the view currently selected. I opted
away from that because I wanted to avoid lowering eframe implementation
specific stuff into egui.
2. Having events contain a `web_propagate` flag that could be set when
handling them. However, that would not be compatible with the current
system of egui events being handled outside of the web event handler.

I just recently started using egui so I am not sure how idiomatic my
approach here is. I would be happy to switch this over to a different
architecture if there are suggestions.

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

* [x] I have followed the instructions in the PR template
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 feature New feature or request web Related to running Egui on the web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants