Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Commit

Permalink
Merge pull request #2991 from bbc/revert-2836
Browse files Browse the repository at this point in the history
Revert 2836 - Add AV embed timeout handling to MP
  • Loading branch information
amywalkerdev authored Jan 23, 2020
2 parents 88d3e25 + d860417 commit 0054bcc
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 231 deletions.
1 change: 1 addition & 0 deletions packages/components/psammead-media-player/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<!-- prettier-ignore -->
| Version | Description |
| ------------- | ---------------------------------------------------------------------------------------------------------------------------- |
| 2.6.0 | [PR#2982](https://github.com/bbc/psammead/pull/2991) Revert [#2836](https://github.com/bbc/psammead/pull/2836) - Add AV embed timeout handling. |
| 2.5.4 | [PR#2992](https://github.com/bbc/psammead/pull/2992) Talos - Bump Dependencies - @bbc/psammead-play-button |
| 2.5.3 | [PR#2989](https://github.com/bbc/psammead/pull/2989) Talos - Bump Dependencies - @bbc/psammead-assets |
| 2.5.2 | [PR#2982](https://github.com/bbc/psammead/pull/2982) Talos - Bump Dependencies - @bbc/psammead-play-button |
Expand Down
4 changes: 0 additions & 4 deletions packages/components/psammead-media-player/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ This component to be used at any point on the page, specifically when a media pl
| [`mediaInfo`](#mediaInfo) | object | Yes | - | `{ title: 'A vertical video pretending to be a cat title', duration: '2:30', durationSpoken: '2 minutes 11 seconds', datetime: 'PT2M30S' }`|
| `noJsClassName` | string | No | `null` | `'no-js'` |
| `noJsMessage` | string | Yes | - | `'This media cannot play in your browser. Please enable Javascript or use a different browser.'` |
| `timeoutMs` | number | No | `5000` | `9001` |

The `src` prop is required, as it tells the component what page it needs to embed.
The `placeholderSrcset` prop is not required, as it allows image responsiveness and optimization depending on the size of the screen.
Expand All @@ -40,7 +39,6 @@ Assuming `showPlaceholder` is `true`, the `placeholderSrc` will be what image to
The `noJsClassName` is an optional prop that defaults to `null` and is used to add styling support to certain elements when javascript is disabled in the browser.
The `noJsMessage` is a required prop used display a fallback text when javascript is disabled in the browser.
The `mediaInfo` prop is required, and has the following properties.
The `timeoutMs` prop allows you to override the default timeout value. If media fails to load in this time, users are shown a timeout message.

#### mediaInfo

Expand Down Expand Up @@ -82,7 +80,6 @@ const Container = ({
mediaInfo,
noJsClassName,
noJsMessage,
timeoutMs,
}) => (
<CanonicalMediaPlayer
src={src}
Expand All @@ -96,7 +93,6 @@ const Container = ({
mediaInfo={mediaInfo}
noJsClassName={noJsClassName}
noJsMessage={noJsMessage}
timeoutMs={timeoutMs}
/>
)
```
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/components/psammead-media-player/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@bbc/psammead-media-player",
"version": "2.5.4",
"version": "2.6.0",
"description": "Provides a media player with optional placeholder",
"main": "dist/index.js",
"module": "esm/index.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

exports[`Media Player: Canonical should render an iframe 1`] = `
.c0 {
border: 0;
left: 0;
overflow: hidden;
position: absolute;
Expand All @@ -10,22 +11,12 @@ exports[`Media Player: Canonical should render an iframe 1`] = `
height: 100%;
}
.c1 {
border: 0;
height: 100%;
width: 100%;
}
<div
<iframe
allow="autoplay; fullscreen"
allowfullscreen=""
class="c0"
>
<iframe
allow="autoplay; fullscreen"
allowfullscreen=""
class="c1"
scrolling="no"
src="https://foo.bar/iframe"
title="Media player"
/>
</div>
scrolling="no"
src="https://foo.bar/iframe"
title="Media player"
/>
`;
67 changes: 21 additions & 46 deletions packages/components/psammead-media-player/src/Canonical/index.jsx
Original file line number Diff line number Diff line change
@@ -1,70 +1,45 @@
import React, { useState, useRef } from 'react';
import { string, number } from 'prop-types';
import styled, { css } from 'styled-components';
import Guidance from '../Guidance';
import useTimeout from './useTimeout';

const Canonical = ({ service, src, title, placeholderSrc, timeoutMs }) => {
const [hasTimedOut, setHasTimedOut] = useState(null);
const iframeRef = useRef(null);

useTimeout(setHasTimedOut, iframeRef, timeoutMs);
import React from 'react';
import { string } from 'prop-types';
import styled from 'styled-components';

const Canonical = ({ src, title, placeholderSrc }) => {
const backgroundStyle = `
background-image: url(${placeholderSrc});
background-repeat: no-repeat;
background-size: contain;
`;

const CanonicalWrapper = styled.div`
const StyledIframe = styled.iframe`
border: 0;
left: 0;
overflow: hidden;
position: absolute;
top: 0;
width: 100%;
height: 100%;
${placeholderSrc &&
css`
background-image: url(${placeholderSrc});
background-repeat: no-repeat;
background-size: contain;
`}
`;

const StyledIframe = styled.iframe`
border: 0;
height: 100%;
width: 100%;
${placeholderSrc ? backgroundStyle : null}
`;

return (
<CanonicalWrapper>
{hasTimedOut ? (
<Guidance
service={service}
guidanceMessage="There was a problem loading this content. Please check your internet connection and try again."
noJsMessage=""
/>
) : (
<StyledIframe
ref={iframeRef}
src={src}
title={title}
allow="autoplay; fullscreen"
scrolling="no"
gesture="media"
allowFullScreen
/>
)}
</CanonicalWrapper>
<StyledIframe
src={src}
title={title}
allow="autoplay; fullscreen"
scrolling="no"
gesture="media"
allowFullScreen
/>
);
};

Canonical.propTypes = {
service: string.isRequired,
src: string.isRequired,
title: string.isRequired,
placeholderSrc: string,
timeoutMs: number,
};

Canonical.defaultProps = {
placeholderSrc: null,
timeoutMs: 5000,
};

export default Canonical;
Original file line number Diff line number Diff line change
@@ -1,55 +1,10 @@
import React from 'react';
import { render, act, cleanup, fireEvent } from '@testing-library/react';
import { shouldMatchSnapshot } from '@bbc/psammead-test-helpers';
import Canonical from '.';

afterEach(cleanup);

describe('Media Player: Canonical', () => {
shouldMatchSnapshot(
'should render an iframe',
<Canonical
src="https://foo.bar/iframe"
title="Media player"
service="news"
/>,
<Canonical src="https://foo.bar/iframe" title="Media player" />,
);

it('should render a timeout message if the timeout expires', () => {
jest.useFakeTimers();
const { getByText } = render(
<Canonical
src="https://foo.bar/iframe"
title="Media player"
service="news"
timeoutMs={2000}
/>,
);

act(() => {
jest.runTimersToTime(3000);
});

expect(
getByText(
'There was a problem loading this content. Please check your internet connection and try again.',
),
);
});

it('should not render a timeout message if it loads successfully', () => {
const { getByTitle, getByText } = render(
<Canonical
src="https://foo.bar/iframe"
title="Media player"
service="news"
/>,
);
fireEvent.load(getByTitle('Media player'));
expect(() =>
getByText(
'There was a problem loading this content. Please check your internet connection and try again.',
),
).toThrow();
});
});

This file was deleted.

This file was deleted.

Loading

0 comments on commit 0054bcc

Please sign in to comment.