-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: allow customization of prevent_default
behavior
#5779
base: master
Are you sure you want to change the base?
Conversation
crates/eframe/src/epi.rs
Outdated
/// [`stopPropagation`](https://developer.mozilla.org/en-US/docs/Web/API/Event/stopPropagation) | ||
/// is called on every event. | ||
pub should_propagate_event: Box<dyn Fn(&egui::Event) -> bool>, | ||
|
||
/// Whether the web event corresponding to an egui event should have `prevent_default` called | ||
/// on it or not. | ||
/// | ||
/// Defaults to true. | ||
pub should_prevent_default: Box<dyn Fn(&egui::Event) -> bool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit weird that one is negated and the other isn't. Since should_do_default
or should_not_prevent_default
is weird, maybe we should rename should_propagate_event
to should_stop_propagation
which would also be consistent with the name of the js function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree - rename one or the other please!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I was a bit busy and then forgot about this PR. Added a commit renaming should_propagate_event
to should_stop_propagation
.
Preview available at https://egui-pr-preview.github.io/pr/5779-propagate-events |
For consistency with the newly introduced `should_prevent_default`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
Currently eframe calls
prevent_default()
for all copy / paste events on the document, making embedding an egui application in a page (e.g. an react application) hard (as all copy & paste functionality for other elements on the page is broken by this).I'm not sure what the motivation for this is, if any.
This commit / PR adds a callback (
should_prevent_default
), similar toshould_propgate_event
, that an egui application can use to overwrite this behavior. It defaults to returningtrue
for all events, to keep the existing behavior.I call
should_prevent_default
in every place thatshould_propagate_event
is called (which is not all places thatprevent_default
is called!). I'm not sure for the motivation of not callingshould_propagate_event
everywhere thatstop_propagation
is called, but I kept that behavior for theshould_prevent_default
callback too.Please let me know if I'm missing some existing functionality that would allow me to do this, or if there's a reason that we don't want applications to be able to customize this (i.e. if there's a reason to always
prevent_default
for all copy / paste events on the whole document)