-
Notifications
You must be signed in to change notification settings - Fork 83
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
Extend EventTarget to support event bubbling #84
Conversation
- add getParent method to Node returning parentNode - on dispatch trigger parent dispatchEvent - unit tests
Couple of changes needed, and I'd love to have |
Thanks! |
Oh ... I see. Sorry, busy days in here ... I'll try to update/change that later on but if you are, somehow, in a rush, please go ahead with the PR (ungap is a bit weird to work with though, no modern JS and stuff) ... as you prefer. |
- method renamed to "_getParent" - dispatchEvent return value now take EventTarget.dispatchEvent return into account as well as parent.dispatchEvent - "stopImmediatePropagation" now tested in unit tests. Todo: when ungap/event-target support "_stopImmediatePropagationFlag" more event listeners could be added to `buttonTarget` to ensure full behaviour is working.
No worries, I didn't mean to rush you :) |
used "protected" here instead of "private", because Node extend EventTarget and needs to overwrite "_getParent" while still allowing "dispatchEvent" to access it.
- Node 16.5 will throw if we try to dispatch the current event to the parent. So instead we need to create a new event with the same options. - replaced "_stopPropagationFlag" by "cancelBubble" to be consistent with Node implementation of Event - currently Node don't handle bubbling, but also don't properly set "_stopPropagationFlag" when calling "stopImmediatePropagation" creating a problem and inconsistency for us. => should we extend the Node Event object to ensure the proper behaviour?
So ... what are we missing here? is that still ungap for the event target? Thanks! |
yes, I can make a PR for that next, it should be way simpler. |
- add getParent method to Node returning parentNode - on dispatch trigger parent dispatchEvent - unit tests
- method renamed to "_getParent" - dispatchEvent return value now take EventTarget.dispatchEvent return into account as well as parent.dispatchEvent - "stopImmediatePropagation" now tested in unit tests. Todo: when ungap/event-target support "_stopImmediatePropagationFlag" more event listeners could be added to `buttonTarget` to ensure full behaviour is working.
used "protected" here instead of "private", because Node extend EventTarget and needs to overwrite "_getParent" while still allowing "dispatchEvent" to access it.
- Node 16.5 will throw if we try to dispatch the current event to the parent. So instead we need to create a new event with the same options. - replaced "_stopPropagationFlag" by "cancelBubble" to be consistent with Node implementation of Event - currently Node don't handle bubbling, but also don't properly set "_stopPropagationFlag" when calling "stopImmediatePropagation" creating a problem and inconsistency for us. => should we extend the Node Event object to ensure the proper behaviour?
@mikemadest this looks good to me, but I don't understand why there are unrelated changes in this MR ... by any chance you can remove those and pull rebase from main instead? git pull --rebase origin main I'd rather keep changes per PR/MR, thanks. Once you do that, I'll squash merge and publish. |
Sorry I'm not sure what happened there, will cleanup |
- add getParent method to Node returning parentNode - on dispatch trigger parent dispatchEvent - unit tests
* Adding NamedNodeMap to global export
- method renamed to "_getParent" - dispatchEvent return value now take EventTarget.dispatchEvent return into account as well as parent.dispatchEvent - "stopImmediatePropagation" now tested in unit tests. Todo: when ungap/event-target support "_stopImmediatePropagationFlag" more event listeners could be added to `buttonTarget` to ensure full behaviour is working.
used "protected" here instead of "private", because Node extend EventTarget and needs to overwrite "_getParent" while still allowing "dispatchEvent" to access it.
- Node 16.5 will throw if we try to dispatch the current event to the parent. So instead we need to create a new event with the same options. - replaced "_stopPropagationFlag" by "cancelBubble" to be consistent with Node implementation of Event - currently Node don't handle bubbling, but also don't properly set "_stopPropagationFlag" when calling "stopImmediatePropagation" creating a problem and inconsistency for us. => should we extend the Node Event object to ensure the proper behaviour?
- add getParent method to Node returning parentNode - on dispatch trigger parent dispatchEvent - unit tests
- method renamed to "_getParent" - dispatchEvent return value now take EventTarget.dispatchEvent return into account as well as parent.dispatchEvent - "stopImmediatePropagation" now tested in unit tests. Todo: when ungap/event-target support "_stopImmediatePropagationFlag" more event listeners could be added to `buttonTarget` to ensure full behaviour is working.
used "protected" here instead of "private", because Node extend EventTarget and needs to overwrite "_getParent" while still allowing "dispatchEvent" to access it.
- Node 16.5 will throw if we try to dispatch the current event to the parent. So instead we need to create a new event with the same options. - replaced "_stopPropagationFlag" by "cancelBubble" to be consistent with Node implementation of Event - currently Node don't handle bubbling, but also don't properly set "_stopPropagationFlag" when calling "stopImmediatePropagation" creating a problem and inconsistency for us. => should we extend the Node Event object to ensure the proper behaviour?
… into event-bubbling
ok so I was just behind main because of other merge, this looks all good now |
Thanks a lot for your help ❤️ |
thank you for bearing with me :) |
This solves: ungap/event-target#8
As discussed there, EventTarget is extended to add bubbling and parent node knowledge only in the DOM context.