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

Replace RCTBubblingEventBlock by RCTDirectEventBlock to avoid event name collision with Expo AV #1625

Merged

Conversation

CHaNGeTe
Copy link
Contributor

Update the changelog

Pending, will do tomorrow.

Provide an example of how to test the change

Test case with expo is reported here: #1618

Describe the changes

Replaces RCTBubblingEventBlock for RCTDirectEventBlock.
As bubbling events are recommended for DOM-like events (scroll, focus, ...) that need to propagate.
This avoids collision on event names with the expo av library. As DeviceEventEmitter is shared among all libraries.

Ref: https://gist.github.com/chourobin/f83f3b3a6fd2053fad29fff69524f91c#ios
Notes: Bubbling events are like DOM events so that a parent component can capture an event fired by its child. Generally these are UI-related, like "the user touched this box". Direct events are not bubbled and are intended for more abstract events like "this image failed to load".

Closes #1618

Tomorrow I will test that events are firing correctly, but I open the PR so people can test it easily.

@CHaNGeTe CHaNGeTe changed the title Replace RCTBubblingEventBlock by RCTDirectEventBlock Replace RCTBubblingEventBlock by RCTDirectEventBlock to avoid event name collision with Expo AV Jun 18, 2019
@CHaNGeTe CHaNGeTe self-assigned this Jun 18, 2019
@jenshandersson
Copy link
Contributor

I've tested and it works. We should change the block types in RCTVideo.h as well though.

@danielmarino24i
Copy link
Contributor

Updated with changes on RCTVideo.h as well

  • Changelog

@CHaNGeTe
Copy link
Contributor Author

  • Tested with few events (onLoad, onProgress, onBuffer, ...) and it is working.

@CHaNGeTe CHaNGeTe changed the base branch from master to release/4.4.2 June 19, 2019 19:10
@CHaNGeTe CHaNGeTe merged commit a3b32ac into TheWidlarzGroup:release/4.4.2 Jun 19, 2019
beauner69 pushed a commit to beauner69/react-native-video that referenced this pull request Oct 10, 2019
…collision-3

Replace RCTBubblingEventBlock  by RCTDirectEventBlock to avoid event name collision with Expo AV

(rebased from commit a3b32ac)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants