-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Add video playback #74
Conversation
381e78d
to
c121c3d
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
c121c3d
to
bfbd9f9
Compare
bfbd9f9
to
8874a31
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
For what it's worth just had a look through the code because i was curious how you implemented the player.
import { stringify } from 'qs'; | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
import shaka from 'shaka-player/dist/shaka-player.ui'; |
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.
Have you considered adding the shaka-player types? ( first hit: https://github.com/niklaskorz/shaka-player/tree/feature/generate-typescript-typedefs/test/typescript )
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 have, but unfortunately that repository is severely out of date. We also prefer to stick with the upstream versions of packages.
The Shaka team does intend to ship typings very soon, however, so this will be fixed when the next version of Shaka Player is released (but that's a guess based on recent activity on their repository)
video { | ||
width: 100vw; | ||
height: 100vh; | ||
} |
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 assume you have tested this, but just to be sure... have you considered overflowing in some browsers?
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, but we probably need to account for safe areas, yeah.
Are you backporting the strings from Jellyfin Web or using Google Translate? If they're from JF-web we shouldn't include them, as context of the strings might be different. Imo, we should rely on new translations for those imo. |
We're usually writing entirely new strings. I don't Translate stuff (aside from French once in a while), I leave that to the folks over on Weblate. |
Is there a way to exit the playback? |
This is currently using the default Shaka playback UI. There is no way to exit the playback and go back to the item details page as current |
OK, good to know. |
You can use the back button on the browser, but that's about it until we add a custom UI. |
Next steps:
Note: Typings are suppressed for Mux.js and Shaka Player. Ideally, we should submit Mux.js typings to DefinitelyTyped. Shaka Player has typings coming in the next version (probably).