Skip to content

Commit fbaad76

Browse files
abeddow91abeddow91
authored
Star rating redesign (#15032)
* Rename existing starRating component to starRatingDeprecated so its easier to add a 0% switch for testing over festive period * Add a StarRating component with updated sizing and colours * Add a medium star rating size and update schemas * Add variable padding to star ratings * Move block component colours into their own style object so they can be controlled via test * Conditionally render star rating in feature cards * Conditionally render star rating in card * Conditionally render star rating on cards in scrollables * Conditionally render star rating on cards in flexibles * Conditionally render star rating on cards in highlights * don't test on older containers as they're being deprecated * Conditionally render in articles * Use dynamic sizing in card component * Update padding in headline to reduce mobile bottom padding fro 28 to 20 * Use dynamic sizing on feature cards for stars * Update star rating sizes for flexible containers * Make sure to use abtest not switch! * Conditionally render block component with background colours if user is not in variant * Conditionally render rich link if user is in variant * Conditionally render star rating on image block component if user is in variant * fix linting * Make variant optional * Add missing variants * Add star rating dark theme param for switching to a darker palette for accessibility requirments. These are needed for cards such as feature cards which have lighter / variable backgrounds * Fix linting * Use guardian space values where possible * Remove logging * Rename deprecated stories so it is easier to clean up post test * Add modern suite of star rating stories. This update uses a ratings map to reduce boiler plate code. * Prefer `alternative` over `dark` when referring to the second theme for star ratings. This is to prevent overloading the term dark which is more commonly used in the code base to refer to dark mode * render the new star rating component or the deprecated component depending on if the user is in the test variant * Update stories for star rating block component * Use new variant on lightbox * Remove padding bottom 1px from rich link title * Add `none` to padding size tpe for controlling 0 top padding
1 parent f4af771 commit fbaad76

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+822
-198
lines changed

‎dotcom-rendering/src/components/ArticleHeadline.tsx‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { css } from '@emotion/react';
2+
import { isUndefined } from '@guardian/libs';
23
import {
34
between,
45
from,
@@ -28,10 +29,12 @@ import {
2829
} from '../lib/articleFormat';
2930
import { getZIndex } from '../lib/getZIndex';
3031
import { palette as themePalette } from '../palette';
32+
import type { StarRating as Rating } from '../types/content';
3133
import type { TagType } from '../types/tag';
3234
import { AgeWarning } from './AgeWarning';
3335
import { DesignTag } from './DesignTag';
3436
import { HeadlineByline } from './HeadlineByline';
37+
import { StarRating } from './StarRating/StarRating';
3538

3639
type Props = {
3740
headlineString: string;
@@ -41,6 +44,8 @@ type Props = {
4144
webPublicationDateDeprecated: string;
4245
hasAvatar?: boolean;
4346
isMatch?: boolean;
47+
starRating?: Rating;
48+
isInStarRatingVariant?: boolean;
4449
};
4550

4651
const topPadding = css`
@@ -121,15 +126,13 @@ const headlineFont = (format: ArticleFormat) => {
121126
if (format.design === ArticleDesign.Gallery) {
122127
return css`
123128
${decideMobileHeadlineFont(format)}
124-
125129
${from.desktop} {
126130
${decideHeadlineFont(format)}
127131
}
128132
`;
129133
}
130134
return css`
131135
${decideMobileHeadlineFont(format)}
132-
133136
${from.tablet} {
134137
${decideHeadlineFont(format)}
135138
}
@@ -138,6 +141,7 @@ const headlineFont = (format: ArticleFormat) => {
138141

139142
const invertedFontLineHeight = css`
140143
line-height: 2.1875rem;
144+
141145
${from.tablet} {
142146
line-height: 2.625rem;
143147
}
@@ -146,6 +150,7 @@ const invertedFontLineHeight = css`
146150
const labsFont = css`
147151
${textSansBold28};
148152
line-height: 2rem;
153+
149154
${from.tablet} {
150155
${textSansBold34};
151156
line-height: 2.375rem;
@@ -155,6 +160,7 @@ const labsFont = css`
155160
const labsGalleryFont = css`
156161
${textSansBold34};
157162
line-height: 2.1875rem;
163+
158164
${from.desktop} {
159165
${textSansBold34};
160166
font-size: 50px;
@@ -166,6 +172,7 @@ const jumboLabsFont = css`
166172
${textSansBold34};
167173
font-size: 3.125rem;
168174
line-height: 3.5rem;
175+
169176
${until.desktop} {
170177
${textSansBold34};
171178
line-height: 2.375rem;
@@ -203,12 +210,15 @@ const immersiveStyles = css`
203210
min-height: 112px;
204211
padding-bottom: ${space[6]}px;
205212
padding-left: ${space[1]}px;
213+
206214
${from.mobileLandscape} {
207215
padding-left: ${space[3]}px;
208216
}
217+
209218
${from.tablet} {
210219
padding-left: ${space[1]}px;
211220
}
221+
212222
margin-right: ${space[5]}px;
213223
`;
214224

@@ -242,12 +252,15 @@ const immersiveWrapper = css`
242252
Make sure we vertically align the headline font with the body font
243253
*/
244254
margin-left: 6px;
255+
245256
${from.tablet} {
246257
margin-left: 16px;
247258
}
259+
248260
${from.leftCol} {
249261
margin-left: 25px;
250262
}
263+
251264
/*
252265
We need this grow to ensure the headline fills the main content column
253266
*/
@@ -257,6 +270,7 @@ const immersiveWrapper = css`
257270
box that extends the black background to the right
258271
*/
259272
z-index: ${getZIndex('articleHeadline')};
273+
260274
${until.mobileLandscape} {
261275
margin-right: 40px;
262276
}
@@ -352,6 +366,7 @@ const decideBottomPadding = ({
352366
if (format.display === ArticleDisplay.Showcase) {
353367
return css`
354368
padding-bottom: ${space[1]}px;
369+
355370
${from.tablet} {
356371
padding-bottom: ${space[6]}px;
357372
}
@@ -364,7 +379,8 @@ const decideBottomPadding = ({
364379
case ArticleDesign.Review: {
365380
if (format.display === ArticleDisplay.Showcase) {
366381
return css`
367-
padding-bottom: 28px;
382+
padding-bottom: ${space[5]}px;
383+
368384
${from.tablet} {
369385
padding-bottom: ${space[6]}px;
370386
}
@@ -383,7 +399,8 @@ const decideBottomPadding = ({
383399
return hasAvatar
384400
? ''
385401
: css`
386-
padding-bottom: 28px;
402+
padding-bottom: ${space[5]}px;
403+
387404
${from.tablet} {
388405
padding-bottom: ${space[9]}px;
389406
}
@@ -396,7 +413,7 @@ const decideBottomPadding = ({
396413
const galleryStyles = css`
397414
${grid.between('grid-start', 'centre-column-end')}
398415
399-
grid-row: 7/9;
416+
grid-row: 7 / 9;
400417
position: relative;
401418
402419
${from.tablet} {
@@ -413,6 +430,8 @@ export const ArticleHeadline = ({
413430
webPublicationDateDeprecated,
414431
hasAvatar,
415432
isMatch,
433+
starRating,
434+
isInStarRatingVariant,
416435
}: Props) => {
417436
switch (format.display) {
418437
case ArticleDisplay.Immersive: {
@@ -496,6 +515,14 @@ export const ArticleHeadline = ({
496515
{headlineString}
497516
</span>
498517
</h1>
518+
{!isUndefined(starRating) &&
519+
isInStarRatingVariant && (
520+
<StarRating
521+
rating={starRating}
522+
paddingSize={'large'}
523+
size={'large'}
524+
/>
525+
)}
499526
</WithAgeWarning>
500527
);
501528
}
@@ -572,6 +599,14 @@ export const ArticleHeadline = ({
572599
>
573600
{headlineString}
574601
</h1>
602+
{!isUndefined(starRating) &&
603+
isInStarRatingVariant && (
604+
<StarRating
605+
rating={starRating}
606+
paddingSize={'medium'}
607+
size={'large'}
608+
/>
609+
)}
575610
</WithAgeWarning>
576611
</div>
577612
);

‎dotcom-rendering/src/components/ArticlePage.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ export const ArticlePage = (props: WebProps | AppProps) => {
7878
<Lightbox
7979
format={format}
8080
switches={frontendData.config.switches}
81+
isInStarRatingVariant={
82+
frontendData.config.abTests.starRatingRedesignVariant ===
83+
'variant'
84+
}
8185
{...(renderingTarget === 'Web'
8286
? {
8387
lightboxImages: frontendData.imagesForLightbox,

‎dotcom-rendering/src/components/Caption.stories.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
Pillar,
1010
} from '../lib/articleFormat';
1111
import { Caption } from './Caption';
12-
import { StarRating } from './StarRating/StarRating';
12+
import { StarRatingDeprecated } from './StarRating/StarRatingDeprecated';
1313

1414
const meta = {
1515
component: Caption,
@@ -191,7 +191,7 @@ export const WhenOverlaidWithStars = {
191191
color: ${palette.neutral[7]};
192192
`}
193193
>
194-
<StarRating rating={3} size="large" />
194+
<StarRatingDeprecated rating={3} size="large" />
195195
</div>
196196
</>
197197
),

‎dotcom-rendering/src/components/Card/Card.tsx‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { DISCUSSION_ID_DATA_ATTRIBUTE } from '../../lib/useCommentCount';
2222
import { BETA_CONTAINERS } from '../../model/enhanceCollections';
2323
import { palette } from '../../palette';
2424
import type { Branding } from '../../types/branding';
25-
import type { StarRating as Rating } from '../../types/content';
25+
import type { StarRating as Rating, RatingSizeType } from '../../types/content';
2626
import type {
2727
AspectRatio,
2828
DCRContainerPalette,
@@ -50,6 +50,7 @@ import { SlideshowCarousel } from '../SlideshowCarousel.importable';
5050
import { Snap } from '../Snap';
5151
import { SnapCssSandbox } from '../SnapCssSandbox';
5252
import { StarRating } from '../StarRating/StarRating';
53+
import { StarRatingDeprecated } from '../StarRating/StarRatingDeprecated';
5354
import type { Alignment } from '../SupportingContent';
5455
import { SupportingContent } from '../SupportingContent';
5556
import { SvgMediaControlsPlay } from '../SvgMediaControlsPlay';
@@ -163,6 +164,8 @@ export type Props = {
163164
/** Determines if the headline should be positioned within the content or outside the content */
164165
headlinePosition?: 'inner' | 'outer';
165166
enableHls?: boolean;
167+
isInStarRatingVariant?: boolean;
168+
starRatingSize?: RatingSizeType;
166169
};
167170

168171
const starWrapper = (cardHasImage: boolean) => css`
@@ -184,7 +187,7 @@ const StarRatingComponent = ({
184187
cardHasImage: boolean;
185188
}) => (
186189
<div css={starWrapper(cardHasImage)}>
187-
<StarRating rating={rating} size="small" />
190+
<StarRatingDeprecated rating={rating} size="small" />
188191
</div>
189192
);
190193

@@ -421,6 +424,8 @@ export const Card = ({
421424
headlinePosition = 'inner',
422425
subtitleSize = 'small',
423426
enableHls = false,
427+
isInStarRatingVariant,
428+
starRatingSize = 'small',
424429
}: Props) => {
425430
const hasSublinks = supportingContent && supportingContent.length > 0;
426431
const sublinkPosition = decideSublinkPosition(
@@ -908,12 +913,18 @@ export const Card = ({
908913
showByline={showByline}
909914
isExternalLink={isExternalLink}
910915
/>
911-
{!isUndefined(starRating) ? (
912-
<StarRatingComponent
913-
rating={starRating}
914-
cardHasImage={!!image}
915-
/>
916-
) : null}
916+
{!isUndefined(starRating) &&
917+
(isInStarRatingVariant ? (
918+
<StarRating
919+
rating={starRating}
920+
size={starRatingSize}
921+
/>
922+
) : (
923+
<StarRatingComponent
924+
rating={starRating}
925+
cardHasImage={!!image}
926+
/>
927+
))}
917928
</div>
918929
)}
919930

@@ -1238,12 +1249,19 @@ export const Card = ({
12381249
: undefined
12391250
}
12401251
/>
1241-
{!isUndefined(starRating) ? (
1242-
<StarRatingComponent
1243-
rating={starRating}
1244-
cardHasImage={!!image}
1245-
/>
1246-
) : null}
1252+
1253+
{!isUndefined(starRating) &&
1254+
(isInStarRatingVariant ? (
1255+
<StarRating
1256+
rating={starRating}
1257+
size={starRatingSize}
1258+
/>
1259+
) : (
1260+
<StarRatingComponent
1261+
rating={starRating}
1262+
cardHasImage={!!image}
1263+
/>
1264+
))}
12471265
</HeadlineWrapper>
12481266
)}
12491267

‎dotcom-rendering/src/components/Carousel.importable.tsx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ type Props = {
4545
discussionApiUrl: string;
4646
serverTime?: number;
4747
renderingTarget: RenderingTarget;
48+
isInStarRatingVariant?: boolean;
4849
};
4950

5051
type ArticleProps = Props & {
@@ -464,6 +465,7 @@ type CarouselCardProps = {
464465
onwardsSource?: OnwardsSource;
465466
containerType?: DCRContainerType;
466467
starRating?: StarRating;
468+
isInStarRatingVariant?: boolean;
467469
};
468470

469471
const CarouselCard = ({
@@ -486,6 +488,7 @@ const CarouselCard = ({
486488
serverTime,
487489
starRating,
488490
index,
491+
isInStarRatingVariant,
489492
}: CarouselCardProps) => {
490493
const cardImagePosition = isOnwardContent ? 'bottom' : 'top';
491494

@@ -533,6 +536,7 @@ const CarouselCard = ({
533536
showTopBarDesktop={!isOnwardContent}
534537
showTopBarMobile={!isOnwardContent}
535538
aspectRatio={'5:4'}
539+
isInStarRatingVariant={isInStarRatingVariant}
536540
/>
537541
</LI>
538542
);
@@ -753,6 +757,7 @@ export const Carousel = ({
753757
isOnwardContent = true,
754758
serverTime,
755759
renderingTarget,
760+
isInStarRatingVariant,
756761
...props
757762
}: ArticleProps | FrontProps) => {
758763
const carouselRef = useRef<HTMLUListElement>(null);
@@ -990,6 +995,9 @@ export const Carousel = ({
990995
discussionApiUrl={discussionApiUrl}
991996
isOnwardContent={isOnwardContent}
992997
starRating={starRating}
998+
isInStarRatingVariant={
999+
isInStarRatingVariant
1000+
}
9931001
/>
9941002
);
9951003
})}

0 commit comments

Comments
 (0)