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.

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

Media card alignment #13040

Merged
merged 4 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 10 additions & 8 deletions dotcom-rendering/src/components/Card/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from '@guardian/source/foundations';
import { Hide, Link } from '@guardian/source/react-components';
import { ArticleDesign, type ArticleFormat } from '../../lib/articleFormat';
import { isMediaCard } from '../../lib/cardHelpers';
import { isMediaCard as isAMediaCard } from '../../lib/cardHelpers';
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, think I prefer this to what you had earlier (where isAMediaCard was the const declared on line 456) 👍

import { getZIndex } from '../../lib/getZIndex';
import { DISCUSSION_ID_DATA_ATTRIBUTE } from '../../lib/useCommentCount';
import { palette } from '../../palette';
Expand Down Expand Up @@ -260,11 +260,14 @@ const getHeadlinePosition = ({
isFlexSplash,
containerType,
showLivePlayable,
isMediaCard,
}: {
containerType?: DCRContainerType;
isFlexSplash?: boolean;
showLivePlayable: boolean;
isMediaCard: boolean;
}) => {
if (isMediaCard) return 'inner';
if (containerType === 'flexible/special' && isFlexSplash) {
return 'outer';
}
Expand Down Expand Up @@ -450,9 +453,9 @@ export const Card = ({
- * Media cards have contrasting background colours. We add additional
* padding to these cards to keep the text readable.
- */
const hasBackgroundColour = isMediaCard(format);
const isMediaCard = isAMediaCard(format);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a better name for this var 👍 I lazily left it alone as it clashed with the method name.


const backgroundColour = hasBackgroundColour
const backgroundColour = isMediaCard
? palette('--card-media-background')
: palette('--card-background');

Expand All @@ -479,6 +482,7 @@ export const Card = ({
containerType,
isFlexSplash,
showLivePlayable,
isMediaCard,
});

const hideTrailTextUntil = () => {
Expand All @@ -498,7 +502,7 @@ export const Card = ({
/** Determines the gap of between card components based on card properties */
const getGapSize = (): GapSize => {
if (isOnwardContent) return 'none';
if (hasBackgroundColour && !isFlexibleContainer) return 'tiny';
if (isMediaCard && !isFlexibleContainer) return 'tiny';
if (!!isFlexSplash || (isFlexibleContainer && imageSize === 'jumbo')) {
return 'small';
}
Expand Down Expand Up @@ -812,7 +816,7 @@ export const Card = ({
imageType={media?.type}
imageSize={imageSize}
imagePositionOnDesktop={imagePositionOnDesktop}
hasBackgroundColour={hasBackgroundColour}
hasBackgroundColour={isMediaCard}
isOnwardContent={isOnwardContent}
isFlexibleContainer={isFlexibleContainer}
>
Expand Down Expand Up @@ -943,9 +947,7 @@ export const Card = ({
<div
style={{
padding:
hasBackgroundColour || isOnwardContent
? `0 ${space[2]}px`
: 0,
isMediaCard || isOnwardContent ? `0 ${space[2]}px` : 0,
}}
>
{showLivePlayable && liveUpdatesPosition === 'outer' && (
Expand Down
21 changes: 13 additions & 8 deletions dotcom-rendering/src/components/FlexibleGeneral.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isMediaCard } from '../lib/cardHelpers';
import { palette } from '../palette';
import type { BoostLevel } from '../types/content';
import type {
Expand Down Expand Up @@ -86,6 +87,7 @@ type BoostedSplashProperties = {
const decideSplashCardProperties = (
boostLevel: BoostLevel,
supportingContentLength: number,
mediaCard: boolean,
): BoostedSplashProperties => {
switch (boostLevel) {
// boostedfont sizing
Expand All @@ -98,7 +100,7 @@ const decideSplashCardProperties = (
mobile: 'medium',
},
imagePositionOnDesktop: 'right',
imagePositionOnMobile: 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'large',
supportingContentAlignment:
supportingContentLength >= 4 ? 'horizontal' : 'vertical',
Expand All @@ -113,7 +115,7 @@ const decideSplashCardProperties = (
mobile: 'large',
},
imagePositionOnDesktop: 'right',
imagePositionOnMobile: 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'jumbo',
supportingContentAlignment:
supportingContentLength >= 4 ? 'horizontal' : 'vertical',
Expand All @@ -127,8 +129,8 @@ const decideSplashCardProperties = (
tablet: 'xlarge',
mobile: 'xlarge',
},
imagePositionOnDesktop: 'bottom',
imagePositionOnMobile: 'bottom',
imagePositionOnDesktop: mediaCard ? 'top' : 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'jumbo',
supportingContentAlignment: 'horizontal',
liveUpdatesAlignment: 'horizontal',
Expand All @@ -141,8 +143,8 @@ const decideSplashCardProperties = (
tablet: 'xlarge',
mobile: 'xxlarge',
},
imagePositionOnDesktop: 'bottom',
imagePositionOnMobile: 'bottom',
imagePositionOnDesktop: mediaCard ? 'top' : 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'jumbo',
supportingContentAlignment: 'horizontal',
liveUpdatesAlignment: 'horizontal',
Expand Down Expand Up @@ -182,6 +184,7 @@ export const SplashCardLayout = ({
} = decideSplashCardProperties(
card.boostLevel ?? 'default',
card.supportingContent?.length ?? 0,
isMediaCard(card.format),
);

return (
Expand Down Expand Up @@ -312,8 +315,10 @@ export const BoostedCardLayout = ({
showAge={showAge}
absoluteServerTimes={absoluteServerTimes}
headlineSizes={headlineSizes}
imagePositionOnDesktop={'right'}
imagePositionOnMobile={'bottom'}
imagePositionOnDesktop="right"
imagePositionOnMobile={
isMediaCard(card.format) ? 'top' : 'bottom'
}
imageSize={imageSize}
trailText={card.trailText}
supportingContent={card.supportingContent}
Expand Down
31 changes: 22 additions & 9 deletions dotcom-rendering/src/components/FlexibleSpecial.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isMediaCard } from '../lib/cardHelpers';
import type { BoostLevel } from '../types/content';
import type {
AspectRatio,
Expand Down Expand Up @@ -42,6 +43,7 @@ type BoostProperties = {
const determineCardProperties = (
boostLevel: BoostLevel,
supportingContentLength: number,
mediaCard: boolean,
): BoostProperties => {
switch (boostLevel) {
// The default boost level is equal to no boost. It is the same as the default card layout.
Expand All @@ -53,7 +55,7 @@ const determineCardProperties = (
mobile: 'medium',
},
imagePositionOnDesktop: 'right',
imagePositionOnMobile: 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'large',
supportingContentAlignment:
supportingContentLength >= 3 ? 'horizontal' : 'vertical',
Expand All @@ -69,7 +71,7 @@ const determineCardProperties = (
mobile: 'large',
},
imagePositionOnDesktop: 'right',
imagePositionOnMobile: 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'jumbo',
supportingContentAlignment:
supportingContentLength >= 3 ? 'horizontal' : 'vertical',
Expand All @@ -83,8 +85,8 @@ const determineCardProperties = (
tablet: 'xlarge',
mobile: 'xlarge',
},
imagePositionOnDesktop: 'bottom',
imagePositionOnMobile: 'bottom',
imagePositionOnDesktop: mediaCard ? 'top' : 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'jumbo',
supportingContentAlignment: 'horizontal',
liveUpdatesAlignment: 'horizontal',
Expand All @@ -97,8 +99,8 @@ const determineCardProperties = (
tablet: 'xxlarge',
mobile: 'xxlarge',
},
imagePositionOnDesktop: 'bottom',
imagePositionOnMobile: 'bottom',
imagePositionOnDesktop: mediaCard ? 'top' : 'bottom',
imagePositionOnMobile: mediaCard ? 'top' : 'bottom',
imageSize: 'jumbo',
supportingContentAlignment: 'horizontal',
liveUpdatesAlignment: 'horizontal',
Expand Down Expand Up @@ -137,6 +139,7 @@ export const OneCardLayout = ({
} = determineCardProperties(
card.boostLevel ?? 'default',
card.supportingContent?.length ?? 0,
isMediaCard(card.format),
);
return (
<UL padBottom={!isLastRow} hasLargeSpacing={!isLastRow}>
Expand Down Expand Up @@ -170,6 +173,15 @@ export const OneCardLayout = ({
);
};

const getImagePosition = (
hasTwoOrFewerCards: boolean,
isAMediaCard: boolean,
) => {
if (isAMediaCard && !hasTwoOrFewerCards) return 'top';
if (hasTwoOrFewerCards) return 'left';
return 'bottom';
};

const TwoCardOrFourCardLayout = ({
cards,
containerPalette,
Expand Down Expand Up @@ -208,9 +220,10 @@ const TwoCardOrFourCardLayout = ({
absoluteServerTimes={absoluteServerTimes}
image={showImage ? card.image : undefined}
imageLoading={imageLoading}
imagePositionOnDesktop={
hasTwoOrFewerCards ? 'left' : 'bottom'
}
imagePositionOnDesktop={getImagePosition(
hasTwoOrFewerCards,
isMediaCard(card.format),
)}
/* we don't want to support sublinks on standard cards here so we hard code to undefined */
supportingContent={undefined}
imageSize={'medium'}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isMediaCard } from '../lib/cardHelpers';
import type {
AspectRatio,
DCRContainerPalette,
Expand Down Expand Up @@ -40,6 +41,9 @@ export const ScrollableMedium = ({
visibleCardsOnTablet={4}
>
{trails.map((trail) => {
const imagePosition = isMediaCard(trail.format)
? 'top'
: 'bottom';
return (
<ScrollableCarousel.Item key={trail.url}>
<FrontCard
Expand All @@ -53,8 +57,8 @@ export const ScrollableMedium = ({
desktop: 'xsmall',
tablet: 'xxsmall',
}}
imagePositionOnDesktop="bottom"
imagePositionOnMobile="bottom"
imagePositionOnDesktop={imagePosition}
imagePositionOnMobile={imagePosition}
imageSize="medium"
trailText={undefined} // unsupported
supportingContent={undefined} // unsupported
Expand Down
5 changes: 4 additions & 1 deletion dotcom-rendering/src/components/StaticMediumFour.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isMediaCard } from '../lib/cardHelpers';
import type {
AspectRatio,
DCRContainerPalette,
Expand Down Expand Up @@ -48,7 +49,9 @@ export const StaticMediumFour = ({
absoluteServerTimes={absoluteServerTimes}
image={showImage ? card.image : undefined}
imageLoading={imageLoading}
imagePositionOnDesktop={'bottom'}
imagePositionOnDesktop={
isMediaCard(card.format) ? 'top' : 'bottom'
}
/* we don't want to support sublinks on standard cards here so we hard code to undefined */
supportingContent={undefined}
imageSize={'medium'}
Expand Down
Loading