Fix prop type warnings

packages/components/psammead-media-player/CHANGELOG.md

@@ -1,27 +1,28 @@
 # Psammead Media Player Changelog
 
 <!-- prettier-ignore -->
-| Version       | Description                                                                                                                  |
-| ------------- | ---------------------------------------------------------------------------------------------------------------------------- |
+| Version | Description |
+| ------- | ----------- |
+| 2.1.4 | [PR#2516](https://github.com/bbc/psammead/pull/2516) Add notes to stories. Fix prop-type warnings |
 | 2.1.3 | [PR#2542](https://github.com/bbc/psammead/pull/2542) Talos - Bump Dependencies - @bbc/psammead-play-button |
 | 2.1.2 | [PR#2467](https://github.com/bbc/psammead/pull/2476) Update image `alt` to be empty for Media Player placeholder |
 | 2.1.1 | [PR#2532](https://github.com/bbc/psammead/pull/2532) Rename "Media Player" story to "MediaPlayer"
 | 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 <React.Fragment> 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 <React.Fragment> 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 |

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 }) => (
   <AmpMediaPlayer
     src={src}
     title={title}
     portrait={portrait}
     placeholderSrc={placeholderSrc}
     placeholderSrcset={placeholderSrcset}
+    height={height}
+    width={width}
   />
 )
 ```

packages/components/psammead-media-player/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@bbc/psammead-media-player",
-  "version": "2.1.3",
+  "version": "2.1.4",
   "description": "Provides a media player with optional placeholder",
   "main": "dist/index.js",
   "module": "esm/index.js",

packages/components/psammead-media-player/src/Amp/__snapshots__/index.test.jsx.snap

@@ -20,11 +20,14 @@ exports[`Media Player: Amp should render an amp-iframe with an amp-img nested in
         title="Media player"
       >
         <amp-img
+          alt=""
           attribution=""
+          height="9"
           layout="fill"
           placeholder="true"
           src="https://foo.bar/placeholder.png"
           srcset="https://bbc.com/300/cat.jpg 300w, https://bbc.com/450/cat.jpg 450w, https://bbc.com/600/cat.jpg 600w"
+          width="16"
         />
       </amp-iframe>
     </div>

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,14 @@ const AmpHead = () => (
   </Helmet>
 );
 
-const AmpMediaPlayer = ({ src, placeholderSrc, placeholderSrcset, title }) => {
+const AmpMediaPlayer = ({
+  src,
+  placeholderSrc,
+  placeholderSrcset,
+  title,
+  height,
+  width,
+}) => {
   return (
     <>
       <AmpHead />
@@ -30,6 +37,9 @@ const AmpMediaPlayer = ({ src, placeholderSrc, placeholderSrcset, title }) => {
           src={placeholderSrc}
           srcset={placeholderSrcset}
           placeholder
+          alt=""
+          height={height}
+          width={width}
         />
       </amp-iframe>
     </>
@@ -41,6 +51,8 @@ AmpMediaPlayer.propTypes = {
   placeholderSrc: string.isRequired,
   placeholderSrcset: string,
   title: string.isRequired,
+  height: number.isRequired,
+  width: number.isRequired,
 };
 AmpMediaPlayer.defaultProps = {
   placeholderSrcset: null,

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}
     />,
   );
 });

packages/components/psammead-media-player/src/__snapshots__/index.test.jsx.snap

@@ -29,10 +29,13 @@ exports[`Media Player: AMP Entry renders a landscape container with an amp-ifram
           title="Media player"
         >
           <amp-img
+            alt=""
             attribution=""
+            height="16"
             layout="fill"
             placeholder="true"
             src="http://foo.bar/placeholder.png"
+            width="9"
           />
         </amp-iframe>
       </div>
@@ -70,10 +73,13 @@ exports[`Media Player: AMP Entry renders a portrait container with amp-iframe an
           title="Media player"
         >
           <amp-img
+            alt=""
             attribution=""
+            height="9"
             layout="fill"
             placeholder="true"
             src="http://foo.bar/placeholder.png"
+            width="16"
           />
         </amp-iframe>
       </div>
@@ -111,10 +117,13 @@ exports[`Media Player: AMP Entry renders the audio skin 1`] = `
           title="Audio player"
         >
           <amp-img
+            alt=""
             attribution=""
+            height="16"
             layout="fill"
             placeholder="true"
             src="http://foo.bar/placeholder.png"
+            width="9"
           />
         </amp-iframe>
       </div>

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}
       />
     </StyledContainer>
   );

packages/components/psammead-media-player/src/index.stories.jsx

@@ -2,61 +2,90 @@ 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',
   durationSpoken: '2 minutes 30 seconds',
   datetime: 'PT2M30S',
 };
 
-storiesOf('Components|MediaPlayer', module).add('Default', () => (
-  <CanonicalMediaPlayer
-    src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/en"
-    placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
-    service="news"
-    mediaInfo={{ title: 'Dog chases cat.', ...withDuration }}
-  />
-));
+storiesOf('Components|Media Player', module)
+  .add(
+    'Default',
+    () => (
+      <CanonicalMediaPlayer
+        src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/en"
+        placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
+        service="news"
+        mediaInfo={{ title: 'Dog chases cat.', ...withDuration }}
+        title="Default Video player"
+      />
+    ),
+    { notes, knobs: { escapeHTML: false } },
+  )
+  .add(
+    'Audio',
+    () => (
+      <CanonicalMediaPlayer
+        src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/en"
+        placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
+        service="news"
+        mediaInfo={{
+          type: 'audio',
+          title: 'Dog barks at cat.',
+          ...withDuration,
+        }}
+        title="Video player"
+      />
+    ),
+    { notes, knobs: { escapeHTML: false } },
+  )
+  .add(
+    'Audio Skin',
+    () => (
+      <CanonicalMediaPlayer
+        src="https://www.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio/ko"
+        showPlaceholder={false}
+        skin="audio"
+        service="news"
+        mediaInfo={{ type: 'audio', title: 'Live show intro.' }}
+        title="Audio player"
+      />
+    ),
+    { notes, knobs: { escapeHTML: false } },
+  );
 
-storiesOf('Components|MediaPlayer', module).add('Audio', () => (
-  <CanonicalMediaPlayer
-    src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/en"
-    placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
-    service="news"
-    mediaInfo={{ type: 'audio', title: 'Dog barks at cat.', ...withDuration }}
-    title="Video player"
-  />
-));
-
-storiesOf('Components|MediaPlayer', module)
-  .addDecorator(ampDecorator)
-  .add('AMP', () => (
-    <AmpMediaPlayer
-      isAmp
-      src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/en/amp"
-      placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
-      title="Video player"
-    />
-  ));
-
-storiesOf('Components|MediaPlayer', module).add('Audio Skin', () => (
-  <CanonicalMediaPlayer
-    src="https://www.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio/ko"
-    showPlaceholder={false}
-    skin="audio"
-    service="news"
-    title="Audio player"
-  />
-));
-
-storiesOf('Components|MediaPlayer', module)
+storiesOf('Components|Media Player', module)
   .addDecorator(ampDecorator)
-  .add('Audio Skin AMP', () => (
-    <AmpMediaPlayer
-      isAmp
-      src="https://www.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio/ko/amp"
-      placeholderSrc="https://news.files.bbci.co.uk/include/articles/public/images/amp_audio_placeholder.png"
-      skin="audio"
-      title="Audio player"
-    />
-  ));
+  .add(
+    'AMP',
+    () => (
+      <AmpMediaPlayer
+        isAmp
+        src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/en/amp"
+        placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
+        service="news"
+        mediaInfo={{
+          type: 'audio',
+          title: 'Dog barks at cat.',
+          ...withDuration,
+        }}
+        title="Video player"
+      />
+    ),
+    { notes, knobs: { escapeHTML: false } },
+  )
+  .add(
+    'Audio Skin AMP',
+    () => (
+      <AmpMediaPlayer
+        isAmp
+        src="https://www.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio/ko/amp"
+        placeholderSrc="https://news.files.bbci.co.uk/include/articles/public/images/amp_audio_placeholder.png"
+        skin="audio"
+        title="Audio player"
+      />
+    ),
+    { notes, knobs: { escapeHTML: false } },
+  );

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}
     />,
   );
 });