WS-1994: Support portrait video on the Live Page#13668
WS-1994: Support portrait video on the Live Page#13668alex-magana wants to merge 30 commits intolatestfrom
Conversation
alex-magana
changed the title
alex-magana
changed the title
| duration: string; | ||
| kind: string; | ||
| guidance: string | null; | ||
| orientation: string | null; |
There was a problem hiding this comment.
Could we use the Orientations type here?
There was a problem hiding this comment.
Yes. This is feasible.
Let me change this to Orientations.
| video: (props: { blocks: MediaBlock[] }) => { | ||
| const { blocks } = props; | ||
| const isPortrait = isPortraitVideo(blocks); | ||
| const className = isPortrait ? 'portrait-clip-media' : ''; |
There was a problem hiding this comment.
Just wondering can we use the existing .media-container classname that exists on the figure element, rather than appending portrait-clip-media to it?
We should be able to do something like:
portraitVideoPlayer: () =>
css({
'.media-container': {
margin: '20px auto 0',
width: `${pixelsToRem(247)}rem`,
},
}),
There was a problem hiding this comment.
Thanks for taking a look at these changes, Aaron.
The rationale behind portrait-clip-media is adopted from the approach used
in the MediaArticlePage. I'll change the styling to media-container and see how it plays out.
| return ORIENTATION_MAPPING.ORIGINAL; | ||
| } | ||
|
|
||
| const transformedConfig = transformOrientationConfigToUpperCase( |
There was a problem hiding this comment.
| const transformedConfig = transformOrientationConfigToUpperCase( | |
| const transformedConfig = mediaOrientationConfig.map(setting => setting.toUpperCase()) |
I think this doesn't need to be in it's own function. I think it's pretty readable as it is.
| const orientationType = | ||
| transformedConfig.find(orientationConfig => | ||
| Object.keys(ORIENTATION_MAPPING).includes( | ||
| orientationConfig.toUpperCase(), | ||
| ), | ||
| ) ?? 'ORIGINAL'; |
There was a problem hiding this comment.
Would just using transformedConfig.find(orientationConfig => orientationConfig in ORIENTATION_MAPPING) ?? 'ORIGINAL' work here instead of using Object.keys()?
Also not sure why orientationConfig needs to be made into uppercase again on line 27
| portraitVideoPlayer: () => | ||
| css({ | ||
| '.media-container': { | ||
| margin: '20px auto 0', |
There was a problem hiding this comment.
Should this be using pixelsToRem(20)?
5 participants
Resolves JIRA: WS-1994
Summary
Adopt portrait video on Live Page posts.
Code changes
clipMedia, in the SMP player config.isPortraitVideoutility so that it's shared between Article Page, Media Article Page and Live Page.AresMediaBlockandAresMediaMetadataBlock, to reflect the structure of the payload provided by FABL.Testing
Useful Links