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

Revert 2836 - Add AV embed timeout handling to MP #2991

Merged
merged 9 commits into from
Jan 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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