-
Notifications
You must be signed in to change notification settings - Fork 125
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
[DRAFT] Demo: store the settings in the url hash #1349
base: dev
Are you sure you want to change the base?
Conversation
I would advocate for both:
|
0b408c2
to
e7c95fc
Compare
const loadContent = React.useCallback((content: IPlayableContent) => { | ||
getLoadVideoOptions(content).then( | ||
(loadVideoOptions) => { | ||
loadVideo(loadVideoOptions); |
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.
I didn't look too much into it yet but have you verified that this code is not sensible to race conditions?
For example, if loadContent
is called multiple consecutive times, are we sure that the last call will lead to playback in the end?
Will look more into it, but that's a question I was asking myself reading this
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.
(the issue may already have been there before)
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.
I tried to provoke a race condition but didn't succeed in doing so.
I still have added the code to prevent any race condition, let me know what you think of that because I'm not 100% sure that this is the correct way of doing so.
90fbc13
to
cd9ff08
Compare
e7c95fc
to
b66c200
Compare
1de6ca5
to
a18689b
Compare
b66c200
to
7731780
Compare
…ite-spinner fix(directfile): media with autoplay:false is infinitly in loading state
When doing some hotfixes for our v4.0.0 version, we directly integrated a browser detection check in the main code of the RxPlayer. The normal way for the RxPlayer of handling device compatibility, is to clearly list and document all differences in the `src/compat` directory, not directly in the main code. This commit fixes that by giving a name to that work-around, documentating it and moving it in the `src/compat` directory.
…from-initial-seek-and-play Remove `isSafariMobile` mention from `main_thread` code.
1d13bfb
to
1a7da65
Compare
Storing the player settings in the url hash let us refresh the demo page while keeping all the settings previously set.
It's more efficient and comfortable when debugging to not have to reset them manually after each page refresh.
To Do: