-
Notifications
You must be signed in to change notification settings - Fork 35
Add onward content to Hosted Gallery pages #16323
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import { css } from '@emotion/react'; | |
| import { isUndefined } from '@guardian/libs'; | ||
| import { between, from, space, until } from '@guardian/source/foundations'; | ||
| import { grid } from '../grid'; | ||
| import { type ArticleFormat } from '../lib/articleFormat'; | ||
| import { ArticleDesign, type ArticleFormat } from '../lib/articleFormat'; | ||
| import { getImage } from '../lib/image'; | ||
| import { palette } from '../palette'; | ||
| import type { ImageBlockElement } from '../types/content'; | ||
|
|
@@ -47,6 +47,12 @@ const styles = css` | |
| } | ||
| `; | ||
|
|
||
| const hostedGalleryOverrides = css` | ||
| ${between.desktop.and.leftCol} { | ||
| ${grid.centreRule(2, 'transparent')} | ||
| } | ||
| `; | ||
|
Comment on lines
+50
to
+54
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| const galleryBodyImageStyles = css` | ||
| display: inline; | ||
| position: relative; | ||
|
|
@@ -87,7 +93,13 @@ export const GalleryImage = ({ | |
| } | ||
|
|
||
| return ( | ||
| <figure css={styles}> | ||
| <figure | ||
| css={[ | ||
| styles, | ||
| format.design === ArticleDesign.HostedGallery && | ||
| hostedGalleryOverrides, | ||
| ]} | ||
| > | ||
| <div | ||
| css={galleryBodyImageStyles} | ||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { css } from '@emotion/react'; | ||
| import { | ||
| palette as sourcePalette, | ||
| from, | ||
| space, | ||
| textSans17, | ||
| textSansBold20, | ||
|
|
@@ -12,25 +12,30 @@ import { HostedContentOnwardsCard } from './HostedContentOnwardsCard'; | |
| type HostedContentOnwardsProps = { | ||
| trails: TrailType[]; | ||
| brandName: string; | ||
| isGalleryPage: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Override --accent-colour variable at a higher CSS specificity | ||
| * for hosted gallery articles only, because this only has a dark design | ||
| */ | ||
| const galleryOverrides = css` | ||
| --accent-colour: ${palette('--onward-text')}; | ||
| `; | ||
|
Comment on lines
+18
to
+24
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to colour contrast issues, we suppress the accent colour from appearing in most dark mode hosted pages. Galleries only ever have "dark mode" so we need to do this in a more permanent way. A simple way to do this is to override the previous one set at the page level by adding a higher specificity colour variable which remains the same between dark and light modes |
||
|
|
||
| const headerStyles = css` | ||
| margin-bottom: ${space[1]}px; | ||
| border-top: ${space[2]}px solid | ||
| var(--accent-colour, ${sourcePalette.neutral[86]}); | ||
| border-top: ${space[2]}px solid var(--accent-colour); | ||
| `; | ||
|
|
||
| const headingStyles = css` | ||
| ${textSans17} | ||
| padding-top: ${space[2]}px; | ||
| color: ${palette('--hosted-content-onwards-heading')}; | ||
| padding-bottom: ${space[2]}px; | ||
| color: ${palette('--onward-text')}; | ||
|
|
||
| @media (prefers-color-scheme: dark) { | ||
| color: ${palette('--hosted-content-onwards-heading')}; | ||
| } | ||
|
|
||
| [data-color-scheme='dark'] & { | ||
| color: ${palette('--hosted-content-onwards-heading')}; | ||
| color: ${palette('--onward-text')}; | ||
| } | ||
|
|
||
| span { | ||
|
|
@@ -47,37 +52,71 @@ const stackedCardsStyles = css` | |
|
|
||
| const stackedCardWrapper = css` | ||
| width: 100%; | ||
| border-top: 2px solid ${palette('--onward-content-border')}; | ||
| padding-top: ${space[2]}px; | ||
| padding-bottom: ${space[2]}px; | ||
| border-top: 1px solid ${palette('--article-border')}; | ||
| padding: ${space[2]}px 0; | ||
|
|
||
| &:last-of-type { | ||
| padding-bottom: 0; | ||
| padding: ${space[2]}px 0 0 0; | ||
| } | ||
| `; | ||
|
|
||
| const rowCardWrapper = css` | ||
| ${stackedCardWrapper} | ||
| ${from.desktop} { | ||
| width: 100%; | ||
| border-top: none; | ||
| border-right: 1px solid ${palette('--article-border')}; | ||
| padding: 0 ${space[2]}px; | ||
|
|
||
| &:first-of-type { | ||
| padding: 0 ${space[2]}px 0 0; | ||
| } | ||
| &:last-of-type { | ||
| border-right: none; | ||
| padding: 0 0 0 ${space[2]}px; | ||
| } | ||
| } | ||
| `; | ||
|
|
||
| const galleryStyles = css` | ||
| ${from.desktop} { | ||
| flex-direction: row; | ||
| } | ||
| `; | ||
|
|
||
| export const HostedContentOnwards = ({ | ||
| trails, | ||
| brandName, | ||
| isGalleryPage = false, | ||
| }: HostedContentOnwardsProps) => { | ||
| return ( | ||
| <> | ||
| <div css={isGalleryPage && galleryOverrides}> | ||
| <header css={headerStyles}> | ||
| <h2 css={headingStyles}> | ||
| More from | ||
| <span>{brandName}</span> | ||
| </h2> | ||
| </header> | ||
|
|
||
| <ul css={stackedCardsStyles}> | ||
| <ul css={[stackedCardsStyles, isGalleryPage && galleryStyles]}> | ||
| {trails.map((trail) => { | ||
| return ( | ||
| <li key={trail.url} css={stackedCardWrapper}> | ||
| <HostedContentOnwardsCard trail={trail} /> | ||
| <li | ||
| key={trail.url} | ||
| css={ | ||
| isGalleryPage | ||
| ? rowCardWrapper | ||
| : stackedCardWrapper | ||
| } | ||
| > | ||
| <HostedContentOnwardsCard | ||
| trail={trail} | ||
| isGalleryPage={isGalleryPage} | ||
| /> | ||
| </li> | ||
| ); | ||
| })} | ||
| </ul> | ||
| </> | ||
| </div> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,15 @@ | ||
| import { css } from '@emotion/react'; | ||
| import { space, textSansBold15 } from '@guardian/source/foundations'; | ||
| import { getZIndex } from '../lib/getZIndex'; | ||
| import { generateImageURL } from '../lib/image'; | ||
| import { palette } from '../palette'; | ||
| import type { TrailType } from '../types/trails'; | ||
|
|
||
| type Props = { | ||
| trail: TrailType; | ||
| isGalleryPage?: boolean; | ||
| }; | ||
|
|
||
| type CardPictureProps = { | ||
| image: string; | ||
| alt: string; | ||
| }; | ||
|
|
||
| const imageStyles = css` | ||
| width: 120px; | ||
| `; | ||
|
|
||
| const mediaOverlayContainerStyles = css` | ||
| position: absolute; | ||
| top: 0; | ||
|
|
@@ -61,28 +54,30 @@ const headingStyles = css` | |
| color: ${palette('--card-headline')}; | ||
| `; | ||
|
|
||
| const CardPicture = ({ image, alt }: CardPictureProps) => { | ||
| return ( | ||
| <> | ||
| <picture> | ||
| <img alt={alt} src={image} css={imageStyles} /> | ||
| </picture> | ||
| <div css={mediaOverlayContainerStyles}> | ||
| <div className="media-overlay" /> | ||
| </div> | ||
| </> | ||
| ); | ||
| }; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this to be inline instead of a separate component as I feel it's slightly easier to follow this way |
||
|
|
||
| export const HostedContentOnwardsCard = ({ trail }: Props) => { | ||
| export const HostedContentOnwardsCard = ({ | ||
| trail, | ||
| isGalleryPage = false, | ||
| }: Props) => { | ||
| return ( | ||
| <a href={trail.url} css={linkStyles}> | ||
| <h3 css={headingStyles}>{trail.headline}</h3> | ||
| {!!trail.image && ( | ||
| <CardPicture | ||
| image={trail.image.src} | ||
| alt={trail.image.altText || ''} | ||
| /> | ||
| <> | ||
| <picture> | ||
| <img | ||
| alt={trail.image.altText} | ||
| src={generateImageURL({ | ||
| mainImage: trail.image.src, | ||
| imageWidth: isGalleryPage ? 180 : 120, | ||
| resolution: 'low', | ||
| aspectRatio: '5:4', | ||
| })} | ||
|
Comment on lines
+69
to
+74
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requests the correct image size from Fastly rather than using the trail image URL directly. This allows us to control the aspect ratio and desired resolution of the images |
||
| /> | ||
| </picture> | ||
| <div css={mediaOverlayContainerStyles}> | ||
| <div className="media-overlay" /> | ||
| </div> | ||
| </> | ||
| )} | ||
| </a> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,25 @@ import { hostedPaletteDecorator } from '../../.storybook/decorators/themeDecorat | |
| import { allModes } from '../../.storybook/modes'; | ||
| import preview from '../../.storybook/preview'; | ||
| import { hostedGallery } from '../../fixtures/manual/hostedGallery'; | ||
| import { hostedOnwardsTrails } from '../../fixtures/manual/onwardsTrails'; | ||
| import { | ||
| ArticleDesign, | ||
| ArticleDisplay, | ||
| ArticleSpecial, | ||
| } from '../lib/articleFormat'; | ||
| import { customMockFetch } from '../lib/mockRESTCalls'; | ||
| import { enhanceArticleType } from '../types/article'; | ||
| import { HostedGalleryLayout } from './HostedGalleryLayout'; | ||
|
|
||
| const mockOnwardsContentFetch = customMockFetch([ | ||
| { | ||
| mockedMethod: 'GET', | ||
| mockedUrl: `${hostedGallery.config.ajaxUrl}/${hostedGallery.config.pageId}/onward.json`, | ||
| mockedStatus: 200, | ||
| mockedBody: { trails: hostedOnwardsTrails }, | ||
| }, | ||
| ]); | ||
|
Comment on lines
+15
to
+22
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mocked response of hosted onward content for Storybook stories |
||
|
|
||
| const meta = preview.meta({ | ||
| title: 'Layouts/HostedGallery', | ||
| component: HostedGalleryLayout, | ||
|
|
@@ -21,6 +32,10 @@ const meta = preview.meta({ | |
| }, | ||
| }, | ||
| }, | ||
| render: (args) => { | ||
| global.fetch = mockOnwardsContentFetch; | ||
| return <HostedGalleryLayout {...args} />; | ||
| }, | ||
| }); | ||
|
|
||
| const { hostedCampaignColour = '' } = | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lift and shift to make the CSS easier to read:
small screen -> large screen CSS ordering