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

fix(android): add subtitleStyle.subtitlesFollowVideo prop to control subtitles positionning #4133

Conversation

freeboub
Copy link
Collaborator

@freeboub freeboub commented Sep 1, 2024

Summary

add subtitleStyle.subtitlesFollowVideo prop to control subtitles positionning
2 options: follow video or follow main view

Here is description of the new property:

subtitlesFollowVideo helps to determine how the subtitles are positionned.
To understand this prop you need to understand how views management works.
The main View style passed to react native video is the position reserved to display the video component.
It may not match exactly the real video size.
For exemple, you can pass a 4:3 video view and render a 16:9 video inside.
So there is a second view, the video view.

Subtitles are managed in a third view.

First react-native-video resize the video to keep aspect ratio (depending on resizeMode property) and put it in main view.

  • When putting subtitlesFollowVideo to true, the subtitle view will be adapt to the video view.
    It means that if the video is displayed out of screen, the subtitles may also be displayed out of screen.

  • When putting subtitlesFollowVideo to false, the subtitle view will keep adapting to the main view.
    It means that if the video is displayed out of screen, the subtitles may also be displayed out of screen.

This prop can be changed on runtime.

Motivation

Fix following regression: #4019 (will be fixed by default)
Regression introduced by following PR: #3830
Linked tickets: #3662 and #4043

Changes

dynamically configure subtitle position with a new prop added in subtitleStyle

Test plan

Can be tested with the sample. I didn't add a button to trigger change, but I tested with manually change the value

Note, it is not implemented on ios, I think it should ...

@freeboub freeboub merged commit 2fa6c43 into TheWidlarzGroup:master Sep 2, 2024
6 checks passed
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.

3 participants