-
Notifications
You must be signed in to change notification settings - Fork 264
WS-1959 SPIKE: Add share function to video carousels #13628
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
base: latest
Are you sure you want to change the base?
Conversation
| if (!playerElementRef.current) return; | ||
|
|
||
| if (playerKeyRef.current === playerKey) { | ||
| console.log('Media Loader player ref already existsRETURN'); | ||
| return; | ||
| } | ||
|
|
||
| playerKeyRef.current = playerKey; | ||
| console.log('Media Loader: INITIALISE PLAYER'); |
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.
These refs are here to prevent infinite re-renders caused by this use effect and the plugin combined.
Cycle of what was happening:
- hook to update videoOverlayContainer passed through to here from the carousel
- the hook is passed to and called by the plugin below (L211)
- this updates the state, and triggers a re-render of the carousel
- this triggers this use effect to run, re calling the hook from the plugin
- this updates the state , the loop continues forever 😢
| playlistObject?: Playlist; | ||
| plugins?: { | ||
| toLoad: { html: string; playerOnly?: boolean }[]; | ||
| toLoad: { html: string; playerOnly?: boolean; data?: any }[]; |
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.
things are set to any for right now whilst i was trying to debug and get something working - obvs not permanent!
| @@ -0,0 +1,24 @@ | |||
| /* eslint-disable import/no-extraneous-dependencies */ | |||
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.
This is copied over from the webcore implementation, would question whether we need it once we progress with this a bit more but have copied everything over as 1-1 as possible for now to help debugging and eliminate possible sources of things going wrong.
| const SPACING_4 = '16px'; | ||
| const SPACING_2 = '8px'; | ||
| const SPACING_7 = '28px'; | ||
|
|
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.
None of this styling follows our conventions, but again same reasoning as above for copying webcore as 1-1 as possible for now.
| if (selectedVideoIndex !== null) { | ||
| console.log( | ||
| 'blocks', | ||
| blocks?.[selectedVideoIndex]?.model?.video?.id.split(':'), | ||
| ); | ||
| urn = blocks?.[selectedVideoIndex]?.model?.video?.id.split(':')[4]; | ||
| } |
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.
probs would be a good idea to do this in the BFF, we could pass through the full share url including the service name from there and it would be a bit cleaner.
| const [shareUrl, setShareUrl] = useState(); | ||
|
|
||
| useEffect(() => { | ||
| setShareUrl(`https://${service}/articles/${urn}`); |
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.
see comment above about doing this work in the bff
Resolves JIRA: WS-1959
Summary
Following webcores pattern for a share button within the player:
Loads in a custom SMP plugin created by Webcore SFV to give access to the shadow-dom of the media player for a Video Overlay to be attached to.
Creates a React Portal for a VideoOverlay component with the share button in to render within.
Adds refs to new instances of the MediaLoader to prevent infinite reloads.
To be done
Code changes
Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links