From 8c99439748dea42270535c8476fb0e659c41364e Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Mon, 28 Oct 2019 16:15:03 +0200 Subject: [PATCH 01/17] Add notes to psammead-media-player stories. --- .../src/index.stories.jsx | 105 +++++++++++------- 1 file changed, 64 insertions(+), 41 deletions(-) diff --git a/packages/components/psammead-media-player/src/index.stories.jsx b/packages/components/psammead-media-player/src/index.stories.jsx index 6ce51bf075..1ceb5d3208 100644 --- a/packages/components/psammead-media-player/src/index.stories.jsx +++ b/packages/components/psammead-media-player/src/index.stories.jsx @@ -2,6 +2,7 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; import { CanonicalMediaPlayer, AmpMediaPlayer } from '.'; import { ampDecorator } from '../../../../.storybook/config'; +import notes from '../README.md'; const withDuration = { duration: '2:30', @@ -9,54 +10,76 @@ const withDuration = { datetime: 'PT2M30S', }; -storiesOf('Components|Media Player', module).add('Default', () => ( - -)); - -storiesOf('Components|Media Player', module).add('Audio', () => ( - -)); +storiesOf('Components|Media Player', module).add( + 'Default', + () => ( + + ), + { notes, knobs: { escapeHTML: false } }, +); -storiesOf('Components|Media Player', module) - .addDecorator(ampDecorator) - .add('AMP', () => ( - ( + - )); - -storiesOf('Components|Media Player', module).add('Audio Skin', () => ( - -)); + ), + { notes, knobs: { escapeHTML: false } }, +); storiesOf('Components|Media Player', module) .addDecorator(ampDecorator) - .add('Audio Skin AMP', () => ( - ( + + ), + { notes, knobs: { escapeHTML: false } }, + ); + +storiesOf('Components|Media Player', module).add( + 'Audio Skin', + () => ( + - )); + ), + { notes, knobs: { escapeHTML: false } }, +); + +storiesOf('Components|Media Player', module) + .addDecorator(ampDecorator) + .add( + 'Audio Skin AMP', + () => ( + + ), + { notes, knobs: { escapeHTML: false } }, + ); From d8153db3bffb07ef77424200cc49578d92b9f09e Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Mon, 28 Oct 2019 16:31:25 +0200 Subject: [PATCH 02/17] Update package.json, package-lock.json, CHANGELOG.md. --- packages/components/psammead-media-player/CHANGELOG.md | 3 ++- packages/components/psammead-media-player/package-lock.json | 2 +- packages/components/psammead-media-player/package.json | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/components/psammead-media-player/CHANGELOG.md b/packages/components/psammead-media-player/CHANGELOG.md index 81b54bf185..b2e7f7167c 100644 --- a/packages/components/psammead-media-player/CHANGELOG.md +++ b/packages/components/psammead-media-player/CHANGELOG.md @@ -3,7 +3,8 @@ | Version | Description | | ------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| 2.1.0 | [PR#2424](https://github.com/bbc/psammead/pull/2424) Add srcset to placeholder image +| 2.1.1 | [PR#]() Add notes to stories | +| 2.1.0 | [PR#2424](https://github.com/bbc/psammead/pull/2424) Add srcset to placeholder image | | 2.0.2 | [PR#2486](https://github.com/bbc/psammead/pull/2486) Talos - Bump Dependencies - @bbc/psammead-image | | 2.0.1 | [PR#2317](https://github.com/bbc/psammead/pull/2317) Add `psammead-play-button` to Media Player placeholder | | 2.0.0 | [PR#2434](https://github.com/bbc/psammead/pull/2434) Add `title` attribute to AV media player | diff --git a/packages/components/psammead-media-player/package-lock.json b/packages/components/psammead-media-player/package-lock.json index 4597bbdee8..8a1923c793 100644 --- a/packages/components/psammead-media-player/package-lock.json +++ b/packages/components/psammead-media-player/package-lock.json @@ -1,6 +1,6 @@ { "name": "@bbc/psammead-media-player", - "version": "2.1.0", + "version": "2.1.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/components/psammead-media-player/package.json b/packages/components/psammead-media-player/package.json index 2ddf98cbca..c78fea9278 100644 --- a/packages/components/psammead-media-player/package.json +++ b/packages/components/psammead-media-player/package.json @@ -1,6 +1,6 @@ { "name": "@bbc/psammead-media-player", - "version": "2.1.0", + "version": "2.1.1", "description": "Provides a media player with optional placeholder", "main": "dist/index.js", "module": "esm/index.js", From 8f5ff8dfbefac7de02f06db81102e21abbbc4881 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Mon, 28 Oct 2019 19:27:28 +0200 Subject: [PATCH 03/17] Add alt, height & width as props. --- .../psammead-media-player/src/Amp/index.jsx | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/components/psammead-media-player/src/Amp/index.jsx b/packages/components/psammead-media-player/src/Amp/index.jsx index 0349a37b44..11e6454719 100644 --- a/packages/components/psammead-media-player/src/Amp/index.jsx +++ b/packages/components/psammead-media-player/src/Amp/index.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { string } from 'prop-types'; +import { string, number } from 'prop-types'; import Helmet from 'react-helmet'; import { AmpImg } from '@bbc/psammead-image'; @@ -13,7 +13,15 @@ const AmpHead = () => ( ); -const AmpMediaPlayer = ({ src, placeholderSrc, placeholderSrcset, title }) => { +const AmpMediaPlayer = ({ + src, + placeholderSrc, + placeholderSrcset, + title, + alt, + height, + width, +}) => { return ( <> @@ -30,6 +38,9 @@ const AmpMediaPlayer = ({ src, placeholderSrc, placeholderSrcset, title }) => { src={placeholderSrc} srcset={placeholderSrcset} placeholder + alt={alt} + height={height} + width={width} /> @@ -41,9 +52,15 @@ AmpMediaPlayer.propTypes = { placeholderSrc: string.isRequired, placeholderSrcset: string, title: string.isRequired, + alt: string, + height: number, + width: number, }; AmpMediaPlayer.defaultProps = { placeholderSrcset: null, + alt: null, + width: null, + height: null, }; export default AmpMediaPlayer; From 46fdcdb745f370cb9a1e966d5ba31f6add7ac7b2 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Mon, 28 Oct 2019 19:30:14 +0200 Subject: [PATCH 04/17] Make alt a prop. --- .../psammead-media-player/src/Amp/index.jsx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/components/psammead-media-player/src/Amp/index.jsx b/packages/components/psammead-media-player/src/Amp/index.jsx index 11e6454719..5543efa07c 100644 --- a/packages/components/psammead-media-player/src/Amp/index.jsx +++ b/packages/components/psammead-media-player/src/Amp/index.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { string, number } from 'prop-types'; +import { string } from 'prop-types'; import Helmet from 'react-helmet'; import { AmpImg } from '@bbc/psammead-image'; @@ -19,8 +19,6 @@ const AmpMediaPlayer = ({ placeholderSrcset, title, alt, - height, - width, }) => { return ( <> @@ -39,8 +37,6 @@ const AmpMediaPlayer = ({ srcset={placeholderSrcset} placeholder alt={alt} - height={height} - width={width} /> @@ -53,14 +49,10 @@ AmpMediaPlayer.propTypes = { placeholderSrcset: string, title: string.isRequired, alt: string, - height: number, - width: number, }; AmpMediaPlayer.defaultProps = { placeholderSrcset: null, - alt: null, - width: null, - height: null, + alt: 'Image alt', }; export default AmpMediaPlayer; From b4528b574d3874f2ce457fef83bacd8622b29047 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Mon, 28 Oct 2019 19:33:13 +0200 Subject: [PATCH 05/17] Update snapshots. --- .../src/Amp/__snapshots__/index.test.jsx.snap | 1 + .../src/__snapshots__/index.test.jsx.snap | 3 +++ 2 files changed, 4 insertions(+) diff --git a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap index 612ea1040e..0be7434934 100644 --- a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap @@ -20,6 +20,7 @@ exports[`Media Player: Amp should render an amp-iframe with an amp-img nested in title="Media player" > Date: Mon, 28 Oct 2019 19:37:56 +0200 Subject: [PATCH 06/17] Update PR number & link. --- packages/components/psammead-media-player/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/psammead-media-player/CHANGELOG.md b/packages/components/psammead-media-player/CHANGELOG.md index b2e7f7167c..16676f68c3 100644 --- a/packages/components/psammead-media-player/CHANGELOG.md +++ b/packages/components/psammead-media-player/CHANGELOG.md @@ -3,7 +3,7 @@ | Version | Description | | ------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| 2.1.1 | [PR#]() Add notes to stories | +| 2.1.1 | [PR#2516](https://github.com/bbc/psammead/pull/2516) Add notes to stories | | 2.1.0 | [PR#2424](https://github.com/bbc/psammead/pull/2424) Add srcset to placeholder image | | 2.0.2 | [PR#2486](https://github.com/bbc/psammead/pull/2486) Talos - Bump Dependencies - @bbc/psammead-image | | 2.0.1 | [PR#2317](https://github.com/bbc/psammead/pull/2317) Add `psammead-play-button` to Media Player placeholder | From 3a7308c890d367fc54d3e7fe20301f154852cd66 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Mon, 28 Oct 2019 19:46:44 +0200 Subject: [PATCH 07/17] Remove extra white spaces. --- .../psammead-media-player/CHANGELOG.md | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/components/psammead-media-player/CHANGELOG.md b/packages/components/psammead-media-player/CHANGELOG.md index 16676f68c3..1213febd18 100644 --- a/packages/components/psammead-media-player/CHANGELOG.md +++ b/packages/components/psammead-media-player/CHANGELOG.md @@ -1,25 +1,25 @@ # Psammead Media Player Changelog -| Version | Description | -| ------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| Version | Description | +| ------- |------------ | | 2.1.1 | [PR#2516](https://github.com/bbc/psammead/pull/2516) Add notes to stories | | 2.1.0 | [PR#2424](https://github.com/bbc/psammead/pull/2424) Add srcset to placeholder image | | 2.0.2 | [PR#2486](https://github.com/bbc/psammead/pull/2486) Talos - Bump Dependencies - @bbc/psammead-image | | 2.0.1 | [PR#2317](https://github.com/bbc/psammead/pull/2317) Add `psammead-play-button` to Media Player placeholder | | 2.0.0 | [PR#2434](https://github.com/bbc/psammead/pull/2434) Add `title` attribute to AV media player | -| 1.1.1-alpha.8 | [PR#2410](https://github.com/bbc/psammead/pull/2410) Update AMP story audio skin AMP placeholder image | -| 1.1.1-alpha.7 | [PR#2312](https://github.com/bbc/psammead/pull/2312) Add lang to the embed src URL in storybook examples -| 1.1.1-alpha.6 | [PR#2297](https://github.com/bbc/psammead/pull/2297) define placeholderSrc | -| 1.1.1-alpha.5 | [PR#2280](https://github.com/bbc/psammead/pull/2280) Remove `scrolling="no"` for a11y best practice | +| 1.1.1-alpha.8 | [PR#2410](https://github.com/bbc/psammead/pull/2410) Update AMP story audio skin AMP placeholder image | +| 1.1.1-alpha.7 | [PR#2312](https://github.com/bbc/psammead/pull/2312) Add lang to the embed src URL in storybook examples | +| 1.1.1-alpha.6 | [PR#2297](https://github.com/bbc/psammead/pull/2297) define placeholderSrc | +| 1.1.1-alpha.5 | [PR#2280](https://github.com/bbc/psammead/pull/2280) Remove `scrolling="no"` for a11y best practice | | 1.1.1-alpha.4 | [PR#2281](https://github.com/bbc/psammead/pull/2281) Change allowfullscreen attribute value to valid value `allowfullscreen` | -| 1.1.1-alpha.3 | [PR#2284](https://github.com/bbc/psammead/pull/2284) Add an AV audio skin story | -| 1.1.1-alpha.2 | [PR#2247](https://github.com/bbc/psammead/pull/2247) Update article embed URLs in MediaPLayer stories | -| 1.1.1-alpha.1 | [PR#2191](https://github.com/bbc/psammead/pull/2191) Talos - Bump Dependencies | -| 1.1.0-alpha.1 | [PR#2144](https://github.com/bbc/psammead/pull/2144) Add `skin` prop | -| 1.0.1-alpha.4 | [PR#1960](https://github.com/bbc/psammead/pull/1960) Change to <> | -| 1.0.1-alpha.3 | [PR#1961](https://github.com/bbc/psammead/pull/1961) Talos - Bump Dependencies | -| 1.0.1-alpha.2 | [PR#1942](https://github.com/bbc/psammead/pull/1942) Talos - Bump React to 16.9.0 | -| 1.0.1-alpha.1 | [PR#1920](https://github.com/bbc/psammead/pull/1920) Use @testing-library-react | -| 1.0.1-alpha.0 | [PR#1842](https://github.com/bbc/psammead/pull/1842) Fix fullscreen for Firefox, Internet Explorer and Safari | -| 1.0.0-alpha.0 | [PR#1831](https://github.com/bbc/psammead/pull/1831) Create `psammead-media-player` component | +| 1.1.1-alpha.3 | [PR#2284](https://github.com/bbc/psammead/pull/2284) Add an AV audio skin story | +| 1.1.1-alpha.2 | [PR#2247](https://github.com/bbc/psammead/pull/2247) Update article embed URLs in MediaPLayer stories | +| 1.1.1-alpha.1 | [PR#2191](https://github.com/bbc/psammead/pull/2191) Talos - Bump Dependencies | +| 1.1.0-alpha.1 | [PR#2144](https://github.com/bbc/psammead/pull/2144) Add `skin` prop | +| 1.0.1-alpha.4 | [PR#1960](https://github.com/bbc/psammead/pull/1960) Change to <> | +| 1.0.1-alpha.3 | [PR#1961](https://github.com/bbc/psammead/pull/1961) Talos - Bump Dependencies | +| 1.0.1-alpha.2 | [PR#1942](https://github.com/bbc/psammead/pull/1942) Talos - Bump React to 16.9.0 | +| 1.0.1-alpha.1 | [PR#1920](https://github.com/bbc/psammead/pull/1920) Use @testing-library-react | +| 1.0.1-alpha.0 | [PR#1842](https://github.com/bbc/psammead/pull/1842) Fix fullscreen for Firefox, Internet Explorer and Safari | +| 1.0.0-alpha.0 | [PR#1831](https://github.com/bbc/psammead/pull/1831) Create `psammead-media-player` component | From 7a42260207a6cf03bbeb520b8c2cd303cb31c051 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Tue, 29 Oct 2019 17:18:06 +0200 Subject: [PATCH 08/17] Add default height and width values. --- .../components/psammead-media-player/src/Amp/index.jsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/components/psammead-media-player/src/Amp/index.jsx b/packages/components/psammead-media-player/src/Amp/index.jsx index 5543efa07c..39977e55fc 100644 --- a/packages/components/psammead-media-player/src/Amp/index.jsx +++ b/packages/components/psammead-media-player/src/Amp/index.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { string } from 'prop-types'; +import { string, number } from 'prop-types'; import Helmet from 'react-helmet'; import { AmpImg } from '@bbc/psammead-image'; @@ -19,6 +19,8 @@ const AmpMediaPlayer = ({ placeholderSrcset, title, alt, + height, + width, }) => { return ( <> @@ -37,6 +39,8 @@ const AmpMediaPlayer = ({ srcset={placeholderSrcset} placeholder alt={alt} + height={height} + width={width} /> @@ -49,10 +53,14 @@ AmpMediaPlayer.propTypes = { placeholderSrcset: string, title: string.isRequired, alt: string, + height: number, + width: number, }; AmpMediaPlayer.defaultProps = { placeholderSrcset: null, alt: 'Image alt', + height: 640, + width: 450, }; export default AmpMediaPlayer; From d35b56067532a39b47d19374b9b158b4abc78d80 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Tue, 29 Oct 2019 17:23:42 +0200 Subject: [PATCH 09/17] Update snapshot. --- packages/components/psammead-media-player/CHANGELOG.md | 2 +- .../src/Amp/__snapshots__/index.test.jsx.snap | 2 ++ .../src/__snapshots__/index.test.jsx.snap | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/components/psammead-media-player/CHANGELOG.md b/packages/components/psammead-media-player/CHANGELOG.md index 1213febd18..507c87b2b3 100644 --- a/packages/components/psammead-media-player/CHANGELOG.md +++ b/packages/components/psammead-media-player/CHANGELOG.md @@ -3,7 +3,7 @@ | Version | Description | | ------- |------------ | -| 2.1.1 | [PR#2516](https://github.com/bbc/psammead/pull/2516) Add notes to stories | +| 2.1.1 | [PR#2516](https://github.com/bbc/psammead/pull/2516) Add notes to stories. Add default `alt`, `height` and `width` values | | 2.1.0 | [PR#2424](https://github.com/bbc/psammead/pull/2424) Add srcset to placeholder image | | 2.0.2 | [PR#2486](https://github.com/bbc/psammead/pull/2486) Talos - Bump Dependencies - @bbc/psammead-image | | 2.0.1 | [PR#2317](https://github.com/bbc/psammead/pull/2317) Add `psammead-play-button` to Media Player placeholder | diff --git a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap index 0be7434934..165b369d27 100644 --- a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap @@ -22,10 +22,12 @@ exports[`Media Player: Amp should render an amp-iframe with an amp-img nested in diff --git a/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap b/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap index df8a5f464b..5b72d6d05d 100644 --- a/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap @@ -31,9 +31,11 @@ exports[`Media Player: AMP Entry renders a landscape container with an amp-ifram @@ -73,9 +75,11 @@ exports[`Media Player: AMP Entry renders a portrait container with amp-iframe an @@ -115,9 +119,11 @@ exports[`Media Player: AMP Entry renders the audio skin 1`] = ` From 2a8f24be4e145b82dcf66f7b54af64c7c0b065ae Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Thu, 31 Oct 2019 18:38:28 +0200 Subject: [PATCH 10/17] Update version number. --- packages/components/psammead-media-player/package-lock.json | 2 +- packages/components/psammead-media-player/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/psammead-media-player/package-lock.json b/packages/components/psammead-media-player/package-lock.json index 8a1923c793..22ec8c4e09 100644 --- a/packages/components/psammead-media-player/package-lock.json +++ b/packages/components/psammead-media-player/package-lock.json @@ -1,6 +1,6 @@ { "name": "@bbc/psammead-media-player", - "version": "2.1.1", + "version": "2.1.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/components/psammead-media-player/package.json b/packages/components/psammead-media-player/package.json index c78fea9278..63590d712b 100644 --- a/packages/components/psammead-media-player/package.json +++ b/packages/components/psammead-media-player/package.json @@ -1,6 +1,6 @@ { "name": "@bbc/psammead-media-player", - "version": "2.1.1", + "version": "2.1.2", "description": "Provides a media player with optional placeholder", "main": "dist/index.js", "module": "esm/index.js", From 7150525387c23dd342e9127b949f70ce13a94d36 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 14:59:25 +0200 Subject: [PATCH 11/17] Make height & width required props. Remove alt. --- .../psammead-media-player/src/Amp/index.jsx | 11 +++-------- .../psammead-media-player/src/index.stories.jsx | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/components/psammead-media-player/src/Amp/index.jsx b/packages/components/psammead-media-player/src/Amp/index.jsx index 39977e55fc..3e7bb3f061 100644 --- a/packages/components/psammead-media-player/src/Amp/index.jsx +++ b/packages/components/psammead-media-player/src/Amp/index.jsx @@ -18,7 +18,6 @@ const AmpMediaPlayer = ({ placeholderSrc, placeholderSrcset, title, - alt, height, width, }) => { @@ -38,7 +37,7 @@ const AmpMediaPlayer = ({ src={placeholderSrc} srcset={placeholderSrcset} placeholder - alt={alt} + alt="" height={height} width={width} /> @@ -52,15 +51,11 @@ AmpMediaPlayer.propTypes = { placeholderSrc: string.isRequired, placeholderSrcset: string, title: string.isRequired, - alt: string, - height: number, - width: number, + height: number.isRequired, + width: number.isRequired, }; AmpMediaPlayer.defaultProps = { placeholderSrcset: null, - alt: 'Image alt', - height: 640, - width: 450, }; export default AmpMediaPlayer; diff --git a/packages/components/psammead-media-player/src/index.stories.jsx b/packages/components/psammead-media-player/src/index.stories.jsx index 12b6dd3710..213473f568 100644 --- a/packages/components/psammead-media-player/src/index.stories.jsx +++ b/packages/components/psammead-media-player/src/index.stories.jsx @@ -38,7 +38,7 @@ storiesOf('Components|Media Player', module).add( { notes, knobs: { escapeHTML: false } }, ); -storiesOf('Components|MediaPlayer', module) +storiesOf('Components|Media Player', module) .addDecorator(ampDecorator) .add( 'AMP', From 8c796db623e82a506d9a078d083cdfbda6a89963 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 15:31:20 +0200 Subject: [PATCH 12/17] Combine canoncial and amp stories. --- .../src/index.stories.jsx | 92 +++++++++---------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/packages/components/psammead-media-player/src/index.stories.jsx b/packages/components/psammead-media-player/src/index.stories.jsx index 213473f568..26278528ad 100644 --- a/packages/components/psammead-media-player/src/index.stories.jsx +++ b/packages/components/psammead-media-player/src/index.stories.jsx @@ -10,33 +10,51 @@ const withDuration = { datetime: 'PT2M30S', }; -storiesOf('Components|Media Player', module).add( - 'Default', - () => ( - - ), - { notes, knobs: { escapeHTML: false } }, -); - -storiesOf('Components|Media Player', module).add( - 'Audio', - () => ( - - ), - { notes, knobs: { escapeHTML: false } }, -); +storiesOf('Components|Media Player', module) + .add( + 'Default', + () => ( + + ), + { notes, knobs: { escapeHTML: false } }, + ) + .add( + 'Audio', + () => ( + + ), + { notes, knobs: { escapeHTML: false } }, + ) + .add( + 'Audio Skin', + () => ( + + ), + { notes, knobs: { escapeHTML: false } }, + ); storiesOf('Components|Media Player', module) .addDecorator(ampDecorator) @@ -57,25 +75,7 @@ storiesOf('Components|Media Player', module) /> ), { notes, knobs: { escapeHTML: false } }, - ); - -storiesOf('Components|Media Player', module).add( - 'Audio Skin', - () => ( - - ), - { notes, knobs: { escapeHTML: false } }, -); - -storiesOf('Components|Media Player', module) - .addDecorator(ampDecorator) + ) .add( 'Audio Skin AMP', () => ( From 62d8c6b2dc76890c2bfcb14472a4eb7da82a8004 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 16:46:49 +0200 Subject: [PATCH 13/17] Add heigth and width. --- packages/components/psammead-media-player/src/index.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/components/psammead-media-player/src/index.jsx b/packages/components/psammead-media-player/src/index.jsx index b168331d01..bf96474895 100644 --- a/packages/components/psammead-media-player/src/index.jsx +++ b/packages/components/psammead-media-player/src/index.jsx @@ -71,6 +71,8 @@ export const AmpMediaPlayer = ({ placeholderSrc={placeholderSrc} src={src} title={title} + height={portrait ? 9 : 16} + width={portrait ? 16 : 9} /> ); From f57e957b8976ef3e184bb0b722c53bf120b5e4f0 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 17:16:37 +0200 Subject: [PATCH 14/17] Update snapshots. --- .../src/Amp/__snapshots__/index.test.jsx.snap | 4 +--- .../src/__snapshots__/index.test.jsx.snap | 18 +++++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap index 165b369d27..8b2b54ea80 100644 --- a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap @@ -20,14 +20,12 @@ exports[`Media Player: Amp should render an amp-iframe with an amp-img nested in title="Media player" > diff --git a/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap b/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap index e063b83a4c..34f266f44f 100644 --- a/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap @@ -29,13 +29,13 @@ exports[`Media Player: AMP Entry renders a landscape container with an amp-ifram title="Media player" > @@ -73,13 +73,13 @@ exports[`Media Player: AMP Entry renders a portrait container with amp-iframe an title="Media player" > @@ -117,13 +117,13 @@ exports[`Media Player: AMP Entry renders the audio skin 1`] = ` title="Audio player" > From ecc3833b737cc3c49af30cce3b3ef27ee9fe718f Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 17:47:46 +0200 Subject: [PATCH 15/17] Add height and width to tests. --- .../components/psammead-media-player/src/index.test.jsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/components/psammead-media-player/src/index.test.jsx b/packages/components/psammead-media-player/src/index.test.jsx index fd018e4a92..5a472b0276 100644 --- a/packages/components/psammead-media-player/src/index.test.jsx +++ b/packages/components/psammead-media-player/src/index.test.jsx @@ -9,6 +9,8 @@ describe('Media Player: AMP Entry', () => { placeholderSrc="http://foo.bar/placeholder.png" src="http://foo.bar/iframe/amp" title="Media player" + height={16} + width={9} />, ); @@ -19,6 +21,8 @@ describe('Media Player: AMP Entry', () => { placeholderSrc="http://foo.bar/placeholder.png" src="http://foo.bar/iframe/amp" title="Media player" + height={9} + width={16} />, ); @@ -30,6 +34,8 @@ describe('Media Player: AMP Entry', () => { skin="audio" placeholderSrc="http://foo.bar/placeholder.png" title="Audio player" + height={9} + width={16} />, ); }); From 78d31d972637ffb5eeea38c2858e731a546e2dbb Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 18:05:07 +0200 Subject: [PATCH 16/17] Update snapshot. --- .../src/Amp/__snapshots__/index.test.jsx.snap | 2 ++ .../components/psammead-media-player/src/Amp/index.test.jsx | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap index 8b2b54ea80..5c0e00de37 100644 --- a/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap @@ -22,10 +22,12 @@ exports[`Media Player: Amp should render an amp-iframe with an amp-img nested in diff --git a/packages/components/psammead-media-player/src/Amp/index.test.jsx b/packages/components/psammead-media-player/src/Amp/index.test.jsx index 81d90390a7..93e59040a9 100644 --- a/packages/components/psammead-media-player/src/Amp/index.test.jsx +++ b/packages/components/psammead-media-player/src/Amp/index.test.jsx @@ -10,6 +10,8 @@ describe('Media Player: Amp', () => { placeholderSrcset="https://bbc.com/300/cat.jpg 300w, https://bbc.com/450/cat.jpg 450w, https://bbc.com/600/cat.jpg 600w" src="https://foo.bar/iframe" title="Media player" + height={9} + width={16} />, ); }); From 09b15f33c6a0e0ca98bb7a413eab550dc26bdf86 Mon Sep 17 00:00:00 2001 From: Ruth Ogendi Date: Fri, 1 Nov 2019 18:28:13 +0200 Subject: [PATCH 17/17] Update README. --- packages/components/psammead-media-player/README.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/components/psammead-media-player/README.md b/packages/components/psammead-media-player/README.md index b74932c95f..ee72e5a128 100644 --- a/packages/components/psammead-media-player/README.md +++ b/packages/components/psammead-media-player/README.md @@ -37,13 +37,17 @@ Assuming `showPlaceholder` is `true`, the `placeholderSrc` will be what image to | Argument | Type | Required | Default | Example | |-----------|---------------------|----------|---------|-----------------| | `src` | string | Yes | - | `http://foobar.com/embeddable_endpoint` | -| `placeholderSrcset` | string | No | - | ` "https://bbc.com/300/cat.jpg 300w, https://bbc.com/450/cat.jpg 450w, https://bbc.com/600/cat.jpg 600w"` | +| `placeholderSrcset` | string | No | `null` | ` "https://bbc.com/300/cat.jpg 300w, https://bbc.com/450/cat.jpg 450w, https://bbc.com/600/cat.jpg 600w"` | | `title` | string | Yes | - | `Video player` | | `portrait` | boolean | No | `false` | `true` | | `placeholderSrc` | string | Yes | - | `http://foobar.com/placeholder.png` | +| `height` | number | Yes | - | `9` | +| `width` | number | Yes | - | `16` | The `placeholderSrc` prop is required for AMP, as in order to have the component load an `amp-iframe` within 600px or 75% of the viewport from the top, we must have an `amp-img` placeholder. For more information on this, please refer to the [AMP docs for amp-iframe](https://amp.dev/documentation/components/amp-iframe/). +The `height` and `width` props are required to be provided in advance by AMP so that the aspect ratio can be known without fetching the image. For more information on this you can refer to the [AMP docs for amp-img](https://amp.dev/documentation/components/amp-img/). + ## Usage ### CanonicalMediaPlayer ```js @@ -65,13 +69,15 @@ const Container = ({ src, title, portrait, showPlaceholder, placeholderSrc }) => ```js import { AmpMediaPlayer } from '@bbc/psammead-media-player'; -const Container = ({ src, title, portrait, placeholderSrc }) => ( +const Container = ({ src, title, portrait, placeholderSrc, height, width }) => ( ) ```