Skip to content

Commit

Permalink
fix(project): player fixes and UI improvements
Browse files Browse the repository at this point in the history
* fix(home): click event not working properly on iOS Safari

For some reason, when including a react-markdown component on the page, all click events don’t work properly anymore. We needed to click two times in order to navigate. Replacing react-markdown with marked fixes this issue as I couldn’t find the real culprit in react-markdown.

* fix(player): player not respecting updated startTime

* fix(player): player not on top when fading in

* fix(player): store watch progress when leaving page

* fix(player): update autostart option in player

* fix: multiple ui improvements

* chore: update snapshots
  • Loading branch information
ChristiaanScheermeijer authored Oct 5, 2022
1 parent d42f9fc commit 5670fd8
Show file tree
Hide file tree
Showing 24 changed files with 139 additions and 571 deletions.
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"i18next-browser-languagedetector": "^6.1.1",
"jwt-decode": "^3.1.2",
"lodash.merge": "^4.6.2",
"marked": "^4.1.1",
"memoize-one": "^5.2.1",
"npm-run-all": "^4.1.5",
"planby": "^0.3.0",
Expand All @@ -55,7 +56,6 @@
"react-helmet": "^6.1.0",
"react-i18next": "^11.10.0",
"react-infinite-scroller": "^1.2.6",
"react-markdown": "^8.0.3",
"react-query": "^3.13.10",
"react-router-dom": "^6.4.0",
"react-virtualized": "^9.22.3",
Expand All @@ -70,15 +70,16 @@
"@testing-library/jest-dom": "^5.16.4",
"@testing-library/react": "^11.2.6",
"@testing-library/react-hooks": "^8.0.1",
"@types/dompurify": "^2.3.3",
"@types/dompurify": "^2.3.4",
"@types/jwplayer": "^8.2.7",
"@types/lodash.merge": "^4.6.6",
"@types/marked": "^4.0.7",
"@types/node": "^17.0.23",
"@types/react": "^17.0.14",
"@types/react-dom": "^17.0.9",
"@types/react-helmet": "^6.1.2",
"@types/react-router-dom": "^5.3.3",
"@types/react-infinite-scroller": "^1.2.3",
"@types/react-router-dom": "^5.3.3",
"@typescript-eslint/eslint-plugin": "^5.17.0",
"@typescript-eslint/parser": "^5.17.0",
"@vitejs/plugin-react": "^1.0.7",
Expand Down
8 changes: 7 additions & 1 deletion src/components/Animation/Animation.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { CSSProperties, useEffect, useRef, useState } from 'react';

