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

[POC] [WIP] AMP validation fixes #878

Closed
wants to merge 14 commits into from
Closed
10 changes: 10 additions & 0 deletions src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Image should render AMP version correctly 1`] = `
<amp-img
alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17."
height="56.25"
layout="responsive"
src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png"
width="100"
/>
`;

exports[`Image should render correctly 1`] = `
.c0 {
display: block;
Expand Down
14 changes: 14 additions & 0 deletions src/app/components/Figure/Image/amp/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react';
import { string, number } from 'prop-types';

const AmpImage = ({ alt, src, ratio }) => (
<amp-img alt={alt} src={src} height={ratio} width="100" layout="responsive"></amp-img> // eslint-disable-line react/self-closing-comp, prettier/prettier
);

AmpImage.propTypes = {
alt: string.isRequired,
src: string.isRequired,
ratio: number.isRequired,
};

export default AmpImage;
15 changes: 15 additions & 0 deletions src/app/components/Figure/Image/index.stories.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React from 'react';
import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies
import Image from './index';
import AmpImage from './amp/index';

const imageAlt =
'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.';
const imageSrc =
'https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png';

storiesOf('Image', module)
.add('default', () => <Image alt={imageAlt} src={imageSrc} />)
.add('default - AMP version', () => (
<AmpImage alt={imageAlt} src={imageSrc} ratio="56.25" />
));
6 changes: 6 additions & 0 deletions src/app/components/Figure/Image/index.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers';
import Image from './index';
import AmpImage from './amp/index';

const imageAlt =
'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.';
Expand All @@ -12,4 +13,9 @@ describe('Image', () => {
'should render correctly',
<Image alt={imageAlt} src={imageSrc} />,
);

shouldMatchSnapshot(
'should render AMP version correctly',
<AmpImage alt={imageAlt} src={imageSrc} ratio="56.25" />,
);
});
4 changes: 4 additions & 0 deletions src/app/containers/Article/__snapshots__/index.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ exports[`ArticleContainer Component should render Persian article correctly 1`]
"headline": [Function],
}
}
platform="canonical"
/>
<Timestamp
timestamp={1514811600000}
Expand Down Expand Up @@ -115,6 +116,7 @@ exports[`ArticleContainer Component should render Persian article correctly 1`]
"text": [Function],
}
}
platform="canonical"
/>
</GridItemConstrained>
</OatWrapper>
Expand Down Expand Up @@ -204,6 +206,7 @@ exports[`ArticleContainer Component should render correctly 1`] = `
"headline": [Function],
}
}
platform="canonical"
/>
<Timestamp
timestamp={1514811600000}
Expand Down Expand Up @@ -237,6 +240,7 @@ exports[`ArticleContainer Component should render correctly 1`] = `
"text": [Function],
}
}
platform="canonical"
/>
</GridItemConstrained>
</OatWrapper>
Expand Down
2 changes: 2 additions & 0 deletions src/app/containers/Article/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const ArticleContainer = ({ loading, error, data }) => {
<Wrapper>
<GridItemConstrained>
<Blocks
platform={isAmp ? 'amp' : 'canonical'}
blocks={headlineBlocks}
componentsToRender={componentsToRenderHeadline}
/>
Expand All @@ -92,6 +93,7 @@ const ArticleContainer = ({ loading, error, data }) => {
<OatWrapper>
<GridItemConstrained>
<Blocks
platform={isAmp ? 'amp' : 'canonical'}
blocks={mainBlocks}
componentsToRender={componentsToRenderMain}
/>
Expand Down
3 changes: 2 additions & 1 deletion src/app/containers/Blocks/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const BlockString = props => {
return <p>{stringProps}</p>;
};

const Blocks = ({ blocks, componentsToRender }) =>
const Blocks = ({ blocks, componentsToRender, platform }) =>
blocks.map((block, index) => {
const { type, model } = block;

Expand All @@ -18,6 +18,7 @@ const Blocks = ({ blocks, componentsToRender }) =>

return (
<Block
platform={platform}
key={nanoid()}
type={type}
typeOfPreviousBlock={typeOfPreviousBlock}
Expand Down
10 changes: 8 additions & 2 deletions src/app/containers/Figure/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import { string, number } from 'prop-types';
import Figure from '../../components/Figure';
import Image from '../../components/Figure/Image';
import AmpImage from '../../components/Figure/Image/amp';
import ImagePlaceholder from '../../components/Figure/ImagePlaceholder';
import Copyright from '../../components/Figure/Copyright';
import Caption from '../../components/Figure/Caption';
Expand All @@ -15,10 +16,14 @@ const renderCaption = captionValue =>
<Text text={captionValue} paragraphOverride={Caption} />
) : null;

const FigureContainer = ({ src, alt, ratio, copyright, caption }) => (
const FigureContainer = ({ src, alt, ratio, copyright, caption, platform }) => (
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

<Figure>
<ImagePlaceholder ratio={ratio}>
<Image alt={alt} src={src} />
{platform === 'amp' ? (
<AmpImage alt={alt} src={src} ratio={ratio} />
) : (
<Image alt={alt} src={src} />
)}
{renderCopyright(copyright)}
</ImagePlaceholder>
{renderCaption(caption)}
Expand All @@ -31,6 +36,7 @@ FigureContainer.propTypes = {
ratio: number.isRequired,
copyright: string,
caption: string,
platform: string.isRequired,
};

FigureContainer.defaultProps = {
Expand Down
3 changes: 2 additions & 1 deletion src/app/containers/Image/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const getIChefURL = (originCode, locator) => {
const getRawImageSrc = (originCode, locator) =>
originCode !== 'pips' ? getIChefURL(originCode, locator) : locator;

const ImageContainer = ({ blocks }) => {
const ImageContainer = ({ blocks, platform }) => {
if (!blocks) {
return null;
}
Expand Down Expand Up @@ -60,6 +60,7 @@ const ImageContainer = ({ blocks }) => {

return (
<Figure
platform={platform}
src={rawImageSrc}
alt={altText}
ratio={ratio}
Expand Down