rdkcentral / Lightning-SDK

SDK for Lightning framework
Apache License 2.0
130 stars 69 forks source link

VideoPlayer: capitalize Error event to maintain consistency #333

Closed ricardo-a-alves-alb closed 2 years ago

ricardo-a-alves-alb commented 2 years ago

Every event that is fired on the component that is consuming a VideoPlayer event is capitalized, resulting in a method named something like $videoPlayerPlay.

However, for the Error event, it is not capitalized properly which means that the method would be named something like $videoPlayererror.

This PR attempts to address this inconsistency.

erikhaandrikman commented 2 years ago

Since error is a property of the events object: https://github.com/rdkcentral/Lightning-SDK/blob/82b8948ca0e7efe7afe5a94ba5700d21a9cc880c/src/VideoPlayer/events.js#L28

is suggest using that value so we can maintain all event names in one place: fireOnConsumer(events['error'], { videoElement: videoEl, event: e })

ricardo-a-alves-alb commented 2 years ago

Since error is a property of the events object:

https://github.com/rdkcentral/Lightning-SDK/blob/82b8948ca0e7efe7afe5a94ba5700d21a9cc880c/src/VideoPlayer/events.js#L28

is suggest using that value so we can maintain all event names in one place: fireOnConsumer(events['error'], { videoElement: videoEl, event: e })

Just changed it and did the same to the Clear, AdStart and AdEnd events.

michielvandergeest commented 2 years ago

Thanks for this PR @ricardo-a-alves-alb

Starting v5.0.0 the VideoPlayer plugin is no longer part of the Lightning-SDK. It’s been moved to the Metrological-SDK.

I would suggest to open this PR there to see if it can get merged in.