type Props = {
className?: string;
createStyle: (status: Status) => CSSProperties;
open?: boolean;
duration?: number;
Expand All @@ -13,6 +14,7 @@ type Props = {
export type Status = 'opening' | 'open' | 'closing' | 'closed';

const Animation: React.FC<Props> = ({
className,
createStyle,
open = true,
duration = 250,
Expand Down Expand Up @@ -58,7 +60,11 @@ const Animation: React.FC<Props> = ({
return null;
}

return <div style={createStyle(status)}>{children}</div>;
return (
<div style={createStyle(status)} className={className}>
{children}
</div>
);
};

export default Animation;
4 changes: 3 additions & 1 deletion src/components/Animation/Fade/Fade.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { CSSProperties } from 'react';
import Animation, { Status } from '../Animation';

type Props = {
className?: string;
open?: boolean;
duration?: number;
delay?: number;
Expand All @@ -11,7 +12,7 @@ type Props = {
onCloseAnimationEnd?: () => void;
};

const Fade: React.FC<Props> = ({ open = true, duration = 250, delay = 0, onOpenAnimationEnd, onCloseAnimationEnd, keepMounted, children }) => {
const Fade: React.FC<Props> = ({ className, open = true, duration = 250, delay = 0, onOpenAnimationEnd, onCloseAnimationEnd, keepMounted, children }) => {
const seconds = duration / 1000;
const transition = `opacity ${seconds}s ease-in-out`;

Expand All @@ -22,6 +23,7 @@ const Fade: React.FC<Props> = ({ open = true, duration = 250, delay = 0, onOpenA

return (
<Animation
className={className}
createStyle={(status: Status) => createStyle(status)}
open={open}
duration={duration}
Expand Down
52 changes: 27 additions & 25 deletions src/components/MarkdownComponent/MarkdownComponent.tsx
Original file line number Diff line number Diff line change
@@ -1,37 +1,39 @@
import React, { useMemo } from 'react';
import DOMPurify from 'dompurify';
import { marked } from 'marked';
import classNames from 'classnames';
import React from 'react';
import ReactMarkdown from 'react-markdown';

import styles from './MarkdownComponent.module.scss';

const renderer = {
link(href: string, title: string, text: string) {
const externalLink = /^(https?|www\.|\/\/)/.test(href || '');
const targetAttr = externalLink ? 'target="_blank"' : undefined;
const relAttr = externalLink ? 'rel="noopener"' : undefined;
const titleAttr = title ? `title="${title}"` : undefined;
const attributes = [targetAttr, relAttr, titleAttr].filter(Boolean);

return `<a href="${href}" ${attributes.join(' ')}>${text}</a>`;
},
};

marked.use({ renderer });

type Props = {
markdownString: string;
className?: string;
disallowedElements?: string[];
inline?: boolean;
};

const MarkdownComponent: React.FC<Props> = ({ markdownString, className, disallowedElements }: Props) => {
return (
<ReactMarkdown
className={classNames(styles.markdown, className)}
components={{
a: ({ node, href, children, ...props }) => {
const externalLink = /^(https?|www\.|\/\/)/.test(href || '');
const target = externalLink ? '_blank' : undefined;
const rel = externalLink ? 'noopener' : undefined;
return (
<a {...props} href={href} target={target} rel={rel}>
{children}
</a>
);
},
}}
disallowedElements={disallowedElements}
unwrapDisallowed
>
{markdownString}
</ReactMarkdown>
);
const MarkdownComponent: React.FC<Props> = ({ markdownString, className, inline = false }) => {
const sanitizedHTMLString = useMemo(() => {
const parseDelegate = inline ? marked.parseInline : marked.parse;
const dirtyHTMLString = parseDelegate(markdownString);

return DOMPurify.sanitize(dirtyHTMLString, { ADD_ATTR: ['target'] });
}, [inline, markdownString]);

return <div className={classNames(styles.markdown, className)} dangerouslySetInnerHTML={{ __html: sanitizedHTMLString }} />;
};

export default MarkdownComponent;
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ exports[`<MarkdownComponent> > renders and matches snapshot 1`] = `
testlink
</a>
</p>
</div>
</div>
`;
21 changes: 21 additions & 0 deletions src/components/Player/Player.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ type Props = {
onUserInActive?: () => void;
onBeforePlay?: () => void;
onFirstFrame?: () => void;
onRemove?: () => void;
onPlaylistItem?: () => void;
onPlaylistItemCallback?: (item: PlaylistItem) => Promise<undefined | PlaylistItem>;
startTime?: number;
autostart?: boolean;
Expand All @@ -38,6 +40,8 @@ const Player: React.FC<Props> = ({
onUserInActive,
onBeforePlay,
onFirstFrame,
onRemove,
onPlaylistItem,
onPlaylistItemCallback,
feedId,
startTime = 0,
Expand All @@ -58,6 +62,8 @@ const Player: React.FC<Props> = ({
const handleUserActive = useEventCallback(onUserActive);
const handleUserInactive = useEventCallback(onUserInActive);
const handleFirstFrame = useEventCallback(onFirstFrame);
const handleRemove = useEventCallback(onRemove);
const handlePlaylistItem = useEventCallback(onPlaylistItem);
const handlePlaylistItemCallback = useEventCallback(onPlaylistItemCallback);
const handleReady = useEventCallback(() => onReady && onReady(playerRef.current));

Expand All @@ -70,6 +76,8 @@ const Player: React.FC<Props> = ({
playerRef.current?.on('userActive', handleUserActive);
playerRef.current?.on('userInactive', handleUserInactive);
playerRef.current?.on('firstFrame', handleFirstFrame);
playerRef.current?.on('remove', handleRemove);
playerRef.current?.on('playlistItem', handlePlaylistItem);
playerRef.current?.setPlaylistItemCallback(handlePlaylistItemCallback);
}, [
handleReady,
Expand All @@ -80,6 +88,8 @@ const Player: React.FC<Props> = ({
handleUserActive,
handleUserInactive,
handleFirstFrame,
handleRemove,
handlePlaylistItem,
handlePlaylistItemCallback,
]);

Expand All @@ -105,6 +115,11 @@ const Player: React.FC<Props> = ({
}
}, [scriptUrl]);

useEffect(() => {
// update the startTimeRef each time the startTime changes
startTimeRef.current = startTime;
}, [startTime]);

useEffect(() => {
if (!playerId) {
return;
Expand All @@ -122,6 +137,12 @@ const Player: React.FC<Props> = ({
logDev('Calling loadPlaylist with the same item, check the dependencies');
return;
}

// update autostart parameter
if (typeof autostart !== 'undefined') {
playerRef.current?.setConfig({ autostart });
}

// load new item
playerRef.current.load([deepCopy({ ...item, starttime: startTimeRef.current })]);
};
Expand Down
2 changes: 1 addition & 1 deletion src/components/VideoDetails/VideoDetails.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
}

.info {
position: relative;
width: 70%;
max-width: 650px;
min-height: 300px;
Expand Down Expand Up @@ -139,7 +140,6 @@
position: absolute;
top: 0;
right: 0;
z-index: -1;
background-repeat: no-repeat;
background-position: center center;
background-size: cover;
Expand Down
2 changes: 1 addition & 1 deletion src/components/VideoDetails/VideoDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const VideoDetails: React.VFC<Props> = ({
[styles.posterNormal]: posterMode === 'normal',
})}
>
<Image className={classNames(styles.poster, styles[posterMode])} image={image} alt={title} width={1280} />
<div className={styles.info}>
<h2 className={styles.title}>{title}</h2>
<div className={styles.metaContainer}>
Expand All @@ -58,7 +59,6 @@ const VideoDetails: React.VFC<Props> = ({
{shareButton}
</div>
</div>
<Image className={classNames(styles.poster, styles[posterMode])} image={image} alt={title} width={1280} />
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ exports[`<VideoDetails> > renders and matches snapshot 1`] = `
<div
class="_main_d0c133 _mainPadding_d0c133"
>
<img
alt="Test video"
class="_poster_d0c133 _fading_d0c133 _image_4c41c3"
src="http://image.jpg?width=1280"
/>
<div
class="_info_d0c133"
>
Expand Down Expand Up @@ -60,11 +65,6 @@ exports[`<VideoDetails> > renders and matches snapshot 1`] = `
</button>
</div>
</div>
<img
alt="Test video"
class="_poster_d0c133 _fading_d0c133 _image_4c41c3"
src="http://image.jpg?width=1280"
/>
</div>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/components/VideoLayout/VideoLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const VideoLayout: React.FC<Props> = ({
return grid ? (
<>
<div className={classNames(styles.relatedVideosGrid, { [styles.inlineLayout]: inlineLayout })}>
{relatedTitle && <h3 className={styles.relatedVideosGridTitle}>{relatedTitle}</h3>}
<h3 className={styles.relatedVideosGridTitle}>{relatedTitle || '\u00A0'}</h3>
{hasFilters && renderFilters(inlineLayout)}
</div>
<CardGrid
Expand Down Expand Up @@ -186,7 +186,6 @@ const VideoLayout: React.FC<Props> = ({

return (
<div className={styles.videoCinemaLayout} data-testid="cinema-layout">
{player}
<VideoDetails
title={title}
description={description}
Expand All @@ -201,6 +200,7 @@ const VideoLayout: React.FC<Props> = ({
/>
{playlist && onItemClick && <div className={styles.relatedVideos}>{renderRelatedVideos(true)}</div>}
{children}
{player}
</div>
);
};
Expand Down
5 changes: 5 additions & 0 deletions src/containers/Cinema/Cinema.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
@use '../../styles/mixins/responsive';
@use '../../styles/mixins/typography';

.fade {
position: relative;
z-index: 1;
}

.cinema {
position: fixed;
top: 0;
Expand Down
5 changes: 1 addition & 4 deletions src/containers/Cinema/Cinema.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,12 @@ const Cinema: React.FC<Props> = ({
return () => {
document.body.style.overflowY = '';
};
// This is needed since we only want this effect to run when the `open` property updates
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [open]);

return (
<Fade open={open}>
<Fade open={open} className={styles.fade}>
<div className={styles.cinema}>
<PlayerContainer
visible={open}
item={item}
feedId={feedId}
onPlay={handlePlay}
Expand Down
1 change: 1 addition & 0 deletions src/containers/Cinema/__snapshots__/Cinema.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`<Cinema> > renders and matches snapshot 1`] = `
<div>
<div
class="_fade_555f3c"
style="transition: opacity 0.25s ease-in-out; opacity: 0;"
>
<div
Expand Down
1 change: 0 additions & 1 deletion src/containers/InlinePlayer/InlinePlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ const InlinePlayer: React.FC<Props> = ({
</Fade>
{!paywall && (
<PlayerContainer
visible={true}
item={item}
feedId={feedId}
onPlay={onPlay}
Expand Down
2 changes: 1 addition & 1 deletion src/containers/Layout/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ const Layout = () => {
</Sidebar>
<Outlet />
</div>
{!!footerText && <MarkdownComponent className={styles.footer} markdownString={footerText} disallowedElements={['p']} />}
{!!footerText && <MarkdownComponent className={styles.footer} markdownString={footerText} inline />}

{/* Config select control to improve testing experience */}
{import.meta.env.APP_INCLUDE_TEST_CONFIGS && <ConfigSelect />}
Expand Down
Loading

0 comments on commit 5670fd8

Please sign in to comment.