Remove scrolling="no" from Media Player

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

@@ -3,6 +3,7 @@
 <!-- prettier-ignore -->
 | Version | Description |
 |---------|-------------|
+| 2.0.0 | [PR#xxx](https://github.com/bbc/psammead/pull/xxx) A11y best practice changes, remove alpha and add new required prop `title` |
 | 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 |

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

@@ -1,7 +1,3 @@
-# ⛔️ This is an alpha component ⛔️
-
-This component is currently tagged as alpha and is not suitable for production use. Following the passing of an accessibility review this component will be marked as ready for production and the alpha tag removed.
-
 # psammead-media-player · [![Known Vulnerabilities](https://snyk.io/test/github/bbc/psammead/badge.svg?targetFile=packages%2Fcomponents%2Fpsammead-brand%2Fpackage.json)](https://snyk.io/test/github/bbc/psammead?targetFile=packages%2Fcomponents%2Fpsammead-brand%2Fpackage.json) [![Dependency Status](https://david-dm.org/bbc/psammead.svg?path=packages/components/psammead-media-player)](https://david-dm.org/bbc/psammead?path=packages/components/psammead-media-player) [![peerDependencies Status](https://david-dm.org/bbc/psammead/peer-status.svg?path=packages/components/psammead-media-player)](https://david-dm.org/bbc/psammead?path=packages/components/psammead-media-player&type=peer) [![Storybook](https://raw.githubusercontent.com/storybooks/brand/master/badge/badge-storybook.svg?sanitize=true)](https://bbc.github.io/psammead/?path=/story/brand--default) [![GitHub license](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](https://github.com/bbc/psammead/blob/latest/LICENSE) [![npm version](https://img.shields.io/npm/v/@bbc/psammead-media-player.svg)](https://www.npmjs.com/package/@bbc/psammead-media-player) [![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg)](https://github.com/bbc/psammead/blob/latest/CONTRIBUTING.md) 
 
 ## Description
@@ -23,11 +19,13 @@ This component to be used at any point on the page, specifically when a media pl
 | Argument  | Type                | Required | Default | Example         |
 |-----------|---------------------|----------|---------|-----------------|
 | `src` | string | Yes   | - | `http://foobar.com/embeddable_endpoint` |
+| `title` | string | Yes   | - | `Video - description of video` |
 | `showPlaceholder` | boolean | No   | `true` | `false` |
 | `placeholderSrc` | string | No   | `null` | `http://foobar.com/placeholder.png` |
 | `portrait` | boolean | No   | `false` | `true` |
 
 The `src` prop is required, as it tells the component what page it needs to embed.
+The `title` prop is required, as it is part of a11y best practices.
 The `portrait` prop is not required, and defaults to `false`. This is to support portrait video content in the future.
 The `showPlaceholder` boolean prop is also not required, and defaults to `true`.
 Assuming `showPlaceholder` is `true`, the `placeholderSrc` will be what image to display as the placeholder.
@@ -37,6 +35,7 @@ Assuming `showPlaceholder` is `true`, the `placeholderSrc` will be what image to
 | Argument  | Type                | Required | Default | Example         |
 |-----------|---------------------|----------|---------|-----------------|
 | `src` | string | Yes   | - | `http://foobar.com/embeddable_endpoint` |
+| `title` | string | Yes   | - | `Video - description of video` |
 | `portrait` | boolean | No   | `false` | `true` |
 | `placeholderSrc` | string | yes   | - | `http://foobar.com/placeholder.png` |
 
@@ -47,9 +46,10 @@ The `placeholderSrc` prop is required for AMP, as in order to have the component
 ```js
 import { CanonicalMediaPlayer } from '@bbc/psammead-media-player';
 
-const Container = ({ src, portrait, showPlaceholder, placeholderSrc }) => (
+const Container = ({ src, portrait, showPlaceholder, placeholderSrc, title }) => (
   <CanonicalMediaPlayer
     src={src}
+    title={title}
     portrait={portrait}
     placeholderSrc={placeholderSrc}
     showPlaceholder={showPlaceholder}
@@ -61,17 +61,18 @@ const Container = ({ src, portrait, showPlaceholder, placeholderSrc }) => (
 ```js
 import { AmpMediaPlayer } from '@bbc/psammead-media-player';
 
-const Container = ({ src, portrait, placeholderSrc }) => (
+const Container = ({ src, portrait, placeholderSrc, title }) => (
   <AmpMediaPlayer
     src={src}
+    title={title}
     portrait={portrait}
     placeholderSrc={placeholderSrc}
   />
 )
 ```
 
 ## Accessibility notes
-This component is still in its initial alpha stages, and requires a full and extensive accessibility review.
+This component requires a `title` value for accessibilty best practices. It also should not have `scrolling="no"` added in the future as this is against a11y best practices.
 
 ## Contributing
 

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

@@ -1,9 +1,6 @@
 {
   "name": "@bbc/psammead-media-player",
-  "version": "1.1.1-alpha.2",
-  "publishConfig": {
-    "tag": "alpha"
-  },
+  "version": "2.0.0",
   "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

@@ -17,6 +17,7 @@ exports[`Media Player: Amp should render an amp-iframe with an amp-img nested in
         layout="fill"
         sandbox="allow-scripts allow-same-origin"
         src="https://foo.bar/iframe"
+        title="An iframe"
       >
         <amp-img
           layout="fill"

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

@@ -12,12 +12,13 @@ const AmpHead = () => (
   </Helmet>
 );
 
-const AmpMediaPlayer = ({ src, placeholderSrc }) => {
+const AmpMediaPlayer = ({ src, placeholderSrc, title }) => {
   return (
     <>
       <AmpHead />
       <amp-iframe
         sandbox="allow-scripts allow-same-origin"
+        title={title}
         layout="fill"
         frameborder="0"
         src={src}
@@ -32,6 +33,7 @@ const AmpMediaPlayer = ({ src, placeholderSrc }) => {
 AmpMediaPlayer.propTypes = {
   src: string.isRequired,
   placeholderSrc: string.isRequired,
+  title: string.isRequired,
 };
 
 export default AmpMediaPlayer;

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

@@ -8,6 +8,7 @@ describe('Media Player: Amp', () => {
     <Amp
       placeholderSrc="https://foo.bar/placeholder.png"
       src="https://foo.bar/iframe"
+      title="An iframe"
     />,
   );
 });

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

@@ -15,7 +15,7 @@ exports[`Media Player: Canonical should render an iframe 1`] = `
   allow="autoplay; fullscreen"
   allowfullscreen=""
   class="c0"
-  scrolling="no"
   src="https://foo.bar/iframe"
+  title="An iframe"
 />
 `;

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

@@ -12,10 +12,10 @@ const StyledIframe = styled.iframe`
   overflow: hidden;
 `;
 
-const Canonical = ({ src }) => (
+const Canonical = ({ src, title }) => (
   <StyledIframe
     src={src}
-    scrolling="no"
+    title={title}
     allow="autoplay; fullscreen"
     gesture="media"
     allowFullScreen
@@ -24,6 +24,7 @@ const Canonical = ({ src }) => (
 
 Canonical.propTypes = {
   src: string.isRequired,
+  title: string.isRequired,
 };
 
 export default Canonical;

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

@@ -5,6 +5,6 @@ import Canonical from '.';
 describe('Media Player: Canonical', () => {
   shouldMatchSnapshot(
     'should render an iframe',
-    <Canonical src="https://foo.bar/iframe" />,
+    <Canonical src="https://foo.bar/iframe" title="An iframe" />,
   );
 });

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

@@ -26,6 +26,7 @@ exports[`Media Player: AMP Entry renders a landscape container with an amp-ifram
           layout="fill"
           sandbox="allow-scripts allow-same-origin"
           src="http://foo.bar/iframe/amp"
+          title="An iframe"
         >
           <amp-img
             layout="fill"
@@ -65,6 +66,7 @@ exports[`Media Player: AMP Entry renders a portrait container with amp-iframe an
           layout="fill"
           sandbox="allow-scripts allow-same-origin"
           src="http://foo.bar/iframe/amp"
+          title="An iframe"
         >
           <amp-img
             layout="fill"
@@ -104,6 +106,7 @@ exports[`Media Player: AMP Entry renders the audio skin 1`] = `
           layout="fill"
           sandbox="allow-scripts allow-same-origin"
           src="https://www.test.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio"
+          title="An iframe"
         >
           <amp-img
             layout="fill"
@@ -214,8 +217,8 @@ exports[`Media Player: Canonical Entry renders an iframe when showPlaceholder is
     allow="autoplay; fullscreen"
     allowfullscreen=""
     class="c1"
-    scrolling="no"
     src="http://foo.bar/iframe"
+    title="An iframe"
   />
 </div>
 `;
@@ -244,8 +247,8 @@ exports[`Media Player: Canonical Entry renders the audio skin 1`] = `
     allow="autoplay; fullscreen"
     allowfullscreen=""
     class="c1"
-    scrolling="no"
     src="https://www.test.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio"
+    title="An iframe"
   />
 </div>
 `;

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

@@ -25,6 +25,7 @@ export const CanonicalMediaPlayer = ({
   portrait,
   src,
   skin,
+  title,
 }) => {
   const [placeholderActive, setPlaceholderActive] = useState(showPlaceholder);
   const handlePlaceholderClick = () => setPlaceholderActive(false);
@@ -37,19 +38,25 @@ export const CanonicalMediaPlayer = ({
       {placeholderActive ? (
         <Placeholder onClick={handlePlaceholderClick} src={placeholderSrc} />
       ) : (
-        <Canonical src={src} />
+        <Canonical src={src} title={title} />
       )}
     </StyledContainer>
   );
 };
 
-export const AmpMediaPlayer = ({ placeholderSrc, portrait, src, skin }) => {
+export const AmpMediaPlayer = ({
+  placeholderSrc,
+  portrait,
+  src,
+  skin,
+  title,
+}) => {
   const StyledContainer =
     skin === 'audio' ? StyledAudioContainer : StyledVideoContainer;
 
   return (
     <StyledContainer portrait={portrait}>
-      <Amp placeholderSrc={placeholderSrc} src={src} />
+      <Amp placeholderSrc={placeholderSrc} src={src} title={title} />
     </StyledContainer>
   );
 };
@@ -59,6 +66,7 @@ CanonicalMediaPlayer.propTypes = {
   portrait: bool,
   showPlaceholder: bool,
   src: string.isRequired,
+  title: string.isRequired,
   skin: oneOf(['classic', 'audio']),
 };
 
@@ -73,6 +81,7 @@ AmpMediaPlayer.propTypes = {
   placeholderSrc: string.isRequired,
   portrait: bool,
   src: string.isRequired,
+  title: string.isRequired,
   skin: oneOf(['classic', 'audio']),
 };
 

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

@@ -6,6 +6,7 @@ import { ampDecorator } from '../../../../.storybook/config';
 storiesOf('Components|Media Player', module).add('default', () => (
   <CanonicalMediaPlayer
     src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp"
+    title="Video - description of video"
     placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
   />
 ));
@@ -15,15 +16,28 @@ storiesOf('Components|Media Player', module)
   .add('AMP', () => (
     <AmpMediaPlayer
       isAmp
-      src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp"
+      src="https://www.test.bbc.co.uk/ws/av-embeds/articles/c3wmq4d1y3wo/p01k6msp/amp"
+      title="Video - description of video"
       placeholderSrc="https://ichef.bbci.co.uk/news/640/cpsdevpb/4eb7/test/ba7482d0-cca8-11e8-b0bf-f33155223fc4.jpg"
     />
   ));
 
 storiesOf('Components|Media Player', module).add('Audio Skin', () => (
   <CanonicalMediaPlayer
     src="https://www.test.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio"
+    title="Audio - Live Korean Radio"
     showPlaceholder={false}
     skin="audio"
   />
 ));
+
+storiesOf('Components|Media Player', module)
+  .addDecorator(ampDecorator)
+  .add('AMP Audio skin', () => (
+    <AmpMediaPlayer
+      isAmp
+      src="https://www.test.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio/amp"
+      title="Audio - Live Korean Radio"
+      placeholderSrc="https://news.files.bbci.co.uk/include/articles/public/images/audio-player-placeholder.png"
+    />
+  ));

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

@@ -8,6 +8,7 @@ describe('Media Player: AMP Entry', () => {
     <AmpMediaPlayer
       placeholderSrc="http://foo.bar/placeholder.png"
       src="http://foo.bar/iframe/amp"
+      title="An iframe"
     />,
   );
 
@@ -17,6 +18,7 @@ describe('Media Player: AMP Entry', () => {
       portrait
       placeholderSrc="http://foo.bar/placeholder.png"
       src="http://foo.bar/iframe/amp"
+      title="An iframe"
     />,
   );
 
@@ -25,6 +27,7 @@ describe('Media Player: AMP Entry', () => {
     <AmpMediaPlayer
       showPlaceholder={false}
       src="https://www.test.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio"
+      title="An iframe"
       skin="audio"
     />,
   );
@@ -36,6 +39,7 @@ describe('Media Player: Canonical Entry', () => {
     <CanonicalMediaPlayer
       placeholderSrc="http://foo.bar/placeholder.png"
       src="http://foo.bar/iframe"
+      title="An iframe"
     />,
   );
 
@@ -44,6 +48,7 @@ describe('Media Player: Canonical Entry', () => {
     <CanonicalMediaPlayer
       placeholderSrc="http://foo.bar/placeholder.png"
       src="http://foo.bar/iframe"
+      title="An iframe"
       portrait
     />,
   );
@@ -53,6 +58,7 @@ describe('Media Player: Canonical Entry', () => {
     <CanonicalMediaPlayer
       showPlaceholder={false}
       src="http://foo.bar/iframe"
+      title="An iframe"
     />,
   );
 
@@ -61,6 +67,7 @@ describe('Media Player: Canonical Entry', () => {
     <CanonicalMediaPlayer
       showPlaceholder={false}
       src="https://www.test.bbc.com/ws/av-embeds/media/bbc_korean_radio/liveradio"
+      title="An iframe"
       skin="audio"
     />,
   );