Skip to content

Conversation

weaverryan
Copy link
Member

Q A
Bug fix? yes
New feature? no
Tickets Fix #30
License MIT

Long-overdue fix for #30. This is a small behavior change, but I consider it a bug fix. The only way that you could be affected by this change in practice is if you have one of the UX components listed inside of itself - e.g. a chart inside a chart, so now the existing listener on the outer chart is fired twice. But I don't think that makes any practical sense for any of these components.

Cheers!

@norkunas
Copy link
Contributor

norkunas commented Dec 9, 2022

Just interested: why not using dispatchEvent method that exists in stimulus controllers and implementing a custom method for this? :)

@kbond kbond changed the title Making all events bundle - affects Chartjs, Cropperjs, Dropzone, LazyImage, Swup Making all events bubble - affects Chartjs, Cropperjs, Dropzone, LazyImage, Swup Dec 9, 2022
@weaverryan
Copy link
Member Author

Lol, good question. Two reasons: 1) Until about a week ago I didn't know this existed! And, related, (2), this dispatch() method was added in Turbo 3, so many of the packages predate that. But, indeed, it looks like we can start using it now by passing false for the prefix option. Good call

@weaverryan
Copy link
Member Author

This is now ready for a quick look-over - all of the new ways to dispatch the events should result in the same event name & target as before, unless I've done something silly or copy-pasta'ed something poorly.

@weaverryan weaverryan merged commit cb54eae into symfony:2.x Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow events to bubble
2 participants