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 playback and transcript scroll for mobile/tablet devices #369

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

Dananji
Copy link
Collaborator

@Dananji Dananji commented Jan 31, 2024

Related issue: #354

@Dananji Dananji marked this pull request as ready for review February 1, 2024 20:56
@Dananji Dananji marked this pull request as draft February 1, 2024 21:04
@Dananji Dananji marked this pull request as ready for review February 1, 2024 21:10
@Dananji Dananji changed the title Use touch event handlers in progress bar for mobile/tablet devices Fix playback and transcript scroll for mobile/tablet devices Feb 1, 2024
Copy link
Member

@cjcolvar cjcolvar left a comment

Choose a reason for hiding this comment

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

I like the autoscroll checkbox and think that code looks great!
The changes for mobile/tablet playback seem fairly big given all the changes to events. Would it be hard to separate these into two different PRs? I'm thinking that if we merge them separate it will be easier to isolate or revert the playback changes if needed.

src/services/browser.js Outdated Show resolved Hide resolved
@Dananji
Copy link
Collaborator Author

Dananji commented Feb 2, 2024

Did you mean split the transcript and event handlers into 2 separate PRs? I don't think it would be hard to do. Let me do that real quick.

@Dananji
Copy link
Collaborator Author

Dananji commented Feb 2, 2024

New PR with transcript changes: #371

@Dananji Dananji merged commit a0de6a6 into main Feb 2, 2024
2 checks passed
@Dananji Dananji deleted the ipad-playback-fix branch February 2, 2024 16:21
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.

2 participants