Skip to content
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.

Sign up for GitHub

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

Rework inline play behaviour #13023

Merged
merged 2 commits into from
Dec 18, 2024
+39 −32
Merged

Rework inline play behaviour #13023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd9da60
Rename video prop to better represent use case
abeddow91 Dec 17, 2024
b839c44
Merge branch 'main' into ab/inline-play
abeddow91 Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions dotcom-rendering/src/components/Card/Card.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const basicCardProps: CardProps = {
imagePositionOnDesktop: 'top',
showAge: true,
isExternalLink: false,
isPlayableMediaCard: true,
canPlayInline: true,
imageLoading: 'eager',
discussionApiUrl: 'https://discussion.theguardian.com/discussion-api/',
showMainVideo: true,
Expand Down Expand Up @@ -1001,7 +1001,7 @@ export const WhenVideoWithPlayButton = () => {
}}
imagePositionOnDesktop="top"
mainMedia={mainVideo}
isPlayableMediaCard={false}
canPlayInline={false}
/>
</LI>
</UL>
Expand Down Expand Up @@ -1032,7 +1032,7 @@ export const WhenVideoWithPlayButton = () => {
}}
imagePositionOnDesktop="left"
mainMedia={mainVideo}
isPlayableMediaCard={false}
canPlayInline={false}
/>
</LI>
<LI padSides={true}>
Expand All @@ -1045,7 +1045,7 @@ export const WhenVideoWithPlayButton = () => {
}}
imagePositionOnDesktop="right"
mainMedia={mainVideo}
isPlayableMediaCard={false}
canPlayInline={false}
/>
</LI>

