Skip to content

Commit

Permalink
Merge branch 'main' into showPreview-lift-up
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielCliftonGuardian authored Jan 26, 2024
2 parents d4b518b + 7675140 commit 2154f88
Show file tree
Hide file tree
Showing 26 changed files with 846 additions and 843 deletions.
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: 🐞 Bug Report
description: Let us know about something unexpected that has happened
title: '[Bug]: '
labels: ['bug', 'Rota']
labels: ['Bug', 'Rota']
projects: ['guardian/88']
body:
- type: markdown
Expand Down
18 changes: 0 additions & 18 deletions .github/ISSUE_TEMPLATE/request-for-change.md

This file was deleted.

18 changes: 18 additions & 0 deletions .github/ISSUE_TEMPLATE/request-for-change.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
name: DCR Request for comments
description: Tell us about a feature that you're planning to implement in dotcom rendering
title: '[RFC]: '
labels: ['RFC']
projects: ['guardian/88']
body:
- type: markdown
attributes:
value: |
The Dotcom team would like to help you with your proposed change. Please provide us with a brief and concise explanation for what you're looking to do we will try to make the process easier with early feedback or advice.
**When should an RFC be used?**
This is an optional process. If you own the code you're working on or feel confident about the change you're making then you should simply raise a PR as normal. We want developers and teams to feel empowered to make the choice about when to reach out.
However, if your change affects core platform code or is introducing a new concept or pattern, or if you're just looking for more eyes and input then this process is for you.
**Who is responsible for completing this RFC process?**
Once the RFC is raised, it is the responsibility of the _Dotcom team_ (@guardian/dotcom-platform) to provide feedback and help you get your change to the finish line.
61 changes: 31 additions & 30 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,47 +9,48 @@ updates:
directory: '/'
schedule:
interval: 'daily'
# Set to minimise the number of chromatic runs.
rebase-strategy: 'disabled'

ignore:
# The version of the aws-cdk[-lib] & constructs dependencies should match exactly the version specified by @guardian/cdk
# The version of the aws-cdk[-lib] & constructs dependencies should match
# exactly the version specified by @guardian/cdk
- dependency-name: 'aws-cdk'
- dependency-name: 'aws-cdk-lib'
- dependency-name: 'constructs'

