Star rating redesign #15032
Star rating redesign #15032
Conversation
…asier to add a 0% switch for testing over festive period
…be controlled via test
…is not in variant
…r accessibility requirments. These are needed for cards such as feature cards which have lighter / variable backgrounds
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
| width: (data.imageAsset ?? fallbackData.imageAsset).fields.width, | ||
| height: (data.imageAsset ?? fallbackData.imageAsset).fields.height, | ||
| } satisfies Parameters<typeof RichLink>[0]['imageData']; | ||
| console.log('rich link component', isInStarRatingVariant); |
There was a problem hiding this comment.
Do we still need this log?
| '1', | ||
| article.config.abTests, | ||
| article.config.abTests.starRatingRedesignVariant, | ||
| ); |
| </> | ||
| ); | ||
| case 'model.dotcomrendering.pageElements.RichLinkBlockElement': | ||
| console.log('render element', abTests); |
|
Looks good @abeddow91 ❤️ Since the new Star Rating design forms part of Source core components, would it make sense to build this component into the Source React component library? This would help align the design-developer handoff for future changes and ensure the component is not tightly coupled to dotcom-rendering |
| rating: Rating; | ||
| size: RatingSizeType; | ||
| paddingSize?: PaddingSizeType; | ||
| /** The dark theme is to account for star ratings that appear on lighter / translucent backgrounds (eg feature cards). The dark theme ensures we meet AA accessibility standard*/ |
There was a problem hiding this comment.
Could this comment be split over multiple lines?
| // ----- Imports ----- // | ||
| import type { Meta } from '@storybook/react-webpack5'; | ||
| import { StarRating } from './StarRating'; | ||
| import { StarRatingDeprecated } from './StarRatingDeprecated'; |
There was a problem hiding this comment.
Can stories be added for the new StarRating component? (In addition to leaving these deprecated ones up for the time being)
| size: RatingSizeType; | ||
| paddingSize?: PaddingSizeType; | ||
| /** The dark theme is to account for star ratings that appear on lighter / translucent backgrounds (eg feature cards). The dark theme ensures we meet AA accessibility standard*/ | ||
| useDarkTheme?: boolean; |
There was a problem hiding this comment.
I agree with your comment on "dark theme" being an overloaded term. I think this can be easily confused with styles that are applied when the user is in dark mode. I think even a less descriptive word such as "secondary" or "alternative" paired with your comment would be better.
| css={[starsWrapper, !isInStarRatingVariant && starWrapperColour]} | ||
| data-spacefinder-role="inline" | ||
| > | ||
| <StarRatingDeprecated rating={rating} size={size} /> |
There was a problem hiding this comment.
How will this work once we remove the test? Will this component be swapped out for the new <StarRating /> component?
There was a problem hiding this comment.
Or will we not need it at all?
| isInStarRatingVariant={ | ||
| isInStarRatingVariant | ||
| } | ||
| starRating={article.starRating} |
There was a problem hiding this comment.
Why does the StarRating component now get called from within the ArticleHeadline component?
| )} | ||
|
|
||
| { | ||
| /* TODO : Remove this star rating once the starRatingVariant testing is complete and new designs are merged (eta Jan 2026)*/ |
There was a problem hiding this comment.
Will we still be using star ratings in lightbox images once the new designs are merged?
domlander
left a comment
domlander
left a comment
There was a problem hiding this comment.
This looks great Anna, thanks for writing all this code!
There's a few comments that need to be deleted as SImon pointed out and it would be great if a few more stories could be added in places we have the new star ratings.
What does this change?
Introduces the redesigned star rating component behind a 0% opt in test.
As part of the redesign the component has been refactored.
It now features
Why?
The star rating component is used site wide. By using a 0% opt in test, we can ensure design and engineering are given ample time to QA this change, especially given we are approaching the festive period.
To allow for the opt in test, the legacy star rating component has been deprecated and a new component has been added. This should make clean up easier once the the designs are fully approved and ready to roll out site wide.
Screenshots