From 206e4b87f9a233794a1c7b1078d8d02349f87795 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 12:16:33 +0000 Subject: [PATCH 01/27] Update Image component to include AmpImg, which extends the canonical Image component --- src/app/components/Figure/Image/README.md | 23 ++++++++ .../__snapshots__/index.amp.test.jsx.snap | 45 ++++++++++++++++ .../Image/__snapshots__/index.test.jsx.snap | 49 ++++++++++++++++- .../components/Figure/Image/fixtureData.js | 16 ++++++ src/app/components/Figure/Image/index.amp.jsx | 6 +++ .../Figure/Image/index.amp.stories.jsx | 27 ++++++++++ .../Figure/Image/index.amp.test.jsx | 52 +++++++++++++++++++ src/app/components/Figure/Image/index.jsx | 17 +++++- .../components/Figure/Image/index.stories.jsx | 33 ++++++++---- .../components/Figure/Image/index.test.jsx | 45 ++++++++++++---- 10 files changed, 292 insertions(+), 21 deletions(-) create mode 100644 src/app/components/Figure/Image/README.md create mode 100644 src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap create mode 100644 src/app/components/Figure/Image/fixtureData.js create mode 100644 src/app/components/Figure/Image/index.amp.jsx create mode 100644 src/app/components/Figure/Image/index.amp.stories.jsx create mode 100644 src/app/components/Figure/Image/index.amp.test.jsx diff --git a/src/app/components/Figure/Image/README.md b/src/app/components/Figure/Image/README.md new file mode 100644 index 00000000000..c109382fc9c --- /dev/null +++ b/src/app/components/Figure/Image/README.md @@ -0,0 +1,23 @@ +# Image component + +## Usage + +Importing the standard Image component, which renders an `` tag. + +```jsx +import { Img } from '../components/Image'; + +const WrappingContainer = ({ alt, height, src, width }) => ( + {alt} +); +``` + +Importing an Amp Image component, which renders an `` tag. + +```jsx +import { AmpImg } from '../components/Image'; + +const WrappingContainer = ({ alt, height, src, width }) => ( + +); +``` diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap new file mode 100644 index 00000000000..187592f02c0 --- /dev/null +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -0,0 +1,45 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + + +`; + +exports[`Image - AmpImg should render landscape image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + + +`; + +exports[`Image - AmpImg should render portrait image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + + +`; + +exports[`Image - AmpImg should render square image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + + +`; diff --git a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap index 97cf5447828..2433d0e77d1 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap @@ -1,6 +1,36 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Image should render correctly 1`] = ` +exports[`Image - Img should render image with custom dimensions correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +By Elisa Decker, from her series \\ +`; + +exports[`Image - Img should render landscape image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Birthday illustration with llama and dogs +`; + +exports[`Image - Img should render portrait image correctly 1`] = ` .c0 { display: block; width: 100%; @@ -9,6 +39,23 @@ exports[`Image should render correctly 1`] = ` Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17. +`; + +exports[`Image - Img should render square image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Tracks through the snow `; diff --git a/src/app/components/Figure/Image/fixtureData.js b/src/app/components/Figure/Image/fixtureData.js new file mode 100644 index 00000000000..6531c15c402 --- /dev/null +++ b/src/app/components/Figure/Image/fixtureData.js @@ -0,0 +1,16 @@ +export const imageAltLandscape = 'Birthday illustration with llama and dogs'; +export const imageSrcLandscape = + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/537D/production/_104837312_crazyrichbirthday_illustration_dogs-nc976.jpg'; + +export const imageAltPortrait = + 'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.'; +export const imageSrcPortrait = + 'https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png'; + +export const imageAltSquare = 'Tracks through the snow'; +export const imageSrcSquare = + 'https://ichef.bbci.co.uk/news/660/cpsprodpb/114FE/production/_104801907_79010.jpg'; + +export const imageAltCustom = 'By Elisa Decker, from her series "Sidewalk"'; +export const imageSrcCustom = + 'https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg'; diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx new file mode 100644 index 00000000000..37e6d8a5f73 --- /dev/null +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -0,0 +1,6 @@ +import React from 'react'; +import { Img } from '.'; + +const AmpImg = () => ; + +export default AmpImg; diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx new file mode 100644 index 00000000000..5ac1770ccfc --- /dev/null +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -0,0 +1,27 @@ +import React from 'react'; +import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies +import AmpImg from './index.amp'; +import { + imageAltLandscape, + imageSrcLandscape, + imageAltPortrait, + imageSrcPortrait, + imageAltSquare, + imageSrcSquare, + imageAltCustom, + imageSrcCustom, +} from './fixtureData'; + +storiesOf('Image - AmpImg', module) + .add('16:9 landscape image', () => ( + + )) + .add('16:9 portrait image', () => ( + + )) + .add('1:1 square image', () => ( + + )) + .add('custom ratio image', () => ( + + )); diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx new file mode 100644 index 00000000000..d205d3b3e03 --- /dev/null +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -0,0 +1,52 @@ +import React from 'react'; +import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; +import AmpImg from './index.amp'; +import { + imageAltLandscape, + imageSrcLandscape, + imageAltPortrait, + imageSrcPortrait, + imageAltSquare, + imageSrcSquare, + imageAltCustom, + imageSrcCustom, +} from './fixtureData'; + +describe('Image - AmpImg', () => { + shouldMatchSnapshot( + 'should render portrait image correctly', + , + ); + shouldMatchSnapshot( + 'should render landscape image correctly', + , + ); + shouldMatchSnapshot( + 'should render square image correctly', + , + ); + shouldMatchSnapshot( + 'should render image with custom dimensions correctly', + , + ); +}); diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index b7bd12c8b58..c27a38a88bf 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -1,8 +1,21 @@ +import React from 'react'; import styled from 'styled-components'; +import { number, string } from 'prop-types'; -const Image = styled.img` +const ImgWithStyles = styled.img` display: block; width: 100%; `; -export default Image; +export const Img = ({ alt, height, src, width }) => ( + +); + +Img.propTypes = { + alt: string.isRequired, + height: number.isRequired, + src: string.isRequired, + width: number.isRequired, +}; + +export default Img; diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index f3cfed837f0..a0bf162a557 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -1,12 +1,27 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies -import Image from './index'; +import { Img } from './index'; +import { + imageAltLandscape, + imageSrcLandscape, + imageAltPortrait, + imageSrcPortrait, + imageAltSquare, + imageSrcSquare, + imageAltCustom, + imageSrcCustom, +} from './fixtureData'; -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', () => ( - {imageAlt} -)); +storiesOf('Image - Img', module) + .add('16:9 landscape image', () => ( + {imageAltLandscape} + )) + .add('16:9 portrait image', () => ( + {imageAltPortrait} + )) + .add('1:1 square image', () => ( + {imageAltSquare} + )) + .add('custom ratio image', () => ( + {imageAltCustom} + )); diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index 42c0686df3e..7e81cf53ca0 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -1,15 +1,42 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; -import Image from './index'; +import { Img } from './index'; +import { + imageAltLandscape, + imageSrcLandscape, + imageAltPortrait, + imageSrcPortrait, + imageAltSquare, + imageSrcSquare, + imageAltCustom, + imageSrcCustom, +} from './fixtureData'; -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'; - -describe('Image', () => { +describe('Image - Img', () => { + shouldMatchSnapshot( + 'should render portrait image correctly', + {imageAltPortrait}, + ); + shouldMatchSnapshot( + 'should render landscape image correctly', + {imageAltLandscape}, + ); + shouldMatchSnapshot( + 'should render square image correctly', + {imageAltSquare}, + ); shouldMatchSnapshot( - 'should render correctly', - {imageAlt}, + 'should render image with custom dimensions correctly', + {imageAltCustom}, ); }); From 2df0125eac5c8848573dae932a1480219373e5b1 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 12:18:48 +0000 Subject: [PATCH 02/27] Use Img in Figure Container --- src/app/containers/Figure/index.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index a2cf4bc2867..8f1a1ecf389 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -1,7 +1,7 @@ import React from 'react'; import { string, number, objectOf, any } from 'prop-types'; import Figure from '../../components/Figure'; -import Image from '../../components/Figure/Image'; +import { Img } from '../../components/Figure/Image'; import ImagePlaceholder from '../../components/Figure/ImagePlaceholder'; import Copyright from '../Copyright'; import Caption from '../Caption'; @@ -14,7 +14,7 @@ const renderCaption = block => (block ? : null); const FigureContainer = ({ src, alt, ratio, copyright, captionBlock }) => (
- {alt} + {alt} {renderCopyright(copyright)} {renderCaption(captionBlock)} From aafe6c3d557f9f1e3249b1ba649392bb6a0d48a2 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 12:28:27 +0000 Subject: [PATCH 03/27] Use Image in Figure Container --- src/app/containers/Figure/index.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index 8f1a1ecf389..a2cf4bc2867 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -1,7 +1,7 @@ import React from 'react'; import { string, number, objectOf, any } from 'prop-types'; import Figure from '../../components/Figure'; -import { Img } from '../../components/Figure/Image'; +import Image from '../../components/Figure/Image'; import ImagePlaceholder from '../../components/Figure/ImagePlaceholder'; import Copyright from '../Copyright'; import Caption from '../Caption'; @@ -14,7 +14,7 @@ const renderCaption = block => (block ? : null); const FigureContainer = ({ src, alt, ratio, copyright, captionBlock }) => (
- {alt} + {alt} {renderCopyright(copyright)} {renderCaption(captionBlock)} From 4e18239d10250c9aa38fcea1364eaa442912dfbf Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 12:30:25 +0000 Subject: [PATCH 04/27] Add tests for default export/import --- .../Image/__snapshots__/index.test.jsx.snap | 68 +++++++++++++++++-- src/app/components/Figure/Image/index.jsx | 4 +- .../components/Figure/Image/index.test.jsx | 43 +++++++++++- 3 files changed, 108 insertions(+), 7 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap index 2433d0e77d1..d574cd8b038 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Image - Img should render image with custom dimensions correctly 1`] = ` +exports[`Image - imported as '{ Img }' should render image with custom dimensions correctly 1`] = ` .c0 { display: block; width: 100%; @@ -15,7 +15,7 @@ exports[`Image - Img should render image with custom dimensions correctly 1`] = /> `; -exports[`Image - Img should render landscape image correctly 1`] = ` +exports[`Image - imported as '{ Img }' should render landscape image correctly 1`] = ` .c0 { display: block; width: 100%; @@ -30,7 +30,7 @@ exports[`Image - Img should render landscape image correctly 1`] = ` /> `; -exports[`Image - Img should render portrait image correctly 1`] = ` +exports[`Image - imported as '{ Img }' should render portrait image correctly 1`] = ` .c0 { display: block; width: 100%; @@ -45,7 +45,67 @@ exports[`Image - Img should render portrait image correctly 1`] = ` /> `; -exports[`Image - Img should render square image correctly 1`] = ` +exports[`Image - imported as '{ Img }' should render square image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Tracks through the snow +`; + +exports[`Image - imported as default 'Image' should render image with custom dimensions correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +By Elisa Decker, from her series \\ +`; + +exports[`Image - imported as default 'Image' should render landscape image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Birthday illustration with llama and dogs +`; + +exports[`Image - imported as default 'Image' should render portrait image correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17. +`; + +exports[`Image - imported as default 'Image' should render square image correctly 1`] = ` .c0 { display: block; width: 100%; diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index c27a38a88bf..28ee8eb1b4a 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -18,4 +18,6 @@ Img.propTypes = { width: number.isRequired, }; -export default Img; +const Image = Img; + +export default Image; diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index 7e81cf53ca0..22d51759992 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; -import { Img } from './index'; +import Image, { Img } from './index'; import { imageAltLandscape, imageSrcLandscape, @@ -12,7 +12,7 @@ import { imageSrcCustom, } from './fixtureData'; -describe('Image - Img', () => { +describe("Image - imported as '{ Img }'", () => { shouldMatchSnapshot( 'should render portrait image correctly', { {imageAltCustom}, ); }); + +describe("Image - imported as default 'Image'", () => { + shouldMatchSnapshot( + 'should render portrait image correctly', + {imageAltPortrait}, + ); + shouldMatchSnapshot( + 'should render landscape image correctly', + {imageAltLandscape}, + ); + shouldMatchSnapshot( + 'should render square image correctly', + {imageAltSquare}, + ); + shouldMatchSnapshot( + 'should render image with custom dimensions correctly', + {imageAltCustom}, + ); +}); From 5c6a97321eb7ebbc529e1c974826a8fd73b05963 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 12:36:52 +0000 Subject: [PATCH 05/27] Refactor tests to remove duplication --- .../components/Figure/Image/index.test.jsx | 47 +++++-------------- 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index 22d51759992..7aaa81f15d0 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -12,10 +12,10 @@ import { imageSrcCustom, } from './fixtureData'; -describe("Image - imported as '{ Img }'", () => { +const snapshotTests = Component => { shouldMatchSnapshot( 'should render portrait image correctly', - {imageAltPortrait} { ); shouldMatchSnapshot( 'should render landscape image correctly', - {imageAltLandscape} { ); shouldMatchSnapshot( 'should render square image correctly', - {imageAltSquare}, - ); - shouldMatchSnapshot( - 'should render image with custom dimensions correctly', - {imageAltCustom}, - ); -}); - -describe("Image - imported as default 'Image'", () => { - shouldMatchSnapshot( - 'should render portrait image correctly', - {imageAltPortrait}, - ); - shouldMatchSnapshot( - 'should render landscape image correctly', - {imageAltLandscape}, - ); - shouldMatchSnapshot( - 'should render square image correctly', - {imageAltSquare} { ); shouldMatchSnapshot( 'should render image with custom dimensions correctly', - {imageAltCustom}, ); +}; + +describe("Image - imported as '{ Img }'", () => { + snapshotTests(Img); +}); + +describe("Image - imported as default 'Image'", () => { + snapshotTests(Image); }); From ed6791f3519edbdf18bec9ede29cea418db1c119 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 13:23:27 +0000 Subject: [PATCH 06/27] Correct image dimensions and examples --- .../__snapshots__/index.amp.test.jsx.snap | 8 +-- .../Image/__snapshots__/index.test.jsx.snap | 40 ++++++------ .../components/Figure/Image/fixtureData.js | 31 ++++++---- .../Figure/Image/index.amp.stories.jsx | 54 ++++++++++++---- .../Figure/Image/index.amp.test.jsx | 62 +++++++++++-------- .../components/Figure/Image/index.stories.jsx | 54 ++++++++++++---- .../components/Figure/Image/index.test.jsx | 56 ++++++++++------- 7 files changed, 193 insertions(+), 112 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index 187592f02c0..a0470ead2b8 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = ` +exports[`Image - AmpImg should render 1:1 square image correctly 1`] = ` .c0 { display: block; width: 100%; @@ -11,7 +11,7 @@ exports[`Image - AmpImg should render image with custom dimensions correctly 1`] /> `; -exports[`Image - AmpImg should render landscape image correctly 1`] = ` +exports[`Image - AmpImg should render 9:16 portrait image correctly 1`] = ` .c0 { display: block; width: 100%; @@ -22,7 +22,7 @@ exports[`Image - AmpImg should render landscape image correctly 1`] = ` /> `; -exports[`Image - AmpImg should render portrait image correctly 1`] = ` +exports[`Image - AmpImg should render 16:9 landscape image correctly 1`] = ` .c0 { display: block; width: 100%; @@ -33,7 +33,7 @@ exports[`Image - AmpImg should render portrait image correctly 1`] = ` /> `; -exports[`Image - AmpImg should render square image correctly 1`] = ` +exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = ` .c0 { display: block; width: 100%; diff --git a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap index d574cd8b038..3ac6c2b3daa 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap @@ -9,9 +9,9 @@ exports[`Image - imported as '{ Img }' should render image with custom dimension By Elisa Decker, from her series \\ `; @@ -22,11 +22,11 @@ exports[`Image - imported as '{ Img }' should render landscape image correctly 1 } Birthday illustration with llama and dogs `; @@ -40,7 +40,7 @@ exports[`Image - imported as '{ Img }' should render portrait image correctly 1` alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." className="c0" height={723} - src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" width={578} /> `; @@ -54,9 +54,9 @@ exports[`Image - imported as '{ Img }' should render square image correctly 1`] Tracks through the snow `; @@ -69,9 +69,9 @@ exports[`Image - imported as default 'Image' should render image with custom dim By Elisa Decker, from her series \\ `; @@ -82,11 +82,11 @@ exports[`Image - imported as default 'Image' should render landscape image corre } Birthday illustration with llama and dogs `; @@ -100,7 +100,7 @@ exports[`Image - imported as default 'Image' should render portrait image correc alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." className="c0" height={723} - src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" width={578} /> `; @@ -114,8 +114,8 @@ exports[`Image - imported as default 'Image' should render square image correctl Tracks through the snow `; diff --git a/src/app/components/Figure/Image/fixtureData.js b/src/app/components/Figure/Image/fixtureData.js index 6531c15c402..1a1509b6f2f 100644 --- a/src/app/components/Figure/Image/fixtureData.js +++ b/src/app/components/Figure/Image/fixtureData.js @@ -1,16 +1,25 @@ -export const imageAltLandscape = 'Birthday illustration with llama and dogs'; -export const imageSrcLandscape = - 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/537D/production/_104837312_crazyrichbirthday_illustration_dogs-nc976.jpg'; +export const imageLandscapeAlt = 'Student sitting an exam'; +export const imageLandscapeSrc = + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg'; +export const imageLandscapeWidth = 1024; +export const imageLandscapeHeight = 576; -export const imageAltPortrait = +export const imagePortraitAlt = 'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.'; -export const imageSrcPortrait = - 'https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png'; +export const imagePortraitSrc = + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png'; +export const imagePortraitWidth = 578; +export const imagePortraitHeight = 723; -export const imageAltSquare = 'Tracks through the snow'; -export const imageSrcSquare = - 'https://ichef.bbci.co.uk/news/660/cpsprodpb/114FE/production/_104801907_79010.jpg'; +export const imageSquareAlt = 'Tracks through the snow'; +export const imageSquareSrc = + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/114FE/production/_104801907_79010.jpg'; +export const imageSquareWidth = 1024; +export const imageSquareHeight = 1024; -export const imageAltCustom = 'By Elisa Decker, from her series "Sidewalk"'; -export const imageSrcCustom = +export const imageCustomAlt = 'By Elisa Decker, from her series "Sidewalk"'; +export const imageCustomSrc = 'https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg'; + +export const imageCustomWidth = 445; +export const imageCustomHeight = 547; diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx index 5ac1770ccfc..0f86cb19860 100644 --- a/src/app/components/Figure/Image/index.amp.stories.jsx +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -2,26 +2,54 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import AmpImg from './index.amp'; import { - imageAltLandscape, - imageSrcLandscape, - imageAltPortrait, - imageSrcPortrait, - imageAltSquare, - imageSrcSquare, - imageAltCustom, - imageSrcCustom, + imageCustomAlt, + imageCustomHeight, + imageCustomSrc, + imageCustomWidth, + imageLandscapeAlt, + imageLandscapeHeight, + imageLandscapeSrc, + imageLandscapeWidth, + imagePortraitAlt, + imagePortraitHeight, + imagePortraitSrc, + imagePortraitWidth, + imageSquareAlt, + imageSquareHeight, + imageSquareSrc, + imageSquareWidth, } from './fixtureData'; storiesOf('Image - AmpImg', module) .add('16:9 landscape image', () => ( - + )) - .add('16:9 portrait image', () => ( - + .add('9:16 portrait image', () => ( + )) .add('1:1 square image', () => ( - + )) .add('custom ratio image', () => ( - + )); diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx index d205d3b3e03..428ae1facc8 100644 --- a/src/app/components/Figure/Image/index.amp.test.jsx +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -2,51 +2,59 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import AmpImg from './index.amp'; import { - imageAltLandscape, - imageSrcLandscape, - imageAltPortrait, - imageSrcPortrait, - imageAltSquare, - imageSrcSquare, - imageAltCustom, - imageSrcCustom, + imageCustomAlt, + imageCustomHeight, + imageCustomSrc, + imageCustomWidth, + imageLandscapeAlt, + imageLandscapeHeight, + imageLandscapeSrc, + imageLandscapeWidth, + imagePortraitAlt, + imagePortraitHeight, + imagePortraitSrc, + imagePortraitWidth, + imageSquareAlt, + imageSquareHeight, + imageSquareSrc, + imageSquareWidth, } from './fixtureData'; describe('Image - AmpImg', () => { shouldMatchSnapshot( - 'should render portrait image correctly', + 'should render 16:9 landscape image correctly', , ); shouldMatchSnapshot( - 'should render landscape image correctly', + 'should render 9:16 portrait image correctly', , ); shouldMatchSnapshot( - 'should render square image correctly', + 'should render 1:1 square image correctly', , ); shouldMatchSnapshot( 'should render image with custom dimensions correctly', , ); }); diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index a0bf162a557..158214347d7 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -2,26 +2,54 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import { Img } from './index'; import { - imageAltLandscape, - imageSrcLandscape, - imageAltPortrait, - imageSrcPortrait, - imageAltSquare, - imageSrcSquare, - imageAltCustom, - imageSrcCustom, + imageCustomAlt, + imageCustomHeight, + imageCustomSrc, + imageCustomWidth, + imageLandscapeAlt, + imageLandscapeHeight, + imageLandscapeSrc, + imageLandscapeWidth, + imagePortraitAlt, + imagePortraitHeight, + imagePortraitSrc, + imagePortraitWidth, + imageSquareAlt, + imageSquareHeight, + imageSquareSrc, + imageSquareWidth, } from './fixtureData'; storiesOf('Image - Img', module) .add('16:9 landscape image', () => ( - {imageAltLandscape} + {imageLandscapeAlt} )) - .add('16:9 portrait image', () => ( - {imageAltPortrait} + .add('9:16 portrait image', () => ( + {imagePortraitAlt} )) .add('1:1 square image', () => ( - {imageAltSquare} + {imageSquareAlt} )) .add('custom ratio image', () => ( - {imageAltCustom} + {imageCustomAlt} )); diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index 7aaa81f15d0..548d250ba4a 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -2,51 +2,59 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import Image, { Img } from './index'; import { - imageAltLandscape, - imageSrcLandscape, - imageAltPortrait, - imageSrcPortrait, - imageAltSquare, - imageSrcSquare, - imageAltCustom, - imageSrcCustom, + imageCustomAlt, + imageCustomHeight, + imageCustomSrc, + imageCustomWidth, + imageLandscapeAlt, + imageLandscapeHeight, + imageLandscapeSrc, + imageLandscapeWidth, + imagePortraitAlt, + imagePortraitHeight, + imagePortraitSrc, + imagePortraitWidth, + imageSquareAlt, + imageSquareHeight, + imageSquareSrc, + imageSquareWidth, } from './fixtureData'; const snapshotTests = Component => { shouldMatchSnapshot( 'should render portrait image correctly', , ); shouldMatchSnapshot( 'should render landscape image correctly', , ); shouldMatchSnapshot( 'should render square image correctly', , ); shouldMatchSnapshot( 'should render image with custom dimensions correctly', , ); }; From da39b3d4e3065bdacf01d1d5659453f8932ad95b Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 13:35:34 +0000 Subject: [PATCH 07/27] Correct image dimensions and examples & refactor fixtureData exports --- .../Image/__snapshots__/index.test.jsx.snap | 12 ++-- .../components/Figure/Image/fixtureData.js | 50 ++++++++++------- .../Figure/Image/index.amp.stories.jsx | 51 ++++++----------- .../Figure/Image/index.amp.test.jsx | 51 ++++++----------- .../components/Figure/Image/index.stories.jsx | 51 ++++++----------- .../components/Figure/Image/index.test.jsx | 55 +++++++------------ 6 files changed, 105 insertions(+), 165 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap index 3ac6c2b3daa..11ee24386e6 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap @@ -39,9 +39,9 @@ exports[`Image - imported as '{ Img }' should render portrait image correctly 1` Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17. `; @@ -99,9 +99,9 @@ exports[`Image - imported as default 'Image' should render portrait image correc Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17. `; diff --git a/src/app/components/Figure/Image/fixtureData.js b/src/app/components/Figure/Image/fixtureData.js index 1a1509b6f2f..2fda09360e8 100644 --- a/src/app/components/Figure/Image/fixtureData.js +++ b/src/app/components/Figure/Image/fixtureData.js @@ -1,25 +1,33 @@ -export const imageLandscapeAlt = 'Student sitting an exam'; -export const imageLandscapeSrc = - 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg'; -export const imageLandscapeWidth = 1024; -export const imageLandscapeHeight = 576; +export const landscape = { + alt: 'Student sitting an exam', + src: + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg', + width: 1024, + height: 576, +}; -export const imagePortraitAlt = - 'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.'; -export const imagePortraitSrc = - 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png'; -export const imagePortraitWidth = 578; -export const imagePortraitHeight = 723; +export const portrait = { + alt: + 'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.', + src: + 'https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png', + width: 640, + height: 1138, +}; -export const imageSquareAlt = 'Tracks through the snow'; -export const imageSquareSrc = - 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/114FE/production/_104801907_79010.jpg'; -export const imageSquareWidth = 1024; -export const imageSquareHeight = 1024; +export const square = { + alt: 'Tracks through the snow', + src: + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/114FE/production/_104801907_79010.jpg', + width: 1024, + height: 1024, +}; -export const imageCustomAlt = 'By Elisa Decker, from her series "Sidewalk"'; -export const imageCustomSrc = - 'https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg'; +export const custom = { + alt: 'By Elisa Decker, from her series "Sidewalk"', + src: + 'https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg', -export const imageCustomWidth = 445; -export const imageCustomHeight = 547; + width: 445, + height: 547, +}; diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx index 0f86cb19860..3cd1094bc27 100644 --- a/src/app/components/Figure/Image/index.amp.stories.jsx +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -1,55 +1,38 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import AmpImg from './index.amp'; -import { - imageCustomAlt, - imageCustomHeight, - imageCustomSrc, - imageCustomWidth, - imageLandscapeAlt, - imageLandscapeHeight, - imageLandscapeSrc, - imageLandscapeWidth, - imagePortraitAlt, - imagePortraitHeight, - imagePortraitSrc, - imagePortraitWidth, - imageSquareAlt, - imageSquareHeight, - imageSquareSrc, - imageSquareWidth, -} from './fixtureData'; +import { custom, landscape, portrait, square } from './fixtureData'; storiesOf('Image - AmpImg', module) .add('16:9 landscape image', () => ( )) .add('9:16 portrait image', () => ( )) .add('1:1 square image', () => ( )) .add('custom ratio image', () => ( )); diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx index 428ae1facc8..eb94d6fb276 100644 --- a/src/app/components/Figure/Image/index.amp.test.jsx +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -1,60 +1,43 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import AmpImg from './index.amp'; -import { - imageCustomAlt, - imageCustomHeight, - imageCustomSrc, - imageCustomWidth, - imageLandscapeAlt, - imageLandscapeHeight, - imageLandscapeSrc, - imageLandscapeWidth, - imagePortraitAlt, - imagePortraitHeight, - imagePortraitSrc, - imagePortraitWidth, - imageSquareAlt, - imageSquareHeight, - imageSquareSrc, - imageSquareWidth, -} from './fixtureData'; +import { custom, landscape, portrait, square } from './fixtureData'; describe('Image - AmpImg', () => { shouldMatchSnapshot( 'should render 16:9 landscape image correctly', , ); shouldMatchSnapshot( 'should render 9:16 portrait image correctly', , ); shouldMatchSnapshot( 'should render 1:1 square image correctly', , ); shouldMatchSnapshot( 'should render image with custom dimensions correctly', , ); }); diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index 158214347d7..6c76d9846d5 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -1,55 +1,38 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import { Img } from './index'; -import { - imageCustomAlt, - imageCustomHeight, - imageCustomSrc, - imageCustomWidth, - imageLandscapeAlt, - imageLandscapeHeight, - imageLandscapeSrc, - imageLandscapeWidth, - imagePortraitAlt, - imagePortraitHeight, - imagePortraitSrc, - imagePortraitWidth, - imageSquareAlt, - imageSquareHeight, - imageSquareSrc, - imageSquareWidth, -} from './fixtureData'; +import { custom, landscape, portrait, square } from './fixtureData'; storiesOf('Image - Img', module) .add('16:9 landscape image', () => ( {imageLandscapeAlt} )) .add('9:16 portrait image', () => ( {imagePortraitAlt} )) .add('1:1 square image', () => ( {imageSquareAlt} )) .add('custom ratio image', () => ( {imageCustomAlt} )); diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index 548d250ba4a..2fc77c6c66b 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -1,60 +1,43 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import Image, { Img } from './index'; -import { - imageCustomAlt, - imageCustomHeight, - imageCustomSrc, - imageCustomWidth, - imageLandscapeAlt, - imageLandscapeHeight, - imageLandscapeSrc, - imageLandscapeWidth, - imagePortraitAlt, - imagePortraitHeight, - imagePortraitSrc, - imagePortraitWidth, - imageSquareAlt, - imageSquareHeight, - imageSquareSrc, - imageSquareWidth, -} from './fixtureData'; +import { custom, landscape, portrait, square } from './fixtureData'; const snapshotTests = Component => { shouldMatchSnapshot( - 'should render portrait image correctly', + 'should render landscape image correctly', , ); shouldMatchSnapshot( - 'should render landscape image correctly', + 'should render portrait image correctly', , ); shouldMatchSnapshot( 'should render square image correctly', , ); shouldMatchSnapshot( 'should render image with custom dimensions correctly', , ); }; From 3dc4bbf5de1d8e07debf1c1cbe91a8c83f0ec7a5 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 17 Dec 2018 15:02:46 +0000 Subject: [PATCH 08/27] Use withComponent syntax for AmpImg extension Add conditional logic for Img AmpImg rendering in Figure container --- src/app/components/Figure/Image/index.amp.jsx | 6 ++--- src/app/components/Figure/Image/index.jsx | 4 ++-- src/app/containers/Figure/index.jsx | 23 +++++++++++++++++-- src/app/containers/Image/index.jsx | 2 ++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index 37e6d8a5f73..c1be418246c 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -1,6 +1,6 @@ -import React from 'react'; -import { Img } from '.'; +import 'styled-components'; +import { StyledImg } from '.'; -const AmpImg = () => ; +const AmpImg = StyledImg.withComponent('amp-img'); export default AmpImg; diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 28ee8eb1b4a..1c4239e5113 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -2,13 +2,13 @@ import React from 'react'; import styled from 'styled-components'; import { number, string } from 'prop-types'; -const ImgWithStyles = styled.img` +export const StyledImg = styled.img` display: block; width: 100%; `; export const Img = ({ alt, height, src, width }) => ( - + ); Img.propTypes = { diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index a2cf4bc2867..3b18ee51ad6 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -2,6 +2,7 @@ import React from 'react'; import { string, number, objectOf, any } from 'prop-types'; import Figure from '../../components/Figure'; import Image from '../../components/Figure/Image'; +import AmpImg from '../../components/Figure/Image/index.amp'; import ImagePlaceholder from '../../components/Figure/ImagePlaceholder'; import Copyright from '../Copyright'; import Caption from '../Caption'; @@ -11,10 +12,26 @@ const renderCopyright = copyright => const renderCaption = block => (block ? : null); -const FigureContainer = ({ src, alt, ratio, copyright, captionBlock }) => ( +const renderImage = (platform, alt, src, height, width) => { + if (platform === 'amp') { + return ; + } + + return {alt}; +}; + +const FigureContainer = ({ + src, + alt, + ratio, + copyright, + captionBlock, + height, + width, +}) => (
- {alt} + {renderImage('canonical', alt, src, height, width)} {renderCopyright(copyright)} {renderCaption(captionBlock)} @@ -23,6 +40,8 @@ const FigureContainer = ({ src, alt, ratio, copyright, captionBlock }) => ( FigureContainer.propTypes = { alt: string.isRequired, + height: number.isRequired, + width: number.isRequired, src: string.isRequired, ratio: number.isRequired, copyright: string, diff --git a/src/app/containers/Image/index.jsx b/src/app/containers/Image/index.jsx index 92934e27564..d79a11e3b8e 100644 --- a/src/app/containers/Image/index.jsx +++ b/src/app/containers/Image/index.jsx @@ -57,6 +57,8 @@ const ImageContainer = ({ blocks }) => { ratio={ratio} copyright={copyright} captionBlock={captionBlock} + height={height} + width={width} /> ); }; From 5121fbc68abd0939c51e0debe78cb0e5b365e52e Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 08:50:01 +0000 Subject: [PATCH 09/27] Refactor tests & stories & update snapshots --- .../__snapshots__/index.amp.test.jsx.snap | 32 ++++++++++---- .../Figure/Image/{ => helpers}/fixtureData.js | 0 .../Figure/Image/helpers/snapshotTests.jsx | 44 +++++++++++++++++++ .../Figure/Image/helpers/stories.jsx | 40 +++++++++++++++++ .../Figure/Image/index.amp.stories.jsx | 38 +--------------- .../Figure/Image/index.amp.test.jsx | 2 +- .../components/Figure/Image/index.stories.jsx | 38 +--------------- .../components/Figure/Image/index.test.jsx | 43 +----------------- 8 files changed, 114 insertions(+), 123 deletions(-) rename src/app/components/Figure/Image/{ => helpers}/fixtureData.js (100%) create mode 100644 src/app/components/Figure/Image/helpers/snapshotTests.jsx create mode 100644 src/app/components/Figure/Image/helpers/stories.jsx diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index a0470ead2b8..7c47aaed0b1 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -6,8 +6,12 @@ exports[`Image - AmpImg should render 1:1 square image correctly 1`] = ` width: 100%; } -Tracks through the snow `; @@ -17,8 +21,12 @@ exports[`Image - AmpImg should render 9:16 portrait image correctly 1`] = ` width: 100%; } -Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17. `; @@ -28,8 +36,12 @@ exports[`Image - AmpImg should render 16:9 landscape image correctly 1`] = ` width: 100%; } -Student sitting an exam `; @@ -39,7 +51,11 @@ exports[`Image - AmpImg should render image with custom dimensions correctly 1`] width: 100%; } -By Elisa Decker, from her series \\ `; diff --git a/src/app/components/Figure/Image/fixtureData.js b/src/app/components/Figure/Image/helpers/fixtureData.js similarity index 100% rename from src/app/components/Figure/Image/fixtureData.js rename to src/app/components/Figure/Image/helpers/fixtureData.js diff --git a/src/app/components/Figure/Image/helpers/snapshotTests.jsx b/src/app/components/Figure/Image/helpers/snapshotTests.jsx new file mode 100644 index 00000000000..ccc7c8759ea --- /dev/null +++ b/src/app/components/Figure/Image/helpers/snapshotTests.jsx @@ -0,0 +1,44 @@ +import React from 'react'; +import { shouldMatchSnapshot } from '../../../../helpers/tests/testHelpers'; +import { custom, landscape, portrait, square } from './fixtureData'; + +const snapshotTests = Component => { + shouldMatchSnapshot( + 'should render landscape image correctly', + , + ); + shouldMatchSnapshot( + 'should render portrait image correctly', + , + ); + shouldMatchSnapshot( + 'should render square image correctly', + , + ); + shouldMatchSnapshot( + 'should render image with custom dimensions correctly', + , + ); +}; + +export default snapshotTests; diff --git a/src/app/components/Figure/Image/helpers/stories.jsx b/src/app/components/Figure/Image/helpers/stories.jsx new file mode 100644 index 00000000000..4ee81f426a6 --- /dev/null +++ b/src/app/components/Figure/Image/helpers/stories.jsx @@ -0,0 +1,40 @@ +import React from 'react'; +import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies +import { custom, landscape, portrait, square } from './fixtureData'; + +const stories = (Component, title) => + storiesOf(title, module) + .add('16:9 landscape image', () => ( + + )) + .add('9:16 portrait image', () => ( + + )) + .add('1:1 square image', () => ( + + )) + .add('custom ratio image', () => ( + + )); + +export default stories; diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx index 3cd1094bc27..cc35ec6f6f2 100644 --- a/src/app/components/Figure/Image/index.amp.stories.jsx +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -1,38 +1,4 @@ -import React from 'react'; -import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import AmpImg from './index.amp'; -import { custom, landscape, portrait, square } from './fixtureData'; +import stories from './helpers/stories'; -storiesOf('Image - AmpImg', module) - .add('16:9 landscape image', () => ( - - )) - .add('9:16 portrait image', () => ( - - )) - .add('1:1 square image', () => ( - - )) - .add('custom ratio image', () => ( - - )); +stories(AmpImg, 'Image - AmpImg'); diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx index eb94d6fb276..5cc0af6a9fe 100644 --- a/src/app/components/Figure/Image/index.amp.test.jsx +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -1,7 +1,7 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import AmpImg from './index.amp'; -import { custom, landscape, portrait, square } from './fixtureData'; +import { custom, landscape, portrait, square } from './helpers/fixtureData'; describe('Image - AmpImg', () => { shouldMatchSnapshot( diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index 6c76d9846d5..34d7f9eeb25 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -1,38 +1,4 @@ -import React from 'react'; -import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import { Img } from './index'; -import { custom, landscape, portrait, square } from './fixtureData'; +import stories from './helpers/stories'; -storiesOf('Image - Img', module) - .add('16:9 landscape image', () => ( - {landscape.alt} - )) - .add('9:16 portrait image', () => ( - {portrait.alt} - )) - .add('1:1 square image', () => ( - {square.alt} - )) - .add('custom ratio image', () => ( - {custom.alt} - )); +stories(Img, 'Image - Img'); diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index 2fc77c6c66b..c1e5fd912a7 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -1,46 +1,5 @@ -import React from 'react'; -import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import Image, { Img } from './index'; -import { custom, landscape, portrait, square } from './fixtureData'; - -const snapshotTests = Component => { - shouldMatchSnapshot( - 'should render landscape image correctly', - , - ); - shouldMatchSnapshot( - 'should render portrait image correctly', - , - ); - shouldMatchSnapshot( - 'should render square image correctly', - , - ); - shouldMatchSnapshot( - 'should render image with custom dimensions correctly', - , - ); -}; +import snapshotTests from './helpers/snapshotTests'; describe("Image - imported as '{ Img }'", () => { snapshotTests(Img); From fec261bc5a2d952d7fa0146c9815bbe3abeca487 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 08:51:48 +0000 Subject: [PATCH 10/27] Update Figure Container to include height and width properties --- .../Figure/__snapshots__/index.test.jsx.snap | 10 ++++++++++ src/app/containers/Figure/fixtureData.jsx | 4 ++++ src/app/containers/Figure/index.jsx | 17 ++++++++--------- .../Image/__snapshots__/index.test.jsx.snap | 12 ++++++++++++ 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/app/containers/Figure/__snapshots__/index.test.jsx.snap b/src/app/containers/Figure/__snapshots__/index.test.jsx.snap index 96182f9cb48..5cacf5af4f2 100644 --- a/src/app/containers/Figure/__snapshots__/index.test.jsx.snap +++ b/src/app/containers/Figure/__snapshots__/index.test.jsx.snap @@ -34,7 +34,9 @@ exports[`Figure should render an image with alt text 1`] = ` Pauline Clayton
@@ -121,7 +123,9 @@ exports[`Figure should render caption and copyright 1`] = ` Pauline Clayton

( ratio={imageRatio} captionBlock={caption} copyright={copyright} + height={height} + width={width} /> ); diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index 3b18ee51ad6..55a92f5777a 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -16,17 +16,16 @@ const renderImage = (platform, alt, src, height, width) => { if (platform === 'amp') { return ; } - return {alt}; }; const FigureContainer = ({ - src, alt, - ratio, - copyright, captionBlock, + copyright, height, + ratio, + src, width, }) => (

@@ -40,17 +39,17 @@ const FigureContainer = ({ FigureContainer.propTypes = { alt: string.isRequired, + captionBlock: objectOf(any), + copyright: string, height: number.isRequired, - width: number.isRequired, - src: string.isRequired, ratio: number.isRequired, - copyright: string, - captionBlock: objectOf(any), + src: string.isRequired, + width: number.isRequired, }; FigureContainer.defaultProps = { - copyright: null, captionBlock: null, + copyright: null, }; export default FigureContainer; diff --git a/src/app/containers/Image/__snapshots__/index.test.jsx.snap b/src/app/containers/Image/__snapshots__/index.test.jsx.snap index e286af0e37d..3ce97b4c990 100644 --- a/src/app/containers/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/containers/Image/__snapshots__/index.test.jsx.snap @@ -5,8 +5,10 @@ exports[`Image with data should render an image with alt text 1`] = ` alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." captionBlock={null} copyright={null} + height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + width={640} /> `; @@ -45,8 +47,10 @@ exports[`Image with data should render an image with alt text and caption 1`] = } } copyright={null} + height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + width={640} /> `; @@ -55,8 +59,10 @@ exports[`Image with data should render an image with alt text and offscreen copy alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." captionBlock={null} copyright="Getty images" + height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + width={640} /> `; @@ -65,8 +71,10 @@ exports[`Image with data should render an image with other originCode - this wou alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." captionBlock={null} copyright="Getty images" + height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/other/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + width={640} /> `; @@ -75,8 +83,10 @@ exports[`Image with data should render an image with the default originCode \`cp alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." captionBlock={null} copyright="Getty images" + height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsdevpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + width={640} /> `; @@ -85,7 +95,9 @@ exports[`Image with data should render an image with the default originCode \`cp alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17." captionBlock={null} copyright="Getty images" + height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsdevpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + width={640} /> `; From a79ff57fdd2dd8be869d4f7244718263ae6109ba Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 09:04:16 +0000 Subject: [PATCH 11/27] Use platform context in figure container --- src/app/containers/Figure/fixtureData.jsx | 23 ++++++++++++++--------- src/app/containers/Figure/index.jsx | 19 ++++++++++++------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/app/containers/Figure/fixtureData.jsx b/src/app/containers/Figure/fixtureData.jsx index 4b73eacf377..89812d9a880 100644 --- a/src/app/containers/Figure/fixtureData.jsx +++ b/src/app/containers/Figure/fixtureData.jsx @@ -1,6 +1,7 @@ import React from 'react'; import FigureContainer from '.'; import { ServiceContext } from '../../contexts/ServiceContext'; +import { PlatformContextProvider } from '../../contexts/PlatformContext'; import { blockContainingText } from '../../models/blocks'; const imageAlt = 'Pauline Clayton'; @@ -69,17 +70,21 @@ const serviceContextStubNews = { imageCaptionOffscreenText: 'Image caption, ', }; +const platformContextCanonical = 'canonical'; + const generateFixtureData = (caption, copyright) => ( - + + + ); diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index 55a92f5777a..7568f63894b 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -6,6 +6,7 @@ import AmpImg from '../../components/Figure/Image/index.amp'; import ImagePlaceholder from '../../components/Figure/ImagePlaceholder'; import Copyright from '../Copyright'; import Caption from '../Caption'; +import { PlatformContextConsumer } from '../../contexts/PlatformContext'; const renderCopyright = copyright => copyright ? {copyright} : null; @@ -28,13 +29,17 @@ const FigureContainer = ({ src, width, }) => ( -
- - {renderImage('canonical', alt, src, height, width)} - {renderCopyright(copyright)} - - {renderCaption(captionBlock)} -
+ + {platform => ( +
+ + {renderImage(platform, alt, src, height, width)} + {renderCopyright(copyright)} + + {renderCaption(captionBlock)} +
+ )} + ); FigureContainer.propTypes = { From 921929a3dd258275818deceec31c33fcb11039dd Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 09:20:53 +0000 Subject: [PATCH 12/27] Use snapshotTests shared file for amp tests --- .../__snapshots__/index.amp.test.jsx.snap | 40 +++++++++--------- .../Figure/Image/index.amp.test.jsx | 41 +------------------ 2 files changed, 22 insertions(+), 59 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index 7c47aaed0b1..638244feb3d 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -1,61 +1,61 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Image - AmpImg should render 1:1 square image correctly 1`] = ` +exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = ` .c0 { display: block; width: 100%; } `; -exports[`Image - AmpImg should render 9:16 portrait image correctly 1`] = ` +exports[`Image - AmpImg should render landscape image correctly 1`] = ` .c0 { display: block; width: 100%; } `; -exports[`Image - AmpImg should render 16:9 landscape image correctly 1`] = ` +exports[`Image - AmpImg should render portrait image correctly 1`] = ` .c0 { display: block; width: 100%; } `; -exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = ` +exports[`Image - AmpImg should render square image correctly 1`] = ` .c0 { display: block; width: 100%; } `; diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx index 5cc0af6a9fe..57c9575214b 100644 --- a/src/app/components/Figure/Image/index.amp.test.jsx +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -1,43 +1,6 @@ -import React from 'react'; -import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; import AmpImg from './index.amp'; -import { custom, landscape, portrait, square } from './helpers/fixtureData'; +import snapshotTests from './helpers/snapshotTests'; describe('Image - AmpImg', () => { - shouldMatchSnapshot( - 'should render 16:9 landscape image correctly', - , - ); - shouldMatchSnapshot( - 'should render 9:16 portrait image correctly', - , - ); - shouldMatchSnapshot( - 'should render 1:1 square image correctly', - , - ); - shouldMatchSnapshot( - 'should render image with custom dimensions correctly', - , - ); + snapshotTests(AmpImg); }); From 58c0ec800e25dd21f4c5503bbe3057a1a609e693 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 09:55:17 +0000 Subject: [PATCH 13/27] Moving helpers to testHelpers directory --- package.json | 3 ++- src/app/components/Figure/Image/index.amp.stories.jsx | 2 +- src/app/components/Figure/Image/index.amp.test.jsx | 2 +- src/app/components/Figure/Image/index.stories.jsx | 2 +- src/app/components/Figure/Image/index.test.jsx | 2 +- .../Figure/Image/{helpers => testHelpers}/fixtureData.js | 0 .../Figure/Image/{helpers => testHelpers}/snapshotTests.jsx | 0 .../Figure/Image/{helpers => testHelpers}/stories.jsx | 0 8 files changed, 6 insertions(+), 5 deletions(-) rename src/app/components/Figure/Image/{helpers => testHelpers}/fixtureData.js (100%) rename src/app/components/Figure/Image/{helpers => testHelpers}/snapshotTests.jsx (100%) rename src/app/components/Figure/Image/{helpers => testHelpers}/stories.jsx (100%) diff --git a/package.json b/package.json index eaa851e0c2d..9cfff74fd55 100644 --- a/package.json +++ b/package.json @@ -132,7 +132,8 @@ "collectCoverageFrom": [ "**/(src|scripts)/**/*.{js,jsx}", "!**/src/app/helpers/tests/**", - "!**/*.stories.jsx" + "!**/*.stories.jsx", + "!**/testHelpers/*.{js,jsx}" ], "setupFiles": [ "./src/app/helpers/tests/jest-setup.js" diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx index cc35ec6f6f2..7a04a57633b 100644 --- a/src/app/components/Figure/Image/index.amp.stories.jsx +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -1,4 +1,4 @@ import AmpImg from './index.amp'; -import stories from './helpers/stories'; +import stories from './testHelpers/stories'; stories(AmpImg, 'Image - AmpImg'); diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx index 57c9575214b..cf5d86ccb2e 100644 --- a/src/app/components/Figure/Image/index.amp.test.jsx +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -1,5 +1,5 @@ import AmpImg from './index.amp'; -import snapshotTests from './helpers/snapshotTests'; +import snapshotTests from './testHelpers/snapshotTests'; describe('Image - AmpImg', () => { snapshotTests(AmpImg); diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index 34d7f9eeb25..6f3e0e85fa7 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -1,4 +1,4 @@ import { Img } from './index'; -import stories from './helpers/stories'; +import stories from './testHelpers/stories'; stories(Img, 'Image - Img'); diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index c1e5fd912a7..ba10298ac10 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -1,5 +1,5 @@ import Image, { Img } from './index'; -import snapshotTests from './helpers/snapshotTests'; +import snapshotTests from './testHelpers/snapshotTests'; describe("Image - imported as '{ Img }'", () => { snapshotTests(Img); diff --git a/src/app/components/Figure/Image/helpers/fixtureData.js b/src/app/components/Figure/Image/testHelpers/fixtureData.js similarity index 100% rename from src/app/components/Figure/Image/helpers/fixtureData.js rename to src/app/components/Figure/Image/testHelpers/fixtureData.js diff --git a/src/app/components/Figure/Image/helpers/snapshotTests.jsx b/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx similarity index 100% rename from src/app/components/Figure/Image/helpers/snapshotTests.jsx rename to src/app/components/Figure/Image/testHelpers/snapshotTests.jsx diff --git a/src/app/components/Figure/Image/helpers/stories.jsx b/src/app/components/Figure/Image/testHelpers/stories.jsx similarity index 100% rename from src/app/components/Figure/Image/helpers/stories.jsx rename to src/app/components/Figure/Image/testHelpers/stories.jsx From 39db323d291fd515e63437137dfc327b55e100a7 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 10:10:21 +0000 Subject: [PATCH 14/27] Update readme to reflect implementation --- src/app/components/Figure/Image/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/Figure/Image/README.md b/src/app/components/Figure/Image/README.md index c109382fc9c..ed240c97b1e 100644 --- a/src/app/components/Figure/Image/README.md +++ b/src/app/components/Figure/Image/README.md @@ -5,7 +5,7 @@ Importing the standard Image component, which renders an `` tag. ```jsx -import { Img } from '../components/Image'; +import Image from '../components/Image'; const WrappingContainer = ({ alt, height, src, width }) => ( {alt} @@ -15,7 +15,7 @@ const WrappingContainer = ({ alt, height, src, width }) => ( Importing an Amp Image component, which renders an `` tag. ```jsx -import { AmpImg } from '../components/Image'; +import AmpImg from '../components/Image/image.amp'; const WrappingContainer = ({ alt, height, src, width }) => ( From bc0d448cbeb1029d0328949de4e381dd26355abd Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 10:20:25 +0000 Subject: [PATCH 15/27] Update export for AmpImg --- src/app/components/Figure/Image/README.md | 2 +- src/app/components/Figure/Image/index.jsx | 3 +++ src/app/containers/Figure/index.jsx | 3 +-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app/components/Figure/Image/README.md b/src/app/components/Figure/Image/README.md index ed240c97b1e..4abe127aa18 100644 --- a/src/app/components/Figure/Image/README.md +++ b/src/app/components/Figure/Image/README.md @@ -15,7 +15,7 @@ const WrappingContainer = ({ alt, height, src, width }) => ( Importing an Amp Image component, which renders an `` tag. ```jsx -import AmpImg from '../components/Image/image.amp'; +import { AmpImg } from '../components/Image'; const WrappingContainer = ({ alt, height, src, width }) => ( diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 1c4239e5113..485322fc49d 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -18,6 +18,9 @@ Img.propTypes = { width: number.isRequired, }; +// eslint-disable-next-line global-require +export const AmpImg = require('./index.amp'); + const Image = Img; export default Image; diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index 7568f63894b..a8659bfd8a3 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -1,8 +1,7 @@ import React from 'react'; import { string, number, objectOf, any } from 'prop-types'; import Figure from '../../components/Figure'; -import Image from '../../components/Figure/Image'; -import AmpImg from '../../components/Figure/Image/index.amp'; +import Image, { AmpImg } from '../../components/Figure/Image'; import ImagePlaceholder from '../../components/Figure/ImagePlaceholder'; import Copyright from '../Copyright'; import Caption from '../Caption'; From aba7527e29fc17d9d16adbda6dd55d9bf35f7d52 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Tue, 18 Dec 2018 12:31:25 +0000 Subject: [PATCH 16/27] Moving StyledImg to another file to remove circular dependencies Fixes Amp page rendering --- .../Image/__snapshots__/styledImg.test.jsx.snap | 12 ++++++++++++ src/app/components/Figure/Image/index.amp.jsx | 2 +- src/app/components/Figure/Image/index.jsx | 10 ++-------- src/app/components/Figure/Image/styledImg.jsx | 8 ++++++++ src/app/components/Figure/Image/styledImg.test.jsx | 8 ++++++++ 5 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 src/app/components/Figure/Image/__snapshots__/styledImg.test.jsx.snap create mode 100644 src/app/components/Figure/Image/styledImg.jsx create mode 100644 src/app/components/Figure/Image/styledImg.test.jsx diff --git a/src/app/components/Figure/Image/__snapshots__/styledImg.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/styledImg.test.jsx.snap new file mode 100644 index 00000000000..ad2708a5fde --- /dev/null +++ b/src/app/components/Figure/Image/__snapshots__/styledImg.test.jsx.snap @@ -0,0 +1,12 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`StyledImg should render correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + + +`; diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index c1be418246c..a2073c37970 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -1,5 +1,5 @@ import 'styled-components'; -import { StyledImg } from '.'; +import StyledImg from './styledImg'; const AmpImg = StyledImg.withComponent('amp-img'); diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 485322fc49d..83723636274 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -1,11 +1,8 @@ import React from 'react'; -import styled from 'styled-components'; import { number, string } from 'prop-types'; +import StyledImg from './styledImg'; -export const StyledImg = styled.img` - display: block; - width: 100%; -`; +export { default as AmpImg } from './index.amp'; export const Img = ({ alt, height, src, width }) => ( @@ -18,9 +15,6 @@ Img.propTypes = { width: number.isRequired, }; -// eslint-disable-next-line global-require -export const AmpImg = require('./index.amp'); - const Image = Img; export default Image; diff --git a/src/app/components/Figure/Image/styledImg.jsx b/src/app/components/Figure/Image/styledImg.jsx new file mode 100644 index 00000000000..b1634b651e1 --- /dev/null +++ b/src/app/components/Figure/Image/styledImg.jsx @@ -0,0 +1,8 @@ +import styled from 'styled-components'; + +const StyledImg = styled.img` + display: block; + width: 100%; +`; + +export default StyledImg; diff --git a/src/app/components/Figure/Image/styledImg.test.jsx b/src/app/components/Figure/Image/styledImg.test.jsx new file mode 100644 index 00000000000..da27b39499f --- /dev/null +++ b/src/app/components/Figure/Image/styledImg.test.jsx @@ -0,0 +1,8 @@ +import React from 'react'; +import StyledImg from './styledImg'; + +import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; + +describe('StyledImg', () => { + shouldMatchSnapshot('should render correctly', ); +}); From a8493281be3b525c3aa3ee6ecbd055988b24dd53 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Wed, 19 Dec 2018 09:52:58 +0000 Subject: [PATCH 17/27] Refactor AmpImg to use styled-components 'as' polymorphic prop to use '' Add srcset prop to Img and AmpImg, ensuring it is optional --- .../__snapshots__/index.amp.test.jsx.snap | 24 +++++++++++--- .../Image/__snapshots__/index.test.jsx.snap | 32 +++++++++++++++++++ src/app/components/Figure/Image/index.amp.jsx | 32 +++++++++++++++++-- src/app/components/Figure/Image/index.jsx | 17 ++++++++-- .../Figure/Image/testHelpers/fixtureData.js | 3 ++ .../Image/testHelpers/snapshotTests.jsx | 10 ++++++ 6 files changed, 109 insertions(+), 9 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index 638244feb3d..42c3ee63431 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -8,13 +8,29 @@ exports[`Image - AmpImg should render image with custom dimensions correctly 1`] `; +exports[`Image - AmpImg should render image with srcset correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + + +`; + exports[`Image - AmpImg should render landscape image correctly 1`] = ` .c0 { display: block; @@ -23,7 +39,7 @@ exports[`Image - AmpImg should render landscape image correctly 1`] = ` `; +exports[`Image - imported as '{ Img }' should render image with srcset correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Student sitting an exam +`; + exports[`Image - imported as '{ Img }' should render landscape image correctly 1`] = ` .c0 { display: block; @@ -75,6 +91,22 @@ exports[`Image - imported as default 'Image' should render image with custom dim /> `; +exports[`Image - imported as default 'Image' should render image with srcset correctly 1`] = ` +.c0 { + display: block; + width: 100%; +} + +Student sitting an exam +`; + exports[`Image - imported as default 'Image' should render landscape image correctly 1`] = ` .c0 { display: block; diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index a2073c37970..474af83d0db 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -1,6 +1,34 @@ -import 'styled-components'; +import React from 'react'; +import { number, string } from 'prop-types'; import StyledImg from './styledImg'; -const AmpImg = StyledImg.withComponent('amp-img'); +const AmpImg = ({ alt, height, src, srcset, width }) => { + const props = { + as: 'amp-img', + layout: 'responsive', + alt, + src, + height, + width, + }; + + if (srcset) { + props.srcset = srcset; + } + + return ; +}; + +AmpImg.propTypes = { + alt: string.isRequired, + height: number.isRequired, + src: string.isRequired, + srcset: string, + width: number.isRequired, +}; + +AmpImg.defaultProps = { + srcset: null, +}; export default AmpImg; diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 83723636274..86f3fa84c6e 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -4,17 +4,28 @@ import StyledImg from './styledImg'; export { default as AmpImg } from './index.amp'; -export const Img = ({ alt, height, src, width }) => ( - -); +export const Img = ({ alt, height, src, srcset, width }) => { + const props = { alt, src, height, width }; + + if (srcset) { + props.srcset = srcset; + } + + return ; +}; Img.propTypes = { alt: string.isRequired, height: number.isRequired, src: string.isRequired, + srcset: string, width: number.isRequired, }; +Img.defaultProps = { + srcset: null, +}; + const Image = Img; export default Image; diff --git a/src/app/components/Figure/Image/testHelpers/fixtureData.js b/src/app/components/Figure/Image/testHelpers/fixtureData.js index 2fda09360e8..bee4be41c22 100644 --- a/src/app/components/Figure/Image/testHelpers/fixtureData.js +++ b/src/app/components/Figure/Image/testHelpers/fixtureData.js @@ -2,6 +2,9 @@ export const landscape = { alt: 'Student sitting an exam', src: 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg', + srcset: + 'https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w', + width: 1024, height: 576, }; diff --git a/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx b/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx index ccc7c8759ea..c55430f0d68 100644 --- a/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx +++ b/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx @@ -39,6 +39,16 @@ const snapshotTests = Component => { width={custom.width} />, ); + shouldMatchSnapshot( + 'should render image with srcset correctly', + , + ); }; export default snapshotTests; From 0b2f5298cf6d41353665d1a4906e1b6abc903808 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Wed, 19 Dec 2018 12:20:15 +0000 Subject: [PATCH 18/27] Add srcset prop to ImageContainer Remove conditional logic for originCode - it is now always expected to be passed by the CMS --- .../Image/__snapshots__/index.test.jsx.snap | 28 ++--------- src/app/containers/Image/index.jsx | 47 +++++++++++++----- src/app/containers/Image/index.test.jsx | 48 ------------------- 3 files changed, 38 insertions(+), 85 deletions(-) diff --git a/src/app/containers/Image/__snapshots__/index.test.jsx.snap b/src/app/containers/Image/__snapshots__/index.test.jsx.snap index 3ce97b4c990..5f4f3512d3f 100644 --- a/src/app/containers/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/containers/Image/__snapshots__/index.test.jsx.snap @@ -8,6 +8,7 @@ exports[`Image with data should render an image with alt text 1`] = ` height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + srcset="https://ichef.bbci.co.uk/news/270/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 270w, https://ichef.bbci.co.uk/news/320/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 320w, https://ichef.bbci.co.uk/news/480/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 480w, https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 640w, https://ichef.bbci.co.uk/news/900/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 900w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 1024w" width={640} /> `; @@ -50,6 +51,7 @@ exports[`Image with data should render an image with alt text and caption 1`] = height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + srcset="https://ichef.bbci.co.uk/news/270/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 270w, https://ichef.bbci.co.uk/news/320/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 320w, https://ichef.bbci.co.uk/news/480/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 480w, https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 640w, https://ichef.bbci.co.uk/news/900/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 900w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 1024w" width={640} /> `; @@ -62,6 +64,7 @@ exports[`Image with data should render an image with alt text and offscreen copy height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png" + srcset="https://ichef.bbci.co.uk/news/270/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 270w, https://ichef.bbci.co.uk/news/320/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 320w, https://ichef.bbci.co.uk/news/480/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 480w, https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 640w, https://ichef.bbci.co.uk/news/900/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 900w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png 1024w" width={640} /> `; @@ -74,30 +77,7 @@ exports[`Image with data should render an image with other originCode - this wou height={420} ratio={65.625} src="https://ichef.bbci.co.uk/news/640/other/439A/production/_100960371_syrians_and_asylum_v2-nc.png" - width={640} -/> -`; - -exports[`Image with data should render an image with the default originCode \`cpsdevpb\` - empty string 1`] = ` - -`; - -exports[`Image with data should render an image with the default originCode \`cpsdevpb\` - null 1`] = ` - `; diff --git a/src/app/containers/Image/index.jsx b/src/app/containers/Image/index.jsx index d79a11e3b8e..223340abc62 100644 --- a/src/app/containers/Image/index.jsx +++ b/src/app/containers/Image/index.jsx @@ -3,7 +3,21 @@ import { filterForBlockType } from '../../helpers/blockHandlers'; import { imageModelPropTypes } from '../../models/propTypes/image'; import Figure from '../Figure'; -const DEFAULT_IMAGE_RES = 640; +// TODO: this should be generalised to work for various products. +const DEFAULT_ICHEF_PRODUCT = 'news'; +const DEFAULT_WIDTH = 640; +const ARRAY_OF_WIDTHS = ['270', '320', '480', '640', '900', '1024']; + +const srcValue = (ichefProduct, width, originCode, locator) => + `https://ichef.bbci.co.uk/${ichefProduct}/${width}/${originCode}/${locator}`; + +const srcsetValue = (ichefProduct, widths, originCode, locator) => + widths + .map( + width => + `https://ichef.bbci.co.uk/${ichefProduct}/${width}/${originCode}/${locator} ${width}w`, + ) + .join(', '); const getText = ({ model }) => model.blocks[0].model.blocks[0].model.text; @@ -15,16 +29,6 @@ const getCopyright = copyrightHolder => { return copyrightHolder; }; -const getIChefURL = (originCode, locator) => { - // temp code - default to 'cpsdevpb' until Optimo complete work to supply non-empty originCode - const overridableOriginCode = originCode || 'cpsdevpb'; - - return `https://ichef.bbci.co.uk/news/${DEFAULT_IMAGE_RES}/${overridableOriginCode}/${locator}`; -}; - -const getRawImageSrc = (originCode, locator) => - originCode !== 'pips' ? getIChefURL(originCode, locator) : locator; - const ImageContainer = ({ blocks }) => { if (!blocks) { return null; @@ -48,16 +52,29 @@ const ImageContainer = ({ blocks }) => { const altText = getText(altTextBlock); const copyright = getCopyright(copyrightHolder); const ratio = (height / width) * 100; - const rawImageSrc = getRawImageSrc(originCode, locator); + const src = srcValue( + DEFAULT_ICHEF_PRODUCT, + DEFAULT_WIDTH, + originCode, + locator, + ); + + const srcset = srcsetValue( + DEFAULT_ICHEF_PRODUCT, + ARRAY_OF_WIDTHS, + originCode, + locator, + ); return (
); @@ -65,4 +82,8 @@ const ImageContainer = ({ blocks }) => { ImageContainer.propTypes = imageModelPropTypes; +ImageContainer.defaultProps = { + srcset: null, +}; + export default ImageContainer; diff --git a/src/app/containers/Image/index.test.jsx b/src/app/containers/Image/index.test.jsx index b038de6b214..2ddd15724fd 100644 --- a/src/app/containers/Image/index.test.jsx +++ b/src/app/containers/Image/index.test.jsx @@ -34,28 +34,6 @@ describe('Image', () => { }, }; - const rawImageBlockWithNullOriginCode = { - type: 'rawImage', - model: { - width: 640, - height: 420, - locator: '439A/production/_100960371_syrians_and_asylum_v2-nc.png', - originCode: null, - copyrightHolder: 'Getty images', - }, - }; - - const rawImageBlockWithEmptyStringOriginCode = { - type: 'rawImage', - model: { - width: 640, - height: 420, - locator: '439A/production/_100960371_syrians_and_asylum_v2-nc.png', - originCode: '', - copyrightHolder: 'Getty images', - }, - }; - const rawImageBlockWithOtherOriginCode = { type: 'rawImage', model: { @@ -130,32 +108,6 @@ describe('Image', () => { , ); - const dataWithNullOriginCode = blockArrayModel([ - rawImageBlockWithNullOriginCode, - blockContainingText( - 'altText', - 'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.', - ), - ]); - - shouldShallowMatchSnapshot( - 'should render an image with the default originCode `cpsdevpb` - null', - , - ); - - const dataWithEmptyStringOriginCode = blockArrayModel([ - rawImageBlockWithEmptyStringOriginCode, - blockContainingText( - 'altText', - 'Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17.', - ), - ]); - - shouldShallowMatchSnapshot( - 'should render an image with the default originCode `cpsdevpb` - empty string', - , - ); - const dataWithOtherOriginCode = blockArrayModel([ rawImageBlockWithOtherOriginCode, blockContainingText( From 13e46dc13e6f9218edac1f3fb4a4c6d7793028c2 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Wed, 19 Dec 2018 13:01:41 +0000 Subject: [PATCH 19/27] Add srcset prop to Figure container, to pass to Image component --- .../Figure/__snapshots__/index.test.jsx.snap | 5 ----- src/app/containers/Figure/index.jsx | 20 +++++++++++++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/app/containers/Figure/__snapshots__/index.test.jsx.snap b/src/app/containers/Figure/__snapshots__/index.test.jsx.snap index 5cacf5af4f2..f789ab24ac4 100644 --- a/src/app/containers/Figure/__snapshots__/index.test.jsx.snap +++ b/src/app/containers/Figure/__snapshots__/index.test.jsx.snap @@ -34,7 +34,6 @@ exports[`Figure should render an image with alt text 1`] = ` Pauline Clayton @@ -123,7 +122,6 @@ exports[`Figure should render caption and copyright 1`] = ` Pauline Clayton @@ -214,7 +212,6 @@ exports[`Figure should render caption text 1`] = ` Pauline Clayton @@ -324,7 +321,6 @@ exports[`Figure should render caption text with inline link 1`] = ` Pauline Clayton @@ -397,7 +393,6 @@ exports[`Figure should render copyright text 1`] = ` Pauline Clayton diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index a8659bfd8a3..a1ee5c9e58f 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -12,11 +12,20 @@ const renderCopyright = copyright => const renderCaption = block => (block ? : null); -const renderImage = (platform, alt, src, height, width) => { +const renderImage = (alt, height, platform, src, srcset, width) => { if (platform === 'amp') { - return ; + return ( + + ); } - return {alt}; + + return {alt}; }; const FigureContainer = ({ @@ -26,13 +35,14 @@ const FigureContainer = ({ height, ratio, src, + srcset, width, }) => ( {platform => (
- {renderImage(platform, alt, src, height, width)} + {renderImage(alt, height, platform, src, srcset, width)} {renderCopyright(copyright)} {renderCaption(captionBlock)} @@ -48,12 +58,14 @@ FigureContainer.propTypes = { height: number.isRequired, ratio: number.isRequired, src: string.isRequired, + srcset: string, width: number.isRequired, }; FigureContainer.defaultProps = { captionBlock: null, copyright: null, + srcset: null, }; export default FigureContainer; From d981939aea150abd240dcf123079c2930d52c874 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Wed, 19 Dec 2018 13:42:18 +0000 Subject: [PATCH 20/27] Reorder props, ensure height is required for AmpImg, tidy up imports/exports --- package.json | 2 +- src/app/components/Figure/Image/README.md | 4 ++-- src/app/components/Figure/Image/index.amp.jsx | 2 +- src/app/components/Figure/Image/index.jsx | 4 +--- src/app/components/Figure/Image/index.stories.jsx | 2 +- src/app/components/Figure/Image/index.test.jsx | 2 +- 6 files changed, 7 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 9cfff74fd55..3d93e3de117 100644 --- a/package.json +++ b/package.json @@ -133,7 +133,7 @@ "**/(src|scripts)/**/*.{js,jsx}", "!**/src/app/helpers/tests/**", "!**/*.stories.jsx", - "!**/testHelpers/*.{js,jsx}" + "!**/testHelpers/**" ], "setupFiles": [ "./src/app/helpers/tests/jest-setup.js" diff --git a/src/app/components/Figure/Image/README.md b/src/app/components/Figure/Image/README.md index 4abe127aa18..a707c5fae1c 100644 --- a/src/app/components/Figure/Image/README.md +++ b/src/app/components/Figure/Image/README.md @@ -8,7 +8,7 @@ Importing the standard Image component, which renders an `` tag. import Image from '../components/Image'; const WrappingContainer = ({ alt, height, src, width }) => ( - {alt} + {alt} ); ``` @@ -18,6 +18,6 @@ Importing an Amp Image component, which renders an `` tag. import { AmpImg } from '../components/Image'; const WrappingContainer = ({ alt, height, src, width }) => ( - + ); ``` diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index 474af83d0db..0d1a7aedf88 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -7,8 +7,8 @@ const AmpImg = ({ alt, height, src, srcset, width }) => { as: 'amp-img', layout: 'responsive', alt, - src, height, + src, width, }; diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 86f3fa84c6e..31c8bdb13bf 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -26,6 +26,4 @@ Img.defaultProps = { srcset: null, }; -const Image = Img; - -export default Image; +export default Img; diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index 6f3e0e85fa7..d6cd796e8e7 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -1,4 +1,4 @@ -import { Img } from './index'; +import { Img } from '.'; import stories from './testHelpers/stories'; stories(Img, 'Image - Img'); diff --git a/src/app/components/Figure/Image/index.test.jsx b/src/app/components/Figure/Image/index.test.jsx index ba10298ac10..e8392bd5d1e 100644 --- a/src/app/components/Figure/Image/index.test.jsx +++ b/src/app/components/Figure/Image/index.test.jsx @@ -1,4 +1,4 @@ -import Image, { Img } from './index'; +import Image, { Img } from '.'; import snapshotTests from './testHelpers/snapshotTests'; describe("Image - imported as '{ Img }'", () => { From 3ef134114cb87848437ef31ff18e17e509a52c02 Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Thu, 20 Dec 2018 10:50:29 +0000 Subject: [PATCH 21/27] Reorder props to group height width & ratio. Don't use ImagePlaceholder and Copyright for AmpImg Use React camelcased srcSet for attribute --- .../__snapshots__/index.amp.test.jsx.snap | 2 +- .../Image/__snapshots__/index.test.jsx.snap | 4 +- src/app/components/Figure/Image/index.amp.jsx | 6 +-- src/app/components/Figure/Image/index.jsx | 4 +- src/app/containers/Figure/index.jsx | 38 ++++++++++++++----- src/app/containers/Image/index.jsx | 4 +- 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index 42c3ee63431..e750fda3f07 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -26,7 +26,7 @@ exports[`Image - AmpImg should render image with srcset correctly 1`] = ` className="c0" height={576} src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg" - srcset="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w" + srcSet="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w" width={1024} /> `; diff --git a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap index 1cf1e66ac40..aa327cf3c08 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap @@ -26,7 +26,7 @@ exports[`Image - imported as '{ Img }' should render image with srcset correctly className="c0" height={576} src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg" - srcset="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w" + srcSet="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w" width={1024} /> `; @@ -102,7 +102,7 @@ exports[`Image - imported as default 'Image' should render image with srcset cor className="c0" height={576} src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg" - srcset="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w" + srcSet="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w, https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg 1024w" width={1024} /> `; diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index 0d1a7aedf88..ab9102894f2 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -7,13 +7,13 @@ const AmpImg = ({ alt, height, src, srcset, width }) => { as: 'amp-img', layout: 'responsive', alt, - height, src, + height, width, }; if (srcset) { - props.srcset = srcset; + props.srcSet = srcset; } return ; @@ -21,9 +21,9 @@ const AmpImg = ({ alt, height, src, srcset, width }) => { AmpImg.propTypes = { alt: string.isRequired, - height: number.isRequired, src: string.isRequired, srcset: string, + height: number.isRequired, width: number.isRequired, }; diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 31c8bdb13bf..5434c817ef6 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -8,7 +8,7 @@ export const Img = ({ alt, height, src, srcset, width }) => { const props = { alt, src, height, width }; if (srcset) { - props.srcset = srcset; + props.srcSet = srcset; } return ; @@ -16,9 +16,9 @@ export const Img = ({ alt, height, src, srcset, width }) => { Img.propTypes = { alt: string.isRequired, - height: number.isRequired, src: string.isRequired, srcset: string, + height: number.isRequired, width: number.isRequired, }; diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index a1ee5c9e58f..686c73990c3 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -12,20 +12,34 @@ const renderCopyright = copyright => const renderCaption = block => (block ? : null); -const renderImage = (alt, height, platform, src, srcset, width) => { +const renderImage = ( + alt, + copyright, + height, + platform, + ratio, + src, + srcset, + width, +) => { if (platform === 'amp') { return ( ); } - return {alt}; + return ( + + {alt} + {renderCopyright(copyright)} + + ); }; const FigureContainer = ({ @@ -41,10 +55,16 @@ const FigureContainer = ({ {platform => (
- - {renderImage(alt, height, platform, src, srcset, width)} - {renderCopyright(copyright)} - + {renderImage( + alt, + copyright, + height, + platform, + ratio, + src, + srcset, + width, + )} {renderCaption(captionBlock)}
)} @@ -55,11 +75,11 @@ FigureContainer.propTypes = { alt: string.isRequired, captionBlock: objectOf(any), copyright: string, - height: number.isRequired, - ratio: number.isRequired, src: string.isRequired, srcset: string, + height: number.isRequired, width: number.isRequired, + ratio: number.isRequired, }; FigureContainer.defaultProps = { diff --git a/src/app/containers/Image/index.jsx b/src/app/containers/Image/index.jsx index 223340abc62..a2dfdb2ccc0 100644 --- a/src/app/containers/Image/index.jsx +++ b/src/app/containers/Image/index.jsx @@ -69,13 +69,13 @@ const ImageContainer = ({ blocks }) => { return (
); }; From db7df3b1159e7801eb7d9c1dd209fe0aba8267ef Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Thu, 20 Dec 2018 11:20:06 +0000 Subject: [PATCH 22/27] Removing styles from AmpImg & refactoring --- src/app/components/Figure/Image/index.amp.jsx | 9 ++++----- src/app/components/Figure/Image/index.jsx | 9 +++++++-- src/app/components/Figure/Image/styledImg.jsx | 8 -------- src/app/components/Figure/Image/styledImg.test.jsx | 8 -------- src/app/containers/Figure/index.jsx | 1 + 5 files changed, 12 insertions(+), 23 deletions(-) delete mode 100644 src/app/components/Figure/Image/styledImg.jsx delete mode 100644 src/app/components/Figure/Image/styledImg.test.jsx diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index ab9102894f2..8a66ad3917a 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -1,11 +1,9 @@ import React from 'react'; import { number, string } from 'prop-types'; -import StyledImg from './styledImg'; -const AmpImg = ({ alt, height, src, srcset, width }) => { +const AmpImg = ({ layout, alt, src, srcset, height, width }) => { const props = { - as: 'amp-img', - layout: 'responsive', + layout, alt, src, height, @@ -16,7 +14,7 @@ const AmpImg = ({ alt, height, src, srcset, width }) => { props.srcSet = srcset; } - return ; + return ; }; AmpImg.propTypes = { @@ -25,6 +23,7 @@ AmpImg.propTypes = { srcset: string, height: number.isRequired, width: number.isRequired, + layout: string.isRequired, }; AmpImg.defaultProps = { diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 5434c817ef6..3adbe214b1e 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -1,10 +1,15 @@ import React from 'react'; import { number, string } from 'prop-types'; -import StyledImg from './styledImg'; +import styled from 'styled-components'; export { default as AmpImg } from './index.amp'; -export const Img = ({ alt, height, src, srcset, width }) => { +const StyledImg = styled.img` + display: block; + width: 100%; +`; + +export const Img = ({ alt, src, srcset, height, width }) => { const props = { alt, src, height, width }; if (srcset) { diff --git a/src/app/components/Figure/Image/styledImg.jsx b/src/app/components/Figure/Image/styledImg.jsx deleted file mode 100644 index b1634b651e1..00000000000 --- a/src/app/components/Figure/Image/styledImg.jsx +++ /dev/null @@ -1,8 +0,0 @@ -import styled from 'styled-components'; - -const StyledImg = styled.img` - display: block; - width: 100%; -`; - -export default StyledImg; diff --git a/src/app/components/Figure/Image/styledImg.test.jsx b/src/app/components/Figure/Image/styledImg.test.jsx deleted file mode 100644 index da27b39499f..00000000000 --- a/src/app/components/Figure/Image/styledImg.test.jsx +++ /dev/null @@ -1,8 +0,0 @@ -import React from 'react'; -import StyledImg from './styledImg'; - -import { shouldMatchSnapshot } from '../../../helpers/tests/testHelpers'; - -describe('StyledImg', () => { - shouldMatchSnapshot('should render correctly', ); -}); diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index ef38615825c..7da7b7b8833 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -28,6 +28,7 @@ const renderImage = ( alt={alt} src={src} srcset={srcset} + layout="responsive" height={height} width={width} /> From a7303a66b7e2084f78c2d78ae981ac3b55d1e34b Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Thu, 20 Dec 2018 11:32:19 +0000 Subject: [PATCH 23/27] Update snapshots & fix ordering --- .../__snapshots__/index.amp.test.jsx.snap | 30 ------------------- .../__snapshots__/styledImg.test.jsx.snap | 12 -------- src/app/components/Figure/Image/index.amp.jsx | 6 ++-- src/app/components/Figure/Image/index.jsx | 3 +- .../Figure/__snapshots__/index.test.jsx.snap | 5 ++++ 5 files changed, 10 insertions(+), 46 deletions(-) delete mode 100644 src/app/components/Figure/Image/__snapshots__/styledImg.test.jsx.snap diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index e750fda3f07..444e1407271 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -1,14 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = ` -.c0 { - display: block; - width: 100%; -} - -`; diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index 8a66ad3917a..e6220745d56 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -1,10 +1,10 @@ import React from 'react'; import { number, string } from 'prop-types'; -const AmpImg = ({ layout, alt, src, srcset, height, width }) => { +const AmpImg = ({ alt, layout, src, srcset, height, width }) => { const props = { - layout, alt, + layout, src, height, width, @@ -19,11 +19,11 @@ const AmpImg = ({ layout, alt, src, srcset, height, width }) => { AmpImg.propTypes = { alt: string.isRequired, + layout: string.isRequired, src: string.isRequired, srcset: string, height: number.isRequired, width: number.isRequired, - layout: string.isRequired, }; AmpImg.defaultProps = { diff --git a/src/app/components/Figure/Image/index.jsx b/src/app/components/Figure/Image/index.jsx index 3adbe214b1e..04daacf85fb 100644 --- a/src/app/components/Figure/Image/index.jsx +++ b/src/app/components/Figure/Image/index.jsx @@ -23,11 +23,12 @@ Img.propTypes = { alt: string.isRequired, src: string.isRequired, srcset: string, - height: number.isRequired, + height: number, width: number.isRequired, }; Img.defaultProps = { + height: null, srcset: null, }; diff --git a/src/app/containers/Figure/__snapshots__/index.test.jsx.snap b/src/app/containers/Figure/__snapshots__/index.test.jsx.snap index 887079ba2e4..631c6c923e9 100644 --- a/src/app/containers/Figure/__snapshots__/index.test.jsx.snap +++ b/src/app/containers/Figure/__snapshots__/index.test.jsx.snap @@ -34,6 +34,7 @@ exports[`Figure should render an image with alt text 1`] = ` Pauline Clayton @@ -122,6 +123,7 @@ exports[`Figure should render caption and copyright 1`] = ` Pauline Clayton @@ -212,6 +214,7 @@ exports[`Figure should render caption text 1`] = ` Pauline Clayton @@ -321,6 +324,7 @@ exports[`Figure should render caption text with inline link 1`] = ` Pauline Clayton @@ -393,6 +397,7 @@ exports[`Figure should render copyright text 1`] = ` Pauline Clayton From 89002afdc1dc1d6282fafcf8fd548f4389652cdf Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Thu, 20 Dec 2018 11:33:44 +0000 Subject: [PATCH 24/27] Add attribution attribute to amp-img --- src/app/components/Figure/Image/index.amp.jsx | 4 +++- src/app/containers/Figure/index.jsx | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/app/components/Figure/Image/index.amp.jsx b/src/app/components/Figure/Image/index.amp.jsx index e6220745d56..30322d25373 100644 --- a/src/app/components/Figure/Image/index.amp.jsx +++ b/src/app/components/Figure/Image/index.amp.jsx @@ -1,9 +1,10 @@ import React from 'react'; import { number, string } from 'prop-types'; -const AmpImg = ({ alt, layout, src, srcset, height, width }) => { +const AmpImg = ({ alt, attribution, layout, src, srcset, height, width }) => { const props = { alt, + attribution, layout, src, height, @@ -19,6 +20,7 @@ const AmpImg = ({ alt, layout, src, srcset, height, width }) => { AmpImg.propTypes = { alt: string.isRequired, + attribution: string.isRequired, layout: string.isRequired, src: string.isRequired, srcset: string, diff --git a/src/app/containers/Figure/index.jsx b/src/app/containers/Figure/index.jsx index 7da7b7b8833..76fe2356a34 100644 --- a/src/app/containers/Figure/index.jsx +++ b/src/app/containers/Figure/index.jsx @@ -25,6 +25,7 @@ const renderImage = ( if (platform === 'amp') { return ( Date: Thu, 20 Dec 2018 12:02:37 +0000 Subject: [PATCH 25/27] Add tests for layout props for AmpImg & update storybook stories to not pass in height --- .../__snapshots__/index.amp.test.jsx.snap | 4 ++++ .../Figure/Image/index.amp.stories.jsx | 6 +++++- .../Figure/Image/index.amp.test.jsx | 6 +++++- .../Image/testHelpers/snapshotTests.jsx | 6 +++++- .../Figure/Image/testHelpers/stories.jsx | 19 ++++++++++++++----- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap index 444e1407271..b97bf984a94 100644 --- a/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap +++ b/src/app/components/Figure/Image/__snapshots__/index.amp.test.jsx.snap @@ -4,6 +4,7 @@ exports[`Image - AmpImg should render image with custom dimensions correctly 1`] @@ -23,6 +24,7 @@ exports[`Image - AmpImg should render landscape image correctly 1`] = ` @@ -32,6 +34,7 @@ exports[`Image - AmpImg should render portrait image correctly 1`] = ` @@ -41,6 +44,7 @@ exports[`Image - AmpImg should render square image correctly 1`] = ` diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx index 7a04a57633b..8c6fd842d2b 100644 --- a/src/app/components/Figure/Image/index.amp.stories.jsx +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -1,4 +1,8 @@ import AmpImg from './index.amp'; import stories from './testHelpers/stories'; -stories(AmpImg, 'Image - AmpImg'); +const additionalProps = { + layout: 'responsive', +}; + +stories(AmpImg, 'Image - AmpImg', additionalProps); diff --git a/src/app/components/Figure/Image/index.amp.test.jsx b/src/app/components/Figure/Image/index.amp.test.jsx index cf5d86ccb2e..32f13b789c2 100644 --- a/src/app/components/Figure/Image/index.amp.test.jsx +++ b/src/app/components/Figure/Image/index.amp.test.jsx @@ -2,5 +2,9 @@ import AmpImg from './index.amp'; import snapshotTests from './testHelpers/snapshotTests'; describe('Image - AmpImg', () => { - snapshotTests(AmpImg); + const additionalProps = { + layout: 'responsive', + }; + + snapshotTests(AmpImg, additionalProps); }); diff --git a/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx b/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx index c55430f0d68..6a1dcc5bdb6 100644 --- a/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx +++ b/src/app/components/Figure/Image/testHelpers/snapshotTests.jsx @@ -2,7 +2,7 @@ import React from 'react'; import { shouldMatchSnapshot } from '../../../../helpers/tests/testHelpers'; import { custom, landscape, portrait, square } from './fixtureData'; -const snapshotTests = Component => { +const snapshotTests = (Component, additionalProps) => { shouldMatchSnapshot( 'should render landscape image correctly', { src={landscape.src} height={landscape.height} width={landscape.width} + {...additionalProps} />, ); shouldMatchSnapshot( @@ -19,6 +20,7 @@ const snapshotTests = Component => { src={portrait.src} height={portrait.height} width={portrait.width} + {...additionalProps} />, ); shouldMatchSnapshot( @@ -28,6 +30,7 @@ const snapshotTests = Component => { src={square.src} height={square.height} width={square.width} + {...additionalProps} />, ); shouldMatchSnapshot( @@ -37,6 +40,7 @@ const snapshotTests = Component => { src={custom.src} height={custom.height} width={custom.width} + {...additionalProps} />, ); shouldMatchSnapshot( diff --git a/src/app/components/Figure/Image/testHelpers/stories.jsx b/src/app/components/Figure/Image/testHelpers/stories.jsx index 4ee81f426a6..7714603d539 100644 --- a/src/app/components/Figure/Image/testHelpers/stories.jsx +++ b/src/app/components/Figure/Image/testHelpers/stories.jsx @@ -2,38 +2,47 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import { custom, landscape, portrait, square } from './fixtureData'; -const stories = (Component, title) => +const stories = (Component, title, additionalProps) => storiesOf(title, module) .add('16:9 landscape image', () => ( )) .add('9:16 portrait image', () => ( )) .add('1:1 square image', () => ( )) .add('custom ratio image', () => ( + )) + .add('image with srcset', () => ( + )); From 8399629f480f7b52c7f1312a76110b9cab24e11c Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 24 Dec 2018 09:02:16 +0000 Subject: [PATCH 26/27] Move global styles decorator into CanonicalDecorator component --- .storybook/config.js | 12 ++---------- .../__snapshots__/index.test.jsx.snap | 10 ++++++++++ .../helpers/storybook/canonicalDecorator/index.jsx | 11 +++++++++++ .../storybook/canonicalDecorator/index.test.jsx | 9 +++++++++ 4 files changed, 32 insertions(+), 10 deletions(-) create mode 100644 src/app/helpers/storybook/canonicalDecorator/__snapshots__/index.test.jsx.snap create mode 100644 src/app/helpers/storybook/canonicalDecorator/index.jsx create mode 100644 src/app/helpers/storybook/canonicalDecorator/index.test.jsx diff --git a/.storybook/config.js b/.storybook/config.js index ef1229fe1a0..8bca2aef343 100644 --- a/.storybook/config.js +++ b/.storybook/config.js @@ -1,6 +1,5 @@ -import React, { Fragment } from 'react'; import { configure, addDecorator } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies -import GlobalStyle from '../src/app/lib/globalStyles'; +import CanonicalDecorator from '../src/app/helpers/storybook/canonicalDecorator'; const req = require.context('../src/app', true, /\.stories\.jsx$/); @@ -8,13 +7,6 @@ function loadStories() { req.keys().forEach(filename => req(filename)); } -addDecorator(story => ( - /* eslint-disable react/jsx-filename-extension */ - - - {story()} - - /* eslint-enable react/jsx-filename-extension */ -)); +addDecorator(CanonicalDecorator); configure(loadStories, module); diff --git a/src/app/helpers/storybook/canonicalDecorator/__snapshots__/index.test.jsx.snap b/src/app/helpers/storybook/canonicalDecorator/__snapshots__/index.test.jsx.snap new file mode 100644 index 00000000000..183018f940c --- /dev/null +++ b/src/app/helpers/storybook/canonicalDecorator/__snapshots__/index.test.jsx.snap @@ -0,0 +1,10 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`CanonicalDecorator should render correctly 1`] = ` + + + + Text here + + +`; diff --git a/src/app/helpers/storybook/canonicalDecorator/index.jsx b/src/app/helpers/storybook/canonicalDecorator/index.jsx new file mode 100644 index 00000000000..7bd5d43745e --- /dev/null +++ b/src/app/helpers/storybook/canonicalDecorator/index.jsx @@ -0,0 +1,11 @@ +import React from 'react'; +import GlobalStyle from '../../../lib/globalStyles'; + +const CanonicalDecorator = storyFn => ( + + + {storyFn()} + +); + +export default CanonicalDecorator; diff --git a/src/app/helpers/storybook/canonicalDecorator/index.test.jsx b/src/app/helpers/storybook/canonicalDecorator/index.test.jsx new file mode 100644 index 00000000000..4b510f32582 --- /dev/null +++ b/src/app/helpers/storybook/canonicalDecorator/index.test.jsx @@ -0,0 +1,9 @@ +import React from 'react'; +import CanonicalDecorator from '.'; + +describe('CanonicalDecorator', () => { + it('should render correctly', () => { + const component = CanonicalDecorator(() => Text here); + expect(component).toMatchSnapshot(); + }); +}); From 974b8ec2bb929f2850df1a76391848472d2f198c Mon Sep 17 00:00:00 2001 From: Sareh Heidari Date: Mon, 24 Dec 2018 09:02:58 +0000 Subject: [PATCH 27/27] Use CanonicalDecorator for Img and AmpDecorator for AmpImg --- .../Figure/Image/index.amp.stories.jsx | 32 +++++++++--- .../components/Figure/Image/index.stories.jsx | 16 +++++- .../Figure/Image/testHelpers/getImageProps.js | 11 +++++ .../Figure/Image/testHelpers/stories.jsx | 49 ------------------- 4 files changed, 51 insertions(+), 57 deletions(-) create mode 100644 src/app/components/Figure/Image/testHelpers/getImageProps.js delete mode 100644 src/app/components/Figure/Image/testHelpers/stories.jsx diff --git a/src/app/components/Figure/Image/index.amp.stories.jsx b/src/app/components/Figure/Image/index.amp.stories.jsx index 8c6fd842d2b..2029ef8501c 100644 --- a/src/app/components/Figure/Image/index.amp.stories.jsx +++ b/src/app/components/Figure/Image/index.amp.stories.jsx @@ -1,8 +1,28 @@ +import React from 'react'; +import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies import AmpImg from './index.amp'; -import stories from './testHelpers/stories'; +import AmpDecorator from '../../../helpers/storybook/ampDecorator'; +import { custom, landscape, portrait, square } from './testHelpers/fixtureData'; +import getImageProps from './testHelpers/getImageProps'; -const additionalProps = { - layout: 'responsive', -}; - -stories(AmpImg, 'Image - AmpImg', additionalProps); +storiesOf('Image - AmpImg', module) + .addDecorator(AmpDecorator) + .add('16:9 landscape image', () => ( + + )) + .add('9:16 portrait image', () => ( + + )) + .add('1:1 square image', () => ( + + )) + .add('custom ratio image', () => ( + + )) + .add('image with srcset', () => ( + + )); diff --git a/src/app/components/Figure/Image/index.stories.jsx b/src/app/components/Figure/Image/index.stories.jsx index d6cd796e8e7..f26bebee2a0 100644 --- a/src/app/components/Figure/Image/index.stories.jsx +++ b/src/app/components/Figure/Image/index.stories.jsx @@ -1,4 +1,16 @@ +import React from 'react'; +import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies +import CanonicalDecorator from '../../../helpers/storybook/canonicalDecorator'; import { Img } from '.'; -import stories from './testHelpers/stories'; +import { custom, landscape, portrait, square } from './testHelpers/fixtureData'; +import getImageProps from './testHelpers/getImageProps'; -stories(Img, 'Image - Img'); +storiesOf('Image - Img', module) + .addDecorator(CanonicalDecorator) + .add('16:9 landscape image', () => ) + .add('9:16 portrait image', () => ) + .add('1:1 square image', () => ) + .add('custom ratio image', () => ) + .add('image with srcset', () => ( + + )); diff --git a/src/app/components/Figure/Image/testHelpers/getImageProps.js b/src/app/components/Figure/Image/testHelpers/getImageProps.js new file mode 100644 index 00000000000..eb8e4d6cd59 --- /dev/null +++ b/src/app/components/Figure/Image/testHelpers/getImageProps.js @@ -0,0 +1,11 @@ +function getImageProps(image, includeHeight) { + const props = { + alt: image.alt, + src: image.src, + width: image.width, + }; + props.height = includeHeight ? image.height : null; + return props; +} + +export default getImageProps; diff --git a/src/app/components/Figure/Image/testHelpers/stories.jsx b/src/app/components/Figure/Image/testHelpers/stories.jsx deleted file mode 100644 index 7714603d539..00000000000 --- a/src/app/components/Figure/Image/testHelpers/stories.jsx +++ /dev/null @@ -1,49 +0,0 @@ -import React from 'react'; -import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies -import { custom, landscape, portrait, square } from './fixtureData'; - -const stories = (Component, title, additionalProps) => - storiesOf(title, module) - .add('16:9 landscape image', () => ( - - )) - .add('9:16 portrait image', () => ( - - )) - .add('1:1 square image', () => ( - - )) - .add('custom ratio image', () => ( - - )) - .add('image with srcset', () => ( - - )); - -export default stories;