open-pull-requests-limit: 10
groups:
# Most storybook dependencies are released with synchronised versions
# and therefore should be updated together.
storybook:
babel:
patterns:
- '@storybook/*'
- 'storybook'
- package-ecosystem: 'npm'
directory: '/apps-rendering'
schedule:
interval: 'daily'
labels:
- 'AR Dependency'
rebase-strategy: 'disabled'
ignore:
# The version of the aws-cdk[-lib] & constructs dependencies should match exactly the version specified by @guardian/cdk
- dependency-name: 'aws-cdk'
- dependency-name: 'aws-cdk-lib'
- dependency-name: 'constructs'
# Types should match major and minor versions of the package being used.
- dependency-name: '@types/node'
update-types:
- 'version-update:semver-major'
- 'version-update:semver-minor'
open-pull-requests-limit: 7
groups:
# Most storybook dependencies are released with synchronised versions
# and therefore should be updated together.
- '*babel*'
eslint:
patterns:
- '*eslint*'
guardian:
patterns:
- '@guardian/*'
jest:
patterns:
- 'jest*'
react:
patterns:
- 'react*'
remark:
patterns:
- 'remark*'
storybook:
patterns:
- '@storybook/*'
- 'storybook'
- '*storybook*'
swc:
patterns:
- '@swc/*'
testing-library:
patterns:
- '@testing-library/*'
webpack:
patterns:
- '*webpack*'
- '*-loader'
- package-ecosystem: 'github-actions'
directory: '/'
schedule:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ exports[`The RenderingCDKStack matches the snapshot 1`] = `
"Namespace": "AWS/ApplicationELB",
"Period": 30,
"Statistic": "Average",
"Threshold": 0.15,
"Threshold": 0.12,
},
"Type": "AWS::CloudWatch::Alarm",
},
Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/cdk/lib/renderingStack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,12 @@ export class RenderingCDKStack extends CDKStack {
{
// No scaling down effect when latency is higher than 0.15s
change: 0,
lower: 0.15,
lower: 0.12,
},
{
// When latency is lower than 0.15s we scale down by 1
change: -1,
upper: 0.15,
upper: 0.12,
lower: 0,
},
],
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/configs/webpack/client.apps.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,5 @@ export default merge(base(import.meta.url), {
// We are making "ophan-tracker-js" external to the apps bundle
// because we never expect to use it in apps pages.
// Tracking is done natively.
externals: { 'ophan-tracker-js': 'ophan-tracker-js' },
externals: { '@guardian/ophan-tracker-js': 'guardian.ophan' },
});
1 change: 1 addition & 0 deletions dotcom-rendering/fixtures/generated/articles/Live.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4585,6 +4585,7 @@ export const Live: DCRArticle = {
inizio: true,
prebidHeaderBidding: true,
a9HeaderBidding: true,
lightbox: true,
},
keywordIds:
'environment/climate-change,environment/environment,science/scienceofclimatechange,science/science,world/eu,world/europe-news,world/world,environment/flooding,world/wildfires,world/natural-disasters',
Expand Down
1 change: 1 addition & 0 deletions dotcom-rendering/fixtures/generated/articles/PhotoEssay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9017,6 +9017,7 @@ export const PhotoEssay: DCRArticle = {
inizio: true,
prebidHeaderBidding: true,
a9HeaderBidding: true,
lightbox: true,
},
keywordIds:
'environment/climate-change,environment/environment,science/scienceofclimatechange,science/science,world/eu,world/europe-news,world/world,environment/flooding,world/wildfires,world/natural-disasters',
Expand Down
6 changes: 3 additions & 3 deletions dotcom-rendering/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"build-storybook": "storybook build"
},
"dependencies": {
"@aws-sdk/client-cloudwatch": "3.350.0",
"@aws-sdk/client-cloudwatch": "3.499.0",
"@babel/core": "7.23.2",
"@babel/helper-compilation-targets": "7.20.0",
"@babel/helper-create-regexp-features-plugin": "7.20.5",
Expand All @@ -48,7 +48,7 @@
"@guardian/bridget": "2.6.0",
"@guardian/browserslist-config": "5.0.0",
"@guardian/cdk": "50.13.0",
"@guardian/commercial": "12.0.0",
"@guardian/commercial": "13.0.0",
"@guardian/consent-management-platform": "13.7.1",
"@guardian/core-web-vitals": "6.0.0",
"@guardian/eslint-config": "7.0.1",
Expand All @@ -57,6 +57,7 @@
"@guardian/identity-auth": "1.1.0",
"@guardian/identity-auth-frontend": "1.0.0",
"@guardian/libs": "16.0.1",
"@guardian/ophan-tracker-js": "2.0.4",
"@guardian/shimport": "1.0.2",
"@guardian/source-foundations": "14.1.2",
"@guardian/source-react-components": "18.0.0",
Expand Down Expand Up @@ -181,7 +182,6 @@
"lz-string": "1.5.0",
"minimatch": "5.1.6",
"mockdate": "3.0.5",
"ophan-tracker-js": "2.0.2",
"parse5": "7.1.2",
"postcss-styled-syntax": "0.6.3",
"preact": "10.15.1",
Expand Down
58 changes: 10 additions & 48 deletions dotcom-rendering/playwright/tests/parallel-2/lightbox.e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: { abTests: { lightboxVariant: 'variant' } },
});
await loadPageWithOverrides(page, photoEssayArticle);

await expectToNotBeVisible(page, '#gu-lightbox');

Expand All @@ -100,11 +98,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await expectToNotBeVisible(page, '#gu-lightbox');

Expand All @@ -122,11 +116,7 @@ test.describe('Lightbox', () => {

test('should trap focus', async ({ context, page }) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await page.locator('article img').first().click({ force: true });
await expectToBeVisible(page, '#gu-lightbox');
Expand Down Expand Up @@ -171,11 +161,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await expectToNotBeVisible(page, '#gu-lightbox');

Expand Down Expand Up @@ -263,11 +249,7 @@ test.describe('Lightbox', () => {
}

await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

// eq(6) here means the 7th button is clicked (base zero)
await page.locator('button.open-lightbox').nth(6).click();
Expand Down Expand Up @@ -306,11 +288,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await page.locator('button.open-lightbox').nth(1).click();
await expectToBeVisible(page, '#gu-lightbox');
Expand Down Expand Up @@ -357,11 +335,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await page.locator('button.open-lightbox').nth(1).click();
await expectToBeVisible(page, '#gu-lightbox');
Expand Down Expand Up @@ -403,11 +377,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, LiveBlog, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, LiveBlog);

await page.locator('button.open-lightbox').nth(1).click();
await expectToBeVisible(page, '#gu-lightbox');
Expand All @@ -434,11 +404,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await page.locator('button.open-lightbox').nth(1).click();
await expectToBeVisible(page, '#gu-lightbox');
Expand Down Expand Up @@ -468,11 +434,7 @@ test.describe('Lightbox', () => {
page,
}) => {
await disableCMP(context);
await loadPageWithOverrides(page, photoEssayArticle, {
configOverrides: {
abTests: { lightboxVariant: 'variant' },
},
});
await loadPageWithOverrides(page, photoEssayArticle);

await expectToNotBeVisible(page, '#gu-lightbox');
// Open lightbox using the second button on the page (the first is main media)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const interceptOphanPlayEvent = ({ page, id }: { page: Page; id: string }) => {
return page.waitForRequest((request) => {
const matchUrl = request
.url()
.startsWith('http://ophan.theguardian.com/img/2?');
.startsWith('https://ophan.theguardian.com/img/2?');
const searchParams = new URLSearchParams(request.url());
const videoSearchParam = searchParams.get('video');
const expectedVideoSearchParam = JSON.stringify({
Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/src/client/ophan/ophan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ export const getOphan = async (
return cachedOphan;
}

// We've taken 'ophan-tracker-js' out of the apps client bundle (made it external in webpack) because we don't ever expect this method to be called. Tracking in apps is done natively.
// We've taken '@guardian/ophan-tracker-js' out of the apps client bundle (made it external in webpack) because we don't ever expect this method to be called. Tracking in apps is done natively.
// @ts-expect-error -- side effect only
await import(/* webpackMode: "eager" */ 'ophan-tracker-js');
await import(/* webpackMode: "eager" */ '@guardian/ophan-tracker-js');

const { ophan } = window.guardian;

Expand Down
6 changes: 2 additions & 4 deletions dotcom-rendering/src/components/ArticlePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,8 @@ export const ArticlePage = (props: WebProps | AppProps) => {
adUnit: article.config.adUnit,
});

const isInLightboxTest =
article.config.abTests.lightboxVariant === 'variant';

const webLightbox = renderingTarget === 'Web' && isInLightboxTest;
const webLightbox =
renderingTarget === 'Web' && !!article.config.switches.lightbox;

return (
<StrictMode>
Expand Down
3 changes: 3 additions & 0 deletions dotcom-rendering/src/components/CartoonComponent.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const Cartoon = () => {
theme: Pillar.News,
}}
element={cartoon}
lightbox={true}
/>
</Figure>
</Wrapper>
Expand All @@ -84,6 +85,7 @@ export const CartoonWithoutCredit = () => {
design: ArticleDesign.Standard,
theme: Pillar.News,
}}
lightbox={true}
/>
</Figure>
</Wrapper>
Expand All @@ -110,6 +112,7 @@ export const CartoonWithNoMobileImages = () => {
design: ArticleDesign.Standard,
theme: Pillar.News,
}}
lightbox={true}
/>
</Figure>
</Wrapper>
Expand Down
Loading

0 comments on commit 2154f88

Please sign in to comment.