-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add pending event listener on the VTag #2300
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ use std::borrow::Cow; | |
use std::cmp::PartialEq; | ||
use std::hint::unreachable_unchecked; | ||
use std::marker::PhantomData; | ||
use std::mem; | ||
use std::ops::Deref; | ||
use std::rc::Rc; | ||
use wasm_bindgen::JsCast; | ||
|
@@ -430,8 +431,28 @@ impl VTag { | |
.insert(key, value.into_prop_value()); | ||
} | ||
|
||
/// Set event listeners on the [VTag]'s [Element] | ||
/// Add pending event listener on the [VTag]'s [Element] | ||
pub fn add_pending_listener(&mut self, listener: Rc<dyn Listener>) -> bool { | ||
if let Listeners::Pending(listeners) = &mut self.listeners { | ||
let mut listeners = mem::take(listeners).into_vec(); | ||
listeners.push(Some(listener)); | ||
|
||
self.set_listeners(listeners.into_boxed_slice()); | ||
true | ||
} else { | ||
false | ||
voidpumpkin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
#[deprecated( | ||
note = "The `set_listener` method is deprecated and will be removed in the future. Use the `set_listeners` method instead." | ||
)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just remove this function instead of deprecating this. Breaking changes are fine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hamza1311 Ok, I removed this function. |
||
pub fn set_listener(&mut self, listeners: Box<[Option<Rc<dyn Listener>>]>) { | ||
self.set_listeners(listeners) | ||
} | ||
|
||
/// Set event listeners on the [VTag]'s [Element] | ||
pub fn set_listeners(&mut self, listeners: Box<[Option<Rc<dyn Listener>>]>) { | ||
self.listeners = Listeners::Pending(listeners); | ||
} | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can you add a little note about what a pending listener is here? It isn't clear what pending listener is from just looking at this method.
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.
Yeah what is a "pending event listener"?
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.
A pending listener is a
Listener
instance pending for registration in the global event listener multiplexer. That's an internal implementation detail, that need not be exposed. This function is better namedadd_listener()
and the other public function can beset_listeners()
, as you have commited.set_listener()
is a misnomer and not needed. I think it got in here via improper merge conflict resolution on my part.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 still have not received an answer anywhere why would the average yew user need this?To my understanding it just skips thehtml!
macro to add a listener?Cant users that dislike that macro use something likeyew-vdom-gen
instead of hacking through yew internal api? (I get the irony thatyew-vdom-gen
uses same internal api)EDIT: nevermind
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.
@bakape @voidpumpkin I renamed this function to
add_listener
.