-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat: implement opacity to control visibility of subtitles #3583
feat: implement opacity to control visibility of subtitles #3583
Conversation
implemented per discussion on TheWidlarzGroup#3579 updated docs and linked onTextTrackDataChanged to the subtitle style to clarify intent on how to control visibility.
@KrzysztofMoch I believe the |
#3580 seems like this might be related. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please see my comment.
I think you can also add the ios change in this PR ?!
android/src/main/java/com/brentvatne/exoplayer/ExoPlayerView.java
Outdated
Show resolved
Hide resolved
actually, yeah I can add those changes, I'm going to update it now. |
… still not render the subtitles. added util function for safeGetFloat updated types
add data structure for subtitle styles for extensibility
@freeboub Added the Tested: ✅ Opacity set to 0: ✅ Opacity set to 0.5( not supported; but, no errors ) this is due to limitations with alpha support on ✅ Opacity set to 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small remark, else it looks good to me !
Co-authored-by: Olivier Bouillet <[email protected]>
great catch, thank you! I think the cache needs to be cleared for the Build Android, and then it should be good for merge? 🥹 |
@coofzilla Yes this is OK for me! |
Summary
Add ability to control subtitle visibility while still maintaining ability of selecting a text track.
Motivation
see #3579
Changes
Updated
subtitleStyle
to include anopacity
property; which, can control the visibility of subtitles.Updated documentation for the new property and provided a link
onTextTrackDataChanged
->subtitleStyles
to clarify intent on how to control visibility of subtitles.Test plan
✅ Tested on example app:
Simply set the following props:
Then observe that the
subtitleTracks
log fromonTextTrackDataChanged
and do not display them :)Subtitles displayed as expected when set to 1:
If not explicitly set, they will be displayed:
Type safe, must be 0 or 1