Skip to content

Refine progress-bar by using VideoJS' native ProgressControl - #664

Merged
Dananji merged 2 commits into
mainfrom
progressbar-637
Oct 21, 2024
Merged

Refine progress-bar by using VideoJS' native ProgressControl#664
Dananji merged 2 commits into
mainfrom
progressbar-637

Conversation

@Dananji

@Dananji Dananji commented Oct 14, 2024

Copy link
Copy Markdown
Collaborator

Related issue: #637

@Dananji Dananji changed the title Progressbar 637 Refine progress-bar by using VideoJS' native ProgressControl Oct 14, 2024

@cjcolvar cjcolvar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good as far as I can tell. It would be nice if there was a cleaner way to override the seek bar implementation in VideoJS so you don't have to override the progress control suppress the native seekbar and forward events but that is probably unavoidable. Overall still seems better than everything custom as it was before.

@Dananji

Dananji commented Oct 21, 2024

Copy link
Copy Markdown
Collaborator Author

Yes, this is not ideal 💯 There's probably a better way to do this, which I want to explore as I get time in the future 🙂
I tried removing the native SeekBar and attaching the custom implementation and it breaks the other child components. So, I tried overriding the code that breaks in the child components (especially in TimeTooltip component) which didn't seem to work.

@Dananji
Dananji merged commit b896213 into main Oct 21, 2024
@Dananji
Dananji deleted the progressbar-637 branch October 21, 2024 15:20
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