Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amp Img #1101

Closed
wants to merge 32 commits into from
Closed

Amp Img #1101

Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
206e4b8
Update Image component to include AmpImg, which extends the canonical…
sareh Dec 17, 2018
2df0125
Use Img in Figure Container
sareh Dec 17, 2018
aafe6c3
Use Image in Figure Container
sareh Dec 17, 2018
4e18239
Add tests for default export/import
sareh Dec 17, 2018
8b5da7b
Merge branch 'latest' into amp-img
sareh Dec 17, 2018
5c6a973
Refactor tests to remove duplication
sareh Dec 17, 2018
ed6791f
Correct image dimensions and examples
sareh Dec 17, 2018
da39b3d
Correct image dimensions and examples & refactor fixtureData exports
sareh Dec 17, 2018
3dc4bbf
Use withComponent syntax for AmpImg extension
sareh Dec 17, 2018
5121fbc
Refactor tests & stories & update snapshots
sareh Dec 18, 2018
fec261b
Update Figure Container to include height and width properties
sareh Dec 18, 2018
a79ff57
Use platform context in figure container
sareh Dec 18, 2018
766a0e5
Merge branch 'latest' into amp-img
sareh Dec 18, 2018
921929a
Use snapshotTests shared file for amp tests
sareh Dec 18, 2018
58c0ec8
Moving helpers to testHelpers directory
sareh Dec 18, 2018
39db323
Update readme to reflect implementation
sareh Dec 18, 2018
bc0d448
Update export for AmpImg
sareh Dec 18, 2018
aba7527
Moving StyledImg to another file to remove circular dependencies
sareh Dec 18, 2018
a849328
Refactor AmpImg to use styled-components 'as' polymorphic prop to use…
sareh Dec 19, 2018
0b2f529
Add srcset prop to ImageContainer
sareh Dec 19, 2018
13e46dc
Add srcset prop to Figure container, to pass to Image component
sareh Dec 19, 2018
d981939
Reorder props, ensure height is required for AmpImg, tidy up imports/…
sareh Dec 19, 2018
ff2bd2a
Merge branch 'latest' into amp-img
sareh Dec 20, 2018
3ef1341
Reorder props to group height width & ratio.
sareh Dec 20, 2018
baa6570
Merge branch 'latest' into amp-img
sareh Dec 20, 2018
a166b7c
Merge branch 'amp-img' of github.com:bbc/simorgh into amp-img
sareh Dec 20, 2018
db7df3b
Removing styles from AmpImg & refactoring
sareh Dec 20, 2018
a7303a6
Update snapshots & fix ordering
sareh Dec 20, 2018
89002af
Add attribution attribute to amp-img
sareh Dec 20, 2018
7e291e3
Add tests for layout props for AmpImg & update storybook stories to n…
sareh Dec 20, 2018
8399629
Move global styles decorator into CanonicalDecorator component
sareh Dec 24, 2018
974b8ec
Use CanonicalDecorator for Img and AmpDecorator for AmpImg
sareh Dec 24, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@
"collectCoverageFrom": [
"**/(src|scripts)/**/*.{js,jsx}",
"!**/src/app/helpers/tests/**",
"!**/*.stories.jsx"
"!**/*.stories.jsx",
"!**/testHelpers/**"
],
"setupFiles": [
"./src/app/helpers/tests/jest-setup.js"
Expand Down
23 changes: 23 additions & 0 deletions src/app/components/Figure/Image/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Image component

## Usage

Importing the standard Image component, which renders an `<img />` tag.

```jsx
import Image from '../components/Image';

const WrappingContainer = ({ alt, height, src, width }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const WrappingContainer = ({ alt, height, src, width }) => (
const WrappingContainer = ({ alt, src, height, width }) => (

(makes it consistent with the other below)

<Img alt={alt} height={height} src={src} width={width} />
);
```

Importing an Amp Image component, which renders an `<amp-img />` tag.

```jsx
import { AmpImg } from '../components/Image';

const WrappingContainer = ({ alt, height, src, width }) => (
<AmpImg alt={alt} height={height} src={src} width={width} />
);
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs the rest of the README structure (guess I could have raised an issue for that!), e.g. https://github.com/BBC-News/psammead/tree/latest/packages/components/psammead-paragraph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CoC and other links will need to change when we move it to Psammead. I've added a note to the description in this Issue: BBC-archive/psammead#203

Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Image - AmpImg should render image with custom dimensions correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<amp-img
alt="By Elisa Decker, from her series \\"Sidewalk\\""
className="c0"
height={547}
src="https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg"
width={445}
/>
`;

exports[`Image - AmpImg should render image with srcset correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<amp-img
alt="Student sitting an exam"
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"
width={1024}
/>
`;

exports[`Image - AmpImg should render landscape image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<amp-img
alt="Student sitting an exam"
className="c0"
height={576}
src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg"
width={1024}
/>
`;

exports[`Image - AmpImg should render portrait image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<amp-img
alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17."
className="c0"
height={1138}
src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png"
width={640}
/>
`;

exports[`Image - AmpImg should render square image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<amp-img
alt="Tracks through the snow"
className="c0"
height={1024}
src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/114FE/production/_104801907_79010.jpg"
width={1024}
/>
`;
141 changes: 140 additions & 1 deletion src/app/components/Figure/Image/__snapshots__/index.test.jsx.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,128 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Image should render correctly 1`] = `
exports[`Image - imported as '{ Img }' should render image with custom dimensions correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="By Elisa Decker, from her series \\"Sidewalk\\""
className="c0"
height={547}
src="https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg"
width={445}
/>
`;

exports[`Image - imported as '{ Img }' should render image with srcset correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Student sitting an exam"
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"
width={1024}
/>
`;

exports[`Image - imported as '{ Img }' should render landscape image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Student sitting an exam"
className="c0"
height={576}
src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg"
width={1024}
/>
`;

exports[`Image - imported as '{ Img }' should render portrait image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17."
className="c0"
height={1138}
src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png"
width={640}
/>
`;

exports[`Image - imported as '{ Img }' should render square image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Tracks through the snow"
className="c0"
height={1024}
src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/114FE/production/_104801907_79010.jpg"
width={1024}
/>
`;

exports[`Image - imported as default 'Image' should render image with custom dimensions correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="By Elisa Decker, from her series \\"Sidewalk\\""
className="c0"
height={547}
src="https://ichef.bbci.co.uk/news/445/cpsprodpb/164BB/production/_104032319_03270dcc-9dda-4bd4-96a0-db89f6b915ae.jpg"
width={445}
/>
`;

exports[`Image - imported as default 'Image' should render image with srcset correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Student sitting an exam"
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"
width={1024}
/>
`;

exports[`Image - imported as default 'Image' should render landscape image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Student sitting an exam"
className="c0"
height={576}
src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/7098/production/_104842882_students.jpg"
width={1024}
/>
`;

exports[`Image - imported as default 'Image' should render portrait image correctly 1`] = `
.c0 {
display: block;
width: 100%;
Expand All @@ -9,6 +131,23 @@ exports[`Image should render correctly 1`] = `
<img
alt="Map of the UK displaying Syrian refugees and asylum seekers per 10000 population. Ranges from 0 to 17."
className="c0"
height={1138}
src="https://ichef.bbci.co.uk/news/640/cpsprodpb/439A/production/_100960371_syrians_and_asylum_v2-nc.png"
width={640}
/>
`;

exports[`Image - imported as default 'Image' should render square image correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
alt="Tracks through the snow"
className="c0"
height={1024}
src="https://ichef.bbci.co.uk/news/1024/cpsprodpb/114FE/production/_104801907_79010.jpg"
width={1024}
/>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`StyledImg should render correctly 1`] = `
.c0 {
display: block;
width: 100%;
}

<img
className="c0"
/>
`;
34 changes: 34 additions & 0 deletions src/app/components/Figure/Image/index.amp.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from 'react';
import { number, string } from 'prop-types';
import StyledImg from './styledImg';

const AmpImg = ({ alt, height, src, srcset, width }) => {
const props = {
as: 'amp-img',
layout: 'responsive',
alt,
src,
height,
width,
};

if (srcset) {
props.srcSet = srcset;
}

return <StyledImg {...props} />;
};

AmpImg.propTypes = {
alt: string.isRequired,
src: string.isRequired,
srcset: string,
height: number.isRequired,
width: number.isRequired,
};

AmpImg.defaultProps = {
srcset: null,
};

export default AmpImg;
4 changes: 4 additions & 0 deletions src/app/components/Figure/Image/index.amp.stories.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import AmpImg from './index.amp';
import stories from './testHelpers/stories';

stories(AmpImg, 'Image - AmpImg');
6 changes: 6 additions & 0 deletions src/app/components/Figure/Image/index.amp.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import AmpImg from './index.amp';
import snapshotTests from './testHelpers/snapshotTests';

describe('Image - AmpImg', () => {
sareh marked this conversation as resolved.
Show resolved Hide resolved
sareh marked this conversation as resolved.
Show resolved Hide resolved
snapshotTests(AmpImg);
});
33 changes: 27 additions & 6 deletions src/app/components/Figure/Image/index.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,29 @@
import styled from 'styled-components';
import React from 'react';
import { number, string } from 'prop-types';
import StyledImg from './styledImg';

const Image = styled.img`
display: block;
width: 100%;
`;
export { default as AmpImg } from './index.amp';

export default Image;
export const Img = ({ alt, height, src, srcset, width }) => {
const props = { alt, src, height, width };

if (srcset) {
props.srcSet = srcset;
}

return <StyledImg {...props} />;
};

Img.propTypes = {
alt: string.isRequired,
src: string.isRequired,
ChrisBAshton marked this conversation as resolved.
Show resolved Hide resolved
srcset: string,
height: number.isRequired,
width: number.isRequired,
};

Img.defaultProps = {
srcset: null,
};

export default Img;
14 changes: 3 additions & 11 deletions src/app/components/Figure/Image/index.stories.jsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
import React from 'react';
import { storiesOf } from '@storybook/react'; // eslint-disable-line import/no-extraneous-dependencies
import Image from './index';
import { Img } from '.';
import stories from './testHelpers/stories';

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

storiesOf('Image', module).add('default', () => (
<Image alt={imageAlt} src={imageSrc} />
));
stories(Img, 'Image - Img');
Loading