Skip to content

Commit

Permalink
Merge branch 'main' into ab/podcast-accent-image
Browse files Browse the repository at this point in the history
  • Loading branch information
abeddow91 authored Jan 7, 2025
2 parents cc15cf2 + 1a4b51b commit e61d697
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 122 deletions.
26 changes: 0 additions & 26 deletions .github/workflows/amp.yml

This file was deleted.

8 changes: 1 addition & 7 deletions .github/workflows/cicd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,12 @@ jobs:
with:
container-image: ${{ needs.container.outputs.container-image }}

amp:
needs: [container]
uses: ./.github/workflows/amp.yml
with:
container-image: ${{ needs.container.outputs.container-image }}

publish:
permissions:
id-token: write
contents: read
pull-requests: write # required by riff-raff action
needs: [container, prettier, jest, lint, playwright, amp]
needs: [container, prettier, jest, lint, playwright]
uses: ./.github/workflows/publish.yml
with:
container-image: ${{ needs.container.outputs.container-image }}
Expand Down
4 changes: 0 additions & 4 deletions dotcom-rendering/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ playwright-open: clear clean-dist install build
$(call log, "starting DEV server and opening Playwright UI")
@pnpm playwright:open

ampValidation: clean-dist install
$(call log, "starting AMP Validation test")
@node scripts/test/amp-validation.js

buildCheck:
$(call log, "checking build files")
@node ./scripts/test/build-check.js
Expand Down
20 changes: 0 additions & 20 deletions dotcom-rendering/playwright/tests/article.e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,25 +96,5 @@ test.describe('E2E Page rendering', () => {
});
});

test.describe('for AMP', function () {
test(`It should load render an AMP page`, async ({ page }) => {
await loadPage(
page,
`/AMPArticle/https://amp.theguardian.com/commentisfree/2019/oct/16/impostor-syndrome-class-unfairness`,
);

await expect(page.locator('header').first()).toContainText(
'Opinion',
);

// In this AMP Article we'd expect three advert slots to be inserted
// (note each of these slots will itself contain a regional slot, that will only appear in certain geos)
// And each to have the follow IDs
await expect(page.locator('#ad-1')).toBeAttached();
await expect(page.locator('#ad-2')).toBeAttached();
await expect(page.locator('#ad-3')).toBeAttached();
});
});

// TODO e2e add skipped tests from article.e2e.cy.js
});
22 changes: 0 additions & 22 deletions dotcom-rendering/playwright/tests/article.embeds.e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,6 @@ import { loadPage } from '../lib/load-page';
import { expectToBeVisible } from '../lib/locators';