Expand All @@ -1059,7 +1059,7 @@ export const WhenVideoWithPlayButton = () => {
}}
imagePositionOnDesktop="right"
mainMedia={mainVideo}
isPlayableMediaCard={false}
canPlayInline={false}
/>
</LI>
</UL>
Expand Down
14 changes: 7 additions & 7 deletions dotcom-rendering/src/components/Card/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export type Props = {
* At 300px or below, the player will begin to lose functionality e.g. volume controls being omitted.
* Youtube requires a minimum width 200px.
*/
isPlayableMediaCard?: boolean;
canPlayInline?: boolean;
kickerText?: string;
showPulsingDot?: boolean;
starRating?: Rating;
Expand Down Expand Up @@ -193,7 +193,7 @@ const getMedia = ({
isCrossword,
slideshowImages,
mainMedia,
isPlayableMediaCard,
canPlayInline,
podcastImage,
isBetaContainer,
}: {
Expand All @@ -203,11 +203,11 @@ const getMedia = ({
isCrossword?: boolean;
slideshowImages?: DCRSlideshowImage[];
mainMedia?: MainMedia;
isPlayableMediaCard?: boolean;
canPlayInline?: boolean;
podcastImage?: PodcastSeriesImage;
isBetaContainer: boolean;
}) => {
if (mainMedia && mainMedia.type === 'Video' && isPlayableMediaCard) {
if (mainMedia && mainMedia.type === 'Video' && canPlayInline) {
return {
type: 'video',
mainMedia,
Expand Down Expand Up @@ -306,7 +306,7 @@ export const Card = ({
avatarUrl,
showClock,
mainMedia,
isPlayableMediaCard,
canPlayInline,
kickerText,
showPulsingDot,
starRating,
Expand Down Expand Up @@ -427,7 +427,7 @@ export const Card = ({
// If the card isn't playable, we need to show a play icon.
// Otherwise, this is handled by the YoutubeAtom
const showPlayIcon =
mainMedia?.type === 'Video' && !isPlayableMediaCard && showMainVideo;
mainMedia?.type === 'Video' && !canPlayInline && showMainVideo;

const media = getMedia({
imageUrl: image?.src,
Expand All @@ -436,7 +436,7 @@ export const Card = ({
isCrossword,
slideshowImages,
mainMedia,
isPlayableMediaCard,
canPlayInline,
podcastImage,
isBetaContainer,
});
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/Carousel.importable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ const CarouselCard = ({
isExternalLink={false}
mainMedia={mainMedia}
minWidthInPixels={220}
isPlayableMediaCard={isVideoContainer}
canPlayInline={isVideoContainer}
onwardsSource={onwardsSource}
containerType={containerType}
imageLoading={imageLoading}
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/FeatureCard.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const cardProps: CardProps = {
},
imagePositionOnDesktop: 'top',
isExternalLink: false,
isPlayableMediaCard: false,
canPlayInline: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a story so not super important, but was this meant to be changed to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, feature cards should play inline 👍

imageLoading: 'eager',
discussionApiUrl: 'https://discussion.theguardian.com/discussion-api/',
absoluteServerTimes: true,
Expand Down
12 changes: 6 additions & 6 deletions dotcom-rendering/src/components/FeatureCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export type Props = {
* At 300px or below, the player will begin to lose functionality e.g. volume controls being omitted.
* Youtube requires a minimum width 200px.
*/
isPlayableMediaCard?: boolean;
canPlayInline?: boolean;
kickerText?: string;
showPulsingDot?: boolean;
starRating?: Rating;
Expand Down Expand Up @@ -166,14 +166,14 @@ const getMedia = ({
imageUrl,
imageAltText,
mainMedia,
isPlayableMediaCard,
canPlayInline,
}: {
imageUrl?: string;
imageAltText?: string;
mainMedia?: MainMedia;
isPlayableMediaCard?: boolean;
canPlayInline?: boolean;
}) => {
if (mainMedia && mainMedia.type === 'Video' && isPlayableMediaCard) {
if (mainMedia && mainMedia.type === 'Video' && canPlayInline) {
return {
type: 'video',
mainMedia,
Expand Down Expand Up @@ -279,7 +279,7 @@ export const FeatureCard = ({
imageLoading,
showClock,
mainMedia,
isPlayableMediaCard,
canPlayInline,
kickerText,
showPulsingDot,
dataLinkName,
Expand All @@ -305,7 +305,7 @@ export const FeatureCard = ({
imageUrl: image?.src,
imageAltText: image?.altText,
mainMedia,
isPlayableMediaCard,
canPlayInline,
});

return (
Expand Down
3 changes: 3 additions & 0 deletions dotcom-rendering/src/components/FlexibleGeneral.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ export const SplashCardLayout = ({
showTopBarDesktop={false}
showTopBarMobile={true}
trailTextSize={trailTextSize}
canPlayInline={true}
/>
</LI>
</UL>
Expand Down Expand Up @@ -329,6 +330,7 @@ export const BoostedCardLayout = ({
showTopBarDesktop={false}
showTopBarMobile={true}
liveUpdatesPosition={liveUpdatesPosition}
canPlayInline={true}
/>
</LI>
</UL>
Expand Down Expand Up @@ -413,6 +415,7 @@ export const StandardCardLayout = ({
}
: undefined
}
canPlayInline={false}
/>
</LI>
);
Expand Down
2 changes: 2 additions & 0 deletions dotcom-rendering/src/components/FlexibleSpecial.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ export const OneCardLayout = ({
showTopBarDesktop={false}
showTopBarMobile={true}
trailTextSize={trailTextSize}
canPlayInline={true}
/>
</LI>
</UL>
Expand Down Expand Up @@ -218,6 +219,7 @@ const TwoCardOrFourCardLayout = ({
showLivePlayable={false}
showTopBarDesktop={false}
showTopBarMobile={true}
canPlayInline={false}
/>
</LI>
);
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/FrontCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const FrontCard = (props: Props) => {
showClock: false,
image: trail.image,
isCrossword: trail.isCrossword,
isPlayableMediaCard: true,
canPlayInline: true,
starRating: trail.starRating,
dataLinkName: trail.dataLinkName,
snapData: trail.snapData,
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/ScrollableFeature.importable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const ScrollableFeature = ({
/** TODO - implement show age */
showClock={false}
image={card.image}
isPlayableMediaCard={true}
canPlayInline={true}
starRating={card.starRating}
dataLinkName={card.dataLinkName}
discussionApiUrl={card.discussionApiUrl}
Expand Down
1 change: 1 addition & 0 deletions dotcom-rendering/src/components/ScrollableSmall.importable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export const ScrollableSmall = ({
showLivePlayable={trail.showLivePlayable}
showTopBarDesktop={false}
showTopBarMobile={false}
canPlayInline={false}
/>
</ScrollableCarousel.Item>
);
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/StaticFeatureTwo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const StaticFeatureTwo = ({
/** TODO - implement show age */
showClock={false}
image={card.image}
isPlayableMediaCard={true}
canPlayInline={true}
starRating={card.starRating}
dataLinkName={card.dataLinkName}
discussionApiUrl={card.discussionApiUrl}
Expand Down
1 change: 1 addition & 0 deletions dotcom-rendering/src/components/StaticMediumFour.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const StaticMediumFour = ({
showLivePlayable={false}
showTopBarDesktop={false}
showTopBarMobile={true}
canPlayInline={false}
/>
</LI>
);
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/src/components/SupportingContent.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const basicCardProps: CardProps = {
imagePositionOnDesktop: 'top',
isExternalLink: false,
showLivePlayable: false,
isPlayableMediaCard: true,
canPlayInline: true,
imageLoading: 'eager',
discussionApiUrl: 'https://discussion.theguardian.com/discussion-api',
absoluteServerTimes: true,
Expand Down
16 changes: 8 additions & 8 deletions dotcom-rendering/src/lib/cardWrappers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ export const Card25Media25 = ({
imageLoading={imageLoading}
isTagPage={isTagPage}
headlineSizes={{ desktop: 'xsmall', tablet: 'xxsmall' }}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down Expand Up @@ -393,7 +393,7 @@ export const Card25Media25SmallHeadline = ({
imageLoading={imageLoading}
isTagPage={isTagPage}
headlineSizes={{ desktop: 'xxsmall', tablet: 'xxsmall' }}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down Expand Up @@ -442,7 +442,7 @@ export const Card25Media25Tall = ({
: undefined
}
supportingContent={trail.supportingContent?.slice(0, 2)}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down Expand Up @@ -483,7 +483,7 @@ export const Card25Media25TallNoTrail = ({
isTagPage={isTagPage}
headlineSizes={{ desktop: 'xsmall', tablet: 'xxsmall' }}
supportingContent={trail.supportingContent?.slice(0, 2)}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down Expand Up @@ -524,7 +524,7 @@ export const Card25Media25TallSmallHeadline = ({
isTagPage={isTagPage}
headlineSizes={{ desktop: 'xxsmall', tablet: 'xxsmall' }}
supportingContent={trail.supportingContent?.slice(0, 2)}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down Expand Up @@ -805,7 +805,7 @@ export const CardDefault = ({
imageLoading={'lazy'}
avatarUrl={undefined}
headlineSizes={{ desktop: 'xxsmall' }}
isPlayableMediaCard={false}
canPlayInline={false}
isTagPage={isTagPage}
trailTextColour={palette('--card-headline')}
/>
Expand Down Expand Up @@ -844,7 +844,7 @@ export const CardDefaultMedia = ({
imageLoading={imageLoading}
isTagPage={isTagPage}
headlineSizes={{ desktop: 'xxsmall' }}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down Expand Up @@ -882,7 +882,7 @@ export const CardDefaultMediaMobile = ({
imageLoading={imageLoading}
isTagPage={isTagPage}
headlineSizes={{ desktop: 'xxsmall' }}
isPlayableMediaCard={false}
canPlayInline={false}
trailTextColour={palette('--card-headline')}
/>
);
Expand Down
Loading