Skip to content
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

Add amp script if amp html attr present #718

Merged
merged 30 commits into from
Oct 2, 2018
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a383e9f
add amp script if amp attr is in HTML
Sep 25, 2018
fe3846b
add test for injecting amp <script>
Sep 25, 2018
5f3b2dc
conditionally render the client side application, and move amp script…
Sep 25, 2018
901876e
maintain previous format
Sep 25, 2018
f254c56
reduce cognitive complexity to keep CodeClimate happy
Sep 25, 2018
58327d8
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
Sep 25, 2018
cc96ede
move methods out of Document to reduce cognitive complexity
Sep 25, 2018
1dbac7d
Merge branch 'add-amp-script-if-amp-html-attr-present' of github.com:…
Sep 25, 2018
bd81545
Merge branch 'latest' of github.com:bbc/simorgh into add-amp-script-i…
Sep 26, 2018
545ceca
Merge branch 'latest' of github.com:bbc/simorgh into add-amp-script-i…
jamesbrumpton Sep 26, 2018
af0f28a
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
Sep 26, 2018
4050a42
Adding tests to check the AMP script loads
jamesbrumpton Sep 26, 2018
534362c
Merge branch 'add-amp-script-if-amp-html-attr-present' of github.com:…
jamesbrumpton Sep 26, 2018
c4b688f
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
Sep 26, 2018
fe9a206
remove test for checking non-AMP scripts aren't loaded - handled in a…
Sep 26, 2018
91b8184
PR is now to just add AMP script - other changes handled in future PR
Sep 26, 2018
d523464
update snapshots
Sep 26, 2018
fc36942
linting
Sep 26, 2018
36bb8c4
move logic to add amp script into metaData
Sep 26, 2018
09cb4d4
these no longer change
Sep 26, 2018
873048c
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
sareh Sep 30, 2018
f81bf2f
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
bcmn Oct 1, 2018
265b54f
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
sareh Oct 1, 2018
5ce9863
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
sareh Oct 2, 2018
a6715a9
Merge branch 'latest' of github.com:BBC-News/simorgh into add-amp-scr…
jamesbrumpton Oct 2, 2018
8eeb696
Updating ampSpec.js
jamesbrumpton Oct 2, 2018
f6bd4ba
Updating Snapshot test
jamesbrumpton Oct 2, 2018
57dc32f
Fixing conflicts
jamesbrumpton Oct 2, 2018
c9b159f
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
jamesbrumpton Oct 2, 2018
edcb50e
Merge branch 'latest' into add-amp-script-if-amp-html-attr-present
jamesbrumpton Oct 2, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cypress/integration/ampSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ describe('AMP Tests', () => {
getElement('html').should('have.attr', 'amp');
});

it('should load the AMP framework', () => {
// .eq(1) gets the amp <script> as the first loaded is a Cypress <script>
const ampScript = getElement('head script').eq(1);
ampScript.should('have.attr', 'src', 'https://cdn.ampproject.org/v0.js');
});

it('should not have an AMP attribute on the main article', () => {
cy.visit('/news/articles/c0000000027o');
getElement('html').should('not.have.attr', 'amp');
Expand Down
8 changes: 8 additions & 0 deletions src/app/components/Metadata/__snapshots__/index.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ exports[`Metadata News AMP article should render correctly 1`] = `
content="An article title"
name="twitter:title"
/>
<script
async={true}
src="https://cdn.ampproject.org/v0.js"
/>
</HelmetWrapper>
`;

Expand Down Expand Up @@ -320,6 +324,10 @@ exports[`Metadata Persian AMP article should render correctly 1`] = `
content="پهپادی که برایتان قهوه می‌آورد"
name="twitter:title"
/>
<script
async={true}
src="https://cdn.ampproject.org/v0.js"
/>
</HelmetWrapper>
`;

Expand Down
5 changes: 5 additions & 0 deletions src/app/components/Metadata/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ const Metadata = ({
htmlAttributes.amp = ''; // empty value as this makes Helmet render 'amp' as per https://www.ampproject.org/docs/fundamentals/spec#ampd
}

const injectAmpScript = amp ? (
<script key="amp" async src="https://cdn.ampproject.org/v0.js" />
) : null;

return (
<Helmet htmlAttributes={htmlAttributes}>
<meta
Expand Down Expand Up @@ -60,6 +64,7 @@ const Metadata = ({
<meta name="twitter:image:src" content={defaultImage} />
<meta name="twitter:site" content={twitterSite} />
<meta name="twitter:title" content={title} />
{injectAmpScript}
</Helmet>
);
};
Expand Down