test.describe('Embeds', () => {
test.describe('AMP', () => {
test('should render the corona interactive atom embed', async ({
page,
}) => {
await loadPage(
page,
'/AMPArticle/https://www.theguardian.com/world/2020/apr/24/new-mother-dies-of-coronavirus-six-days-after-giving-birth',
);
await cmpAcceptAll(
page,
'amp-consent > iframe[src*="sourcepoint"]',
);

await expect(
await getIframeBody(
page,
'amp-iframe[data-testid="atom-embed-url"] > iframe',
),
).toContainText('Daily cases');
});
});

test.describe('WEB', function () {
test('should render the click to view overlay revealing the embed when clicked', async ({
page,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const getAuthStatus = getAuthStatus_ as jest.MockedFunction<
const PERSISTENCE_KEYS = {
USER_FEATURES_EXPIRY_COOKIE: 'gu_user_features_expiry',
AD_FREE_USER_COOKIE: 'GU_AF1',
ACTION_REQUIRED_FOR_COOKIE: 'gu_action_required_for',
SUPPORT_ONE_OFF_CONTRIBUTION_COOKIE: 'gu.contributions.contrib-timestamp',
HIDE_SUPPORT_MESSAGING_COOKIE: 'gu_hide_support_messaging',
};
Expand All @@ -62,16 +61,11 @@ const setAllFeaturesData = (opts: { isExpired: boolean }) => {
name: PERSISTENCE_KEYS.USER_FEATURES_EXPIRY_COOKIE,
value: expiryDate.getTime().toString(),
});
setCookie({
name: PERSISTENCE_KEYS.ACTION_REQUIRED_FOR_COOKIE,
value: 'test',
});
};

const deleteAllFeaturesData = () => {
removeCookie({ name: PERSISTENCE_KEYS.USER_FEATURES_EXPIRY_COOKIE });
removeCookie({ name: PERSISTENCE_KEYS.AD_FREE_USER_COOKIE });
removeCookie({ name: PERSISTENCE_KEYS.ACTION_REQUIRED_FOR_COOKIE });
removeCookie({ name: PERSISTENCE_KEYS.HIDE_SUPPORT_MESSAGING_COOKIE });
};

Expand Down
11 changes: 0 additions & 11 deletions dotcom-rendering/src/client/userFeatures/user-features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
import type { UserFeaturesResponse } from './user-features-lib';

const USER_FEATURES_EXPIRY_COOKIE = 'gu_user_features_expiry';
const ACTION_REQUIRED_FOR_COOKIE = 'gu_action_required_for';
const HIDE_SUPPORT_MESSAGING_COOKIE = 'gu_hide_support_messaging';
const AD_FREE_USER_COOKIE = 'GU_AF1';

Expand All @@ -36,7 +35,6 @@ const forcedAdFreeMode = !!/[#&]noadsaf(&.*)?$/.exec(window.location.hash);
const userHasData = () => {
const cookie =
getAdFreeCookie() ??
getCookie({ name: ACTION_REQUIRED_FOR_COOKIE }) ??
getCookie({ name: USER_FEATURES_EXPIRY_COOKIE }) ??
getCookie({ name: HIDE_SUPPORT_MESSAGING_COOKIE });
return !!cookie;
Expand Down Expand Up @@ -65,14 +63,6 @@ const persistResponse = (JsonResponse: UserFeaturesResponse) => {
value: String(!JsonResponse.showSupportMessaging),
});

removeCookie({ name: ACTION_REQUIRED_FOR_COOKIE });
if (JsonResponse.alertAvailableFor) {
setCookie({
name: ACTION_REQUIRED_FOR_COOKIE,
value: JsonResponse.alertAvailableFor,
});
}

if (JsonResponse.contentAccess.digitalPack) {
setAdFreeCookie(2);
} else if (adFreeDataIsPresent() && !forcedAdFreeMode) {
Expand All @@ -83,7 +73,6 @@ const persistResponse = (JsonResponse: UserFeaturesResponse) => {
const deleteOldData = (): void => {
removeCookie({ name: AD_FREE_USER_COOKIE });
removeCookie({ name: USER_FEATURES_EXPIRY_COOKIE });
removeCookie({ name: ACTION_REQUIRED_FOR_COOKIE });
removeCookie({ name: HIDE_SUPPORT_MESSAGING_COOKIE });
};

Expand Down
29 changes: 12 additions & 17 deletions dotcom-rendering/src/components/Card/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -459,11 +459,6 @@ 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' && !canPlayInline && showMainVideo;

const media = getMedia({
imageUrl: image?.src,
imageAltText: image?.altText,
Expand Down Expand Up @@ -683,7 +678,6 @@ export const Card = ({
imageType={media.type}
imagePositionOnDesktop={imagePositionOnDesktop}
imagePositionOnMobile={imagePositionOnMobile}
showPlayIcon={showPlayIcon}
hideImageOverlay={
media.type === 'slideshow' && isFlexibleContainer
}
Expand Down Expand Up @@ -835,17 +829,18 @@ export const Card = ({
roundedCorners={isOnwardContent}
aspectRatio={aspectRatio}
/>
{showPlayIcon && mainMedia.duration > 0 && (
<MediaDuration
mediaDuration={mainMedia.duration}
imagePositionOnDesktop={
imagePositionOnDesktop
}
imagePositionOnMobile={
imagePositionOnMobile
}
/>
)}
{mainMedia?.type === 'Video' &&
mainMedia.duration > 0 && (
<MediaDuration
mediaDuration={mainMedia.duration}
imagePositionOnDesktop={
imagePositionOnDesktop
}
imagePositionOnMobile={
imagePositionOnMobile
}
/>
)}
</>
)}
{media.type === 'crossword' && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { SerializedStyles } from '@emotion/react';
import { css } from '@emotion/react';
import { between, from, space, until } from '@guardian/source/foundations';
import type { CardImageType } from '../../../types/layout';
import { PlayIcon } from './PlayIcon';

const imageFixedSize = {
tiny: 86,
Expand Down Expand Up @@ -35,7 +34,6 @@ type Props = {
imageType?: CardImageType;
imagePositionOnDesktop: ImagePositionType;
imagePositionOnMobile: ImagePositionType;
showPlayIcon: boolean;
/**
* Forces hiding the image overlay added to pictures & slideshows on hover.
* This is to allow hiding the overlay on slideshow carousels where we don't
Expand Down Expand Up @@ -127,7 +125,6 @@ export const ImageWrapper = ({
imageType,
imagePositionOnDesktop,
imagePositionOnMobile,
showPlayIcon,
hideImageOverlay,
padImage,
}: Props) => {
Expand Down Expand Up @@ -186,12 +183,6 @@ export const ImageWrapper = ({
{/* This image overlay is styled when the CardLink is hovered */}
{(imageType === 'picture' || imageType === 'slideshow') &&
!hideImageOverlay && <div className="image-overlay" />}
{imageType === 'picture' && showPlayIcon && (
<PlayIcon
imageSize={imageSize}
imagePositionOnMobile={imagePositionOnMobile}
/>
)}
</>
</div>
);
Expand Down

0 comments on commit e61d697

Please sign in to comment.