feat: Videojs POC #2974
feat: Videojs POC #2974
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 39ffa92. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 3 targetsSent with 💌 from NxCloud. |
📬Published Alpha Packages:@codecademy/[email protected] |
|
🚀 Styleguide deploy preview ready! |
| const fullscreenToggle = controlBarFullScreen?.el() | ||
| fullscreenToggle?.insertAdjacentElement('beforebegin', el) | ||
|
|
||
| ReactDOM.render( |
There was a problem hiding this comment.
There were different ways to add custom components but none of them worked when the player is in fullscreen mode (REF), since the component needs to be within videojs tree. This approach solves that but has an limitation - gamut theme would not apply to ControlSettings component since it is being rendered from a separate dom tree than our primary.
There was a problem hiding this comment.
this brings me pause because it breaks ColorMode context. we could do this inline or maybe create.a wrapper around the render to pass the current mode context?
| const [show, setShow] = useState(false); | ||
| const target = useRef<HTMLButtonElement>(null); | ||
| return ( | ||
| <GamutProvider theme={coreTheme} useCache useGlobals> |
There was a problem hiding this comment.
Due to ReactDOM.render as it is not part of the actual dom tree, GamutProvider needs to be defined separately to make it work with different themes and colormodes
| @@ -0,0 +1,339 @@ | |||
| // eslint-disable-next-line gamut/no-css-standalone, import/no-webpack-loader-syntax | |||
| import '!style-loader!css-loader!video.js/dist/video-js.css'; | |||
There was a problem hiding this comment.
Had to force using style and css loader here, If not we would need to copy the entire css of videojs from video.js/dist/video-js.css
|
|
||
| import { css, Global } from '@emotion/react'; | ||
|
|
||
| const videojsStyle = css` |
There was a problem hiding this comment.
For customisations/themes custom css are the only way videojs supports right now. Open to suggestions if there's a better way of defining custom styles
There was a problem hiding this comment.
do we want to use percipio's styles? maybe we can loop stacey in to update these
| const videoSrc = playerOptions.src; | ||
| const videoPoster = playerOptions.poster; | ||
| videoSrc && player.src(videoSrc); | ||
| videoPoster && player.poster(videoPoster); |
There was a problem hiding this comment.
A lot of Accessibility fixes will also be needed here by manipulating DOM directly
| interface VideoJSProps { | ||
| options: any; | ||
| onReady?: (player: any) => void; | ||
| } |
There was a problem hiding this comment.
@type/videojs does not work for version 8 of videojs. v8+ comes with its own types but lacks a lot of types for options and players, which we would need to define separately
There was a problem hiding this comment.
still need to look into some of this CSS stuff here, but i'm wondering because of the added complexity and dependecies here if this should be moved into it's own package outside of Gamut (esp the CSS stuff makes me think this might be best?) @jakemhiller curious about your thoughts here!
i'm also going to look to see if other design systems have video players and take a look around
| const fullscreenToggle = controlBarFullScreen?.el() | ||
| fullscreenToggle?.insertAdjacentElement('beforebegin', el) | ||
|
|
||
| ReactDOM.render( |
There was a problem hiding this comment.
this brings me pause because it breaks ColorMode context. we could do this inline or maybe create.a wrapper around the render to pass the current mode context?
| @@ -0,0 +1,339 @@ | |||
| // eslint-disable-next-line gamut/no-css-standalone, import/no-webpack-loader-syntax | |||
| import '!style-loader!css-loader!video.js/dist/video-js.css'; | |||
Yeah, i think a separate package makes sense. @dreamwasp were you thinking it should still be in this repo? i think it would be fine in the repo, since the dependencies and stuff will be mostly isolated. I haven't taken a really deep look at the code, but it does seem like we have to do a lot to make it work, which isn't great. I haven't been close enough to the project to suggest any alternatives though, if that's even an option. |
Overview
Early draft of Videojs Integration.
Documentation
Notes
.cssfile results into build errors. Right now we have just copied entire videojs's styles from lib directly into our Global, we could also import css as -!style-loader!css-loader!video.js/dist/video-js.cssto force style-loader and get rid of base videojs styles from gamutReactDOM.renderit is not part of the primary dom tree the context gets lost for themes and colormodes so GamutProvider needs to be defined again.