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: subtitle syncing issues #112

Merged
merged 5 commits into from
Jan 21, 2025
Merged

fix: subtitle syncing issues #112

merged 5 commits into from
Jan 21, 2025

Conversation

mta452
Copy link

@mta452 mta452 commented Jan 8, 2025

LEARNER-10314: iOS - Transcript gets out of sync when user seeks the video

Following scenarios have been implemented.

  • The view will automatically scroll to current subtitle when the user seeks the video
  • If the user manually scrolls the subtitles view, he will be automatically taken to the current subtitle after three seconds
  • The view will also scroll to current subtitle if the user seeks the video in full screen mode

Here's a quick demo of the implementation.

Subtitle.Sync.Demo.mov

Copy link
Collaborator

@rnr rnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if viewModel.isPlaying {
isAnimating = true

withAnimation(.linear(duration: 0.3)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: I think we should move it to a private property so that if we need to change the auto-scroll time later, anyone can simply update this value without digging deep into the code. WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have put the constants in a private enum.

Copy link

@shafqat-muneer shafqat-muneer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@mta452 mta452 merged commit f4c2502 into 2U/develop Jan 21, 2025
2 checks passed
shafqat-muneer pushed a commit that referenced this pull request Feb 4, 2025
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.

4 participants