Skip to content

Commit

Permalink
Fix VideoPlayer accessibility issues (#703)
Browse files Browse the repository at this point in the history
* improve contrast of VideoPlayer focus styles

* add changeset

* remove overlay from playing video

* improve contrast of VideoPlayer

* fix issue of range tooltip overflowing container

* fix color of custom play icon

* update changeset

* address pr feedback

* fix issue where time tooltip can go out of range

* fix colors after rebase

* hide ControlsBar when video doesn't have focus or mouse

* update snapshots

* hide controls on inactivity
  • Loading branch information
joshfarrant authored Dec 19, 2024
1 parent b18d390 commit 621d8ee
Show file tree
Hide file tree
Showing 20 changed files with 148 additions and 52 deletions.
10 changes: 10 additions & 0 deletions .changeset/hot-laws-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@primer/react-brand': patch
---

`VideoPlayer` accessibility improvements

- Improved contrast of play overlay focus styles.
- Improved contrast of controls and title.
- The title bar now hides while the video is playing.
- The controls bar now hides when the cursor or keyboard focus leaves the video player, or after a few seconds of inactivity, and reappears when the cursor or keyboard focus returns.
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ module.exports = {
},
title: {
bgColor: {
value: 'linear-gradient(#01040966, var(--base-color-scale-transparent))',
dark: 'linear-gradient(#01040966, var(--base-color-scale-transparent))',
value: 'linear-gradient(180deg, #000000e6, #00000073 66%, transparent)',
dark: 'linear-gradient(180deg, #000000e6, #00000073 66%, transparent)',
},
fgColor: {
value: 'var(--base-color-scale-gray-0)',
Expand All @@ -26,8 +26,8 @@ module.exports = {
},
controls: {
bgColor: {
value: 'linear-gradient(var(--base-color-scale-transparent), #01040966)',
dark: 'linear-gradient(var(--base-color-scale-transparent), #01040966)',
value: '#000000bf',
dark: '#000000bf',
},
fgColor: {
value: 'var(--base-color-scale-gray-0)',
Expand Down Expand Up @@ -65,8 +65,8 @@ module.exports = {
dark: 'var(--base-color-scale-gray-0)',
},
progress: {
value: 'var(--base-color-scale-blue-5)',
dark: 'var(--base-color-scale-blue-5)',
value: 'var(--base-color-scale-blue-4)',
dark: 'var(--base-color-scale-blue-4)',
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {VideoPlayer} from '.'
import {Stack} from '../Stack'
import {Button} from '../Button'
import {useVideo} from './hooks'
import styles from './VideoPlayer.stories.module.css'

export default {
title: 'Components/VideoPlayer/Features',
Expand Down Expand Up @@ -98,7 +99,7 @@ export const ControlledProgrammatically = () => {
}

export const CustomPlayIcon = () => (
<VideoPlayer title="GitHub media player" playIcon={() => <PlayIcon size={96} />}>
<VideoPlayer title="GitHub media player" playIcon={() => <PlayIcon size={96} className={styles.customPlayIcon} />}>
<VideoPlayer.Source src="./example.mp4" type="video/mp4" />
<VideoPlayer.Track src="./example.vtt" default />
</VideoPlayer>
Expand Down
42 changes: 26 additions & 16 deletions packages/react/src/VideoPlayer/VideoPlayer.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
.VideoPlayer__container {
width: 100%;
position: relative;
overflow: hidden;
border-radius: var(--brand-borderRadius-medium);
}

Expand All @@ -37,7 +36,7 @@
/* 3. Center Play Button */
/* ---------------------------------------------------------- */

.VideoPlayer__playButton {
.VideoPlayer__playButtonOverlay {
position: absolute;
top: 0;
left: 0;
Expand All @@ -48,20 +47,21 @@
z-index: 1;
}

.VideoPlayer__playButtonOverlay {
.VideoPlayer__playButtonOverlay.VideoPlayer__playButtonOverlay--transparent {
background: transparent;
}

.VideoPlayer__playButtonOverlay:focus-visible {
outline: var(--brand-borderWidth-thicker) solid var(--brand-color-focus);
outline-offset: var(--base-size-2);
}

.VideoPlayer__playButton {
width: 25%;
height: 25%;
max-width: var(--brand-VideoPlayer-playButton-width);
max-height: var(--brand-VideoPlayer-playButton-height);
opacity: 0.8;
}

.VideoPlayer__playButton:focus {
border: var(--brand-borderWidth-thick) solid var(--brand-color-focus);
box-shadow: 0 0 0 0.125rem var(--brand-color-focus);
}

.VideoPlayer__playButton svg {
color: var(--brand-videoPlayer-playButton-fgColor-rest);
}

Expand All @@ -73,7 +73,6 @@
transition: var(--brand-VideoPlayer-transition);
top: 0;
position: absolute;
border-radius: var(--brand-borderRadius-medium);
left: 0;
width: 100%;
z-index: 2;
Expand All @@ -85,6 +84,12 @@
grid-gap: var(--base-size-12);
grid-template-columns: auto auto;
background: var(--brand-videoPlayer-title-bgColor);
transition: all var(--brand-animation-duration-fast) var(--brand-animation-easing-default);
}

.VideoPlayer__title.VideoPlayer__title--hidden {
opacity: 0;
visibility: hidden;
}

/* ---------------------------------------------------------- */
Expand Down Expand Up @@ -115,8 +120,14 @@
left: 0;
width: 100%;
background: var(--brand-videoPlayer-controls-bgColor);
padding: var(--base-size-16) var(--base-size-24);
padding: var(--base-size-12) var(--base-size-16);
pointer-events: all;
opacity: 1;
}

.VideoPlayer__controlsBar--fade {
transition: opacity var(--brand-animation-duration-default) var(--brand-animation-easing-default);
opacity: 0;
}

.VideoPlayer__controls:focus,
Expand Down Expand Up @@ -228,9 +239,8 @@
}

.VideoPlayer__rangeInput:focus-visible {
border-color: var(--brand-color-focus);
outline: none;
box-shadow: 0 0 0 0.125rem var(--brand-color-focus);
outline: var(--brand-borderWidth-thick) solid var(--brand-color-focus);
outline-offset: var(--base-size-4);
opacity: 1;
}

Expand Down
5 changes: 4 additions & 1 deletion packages/react/src/VideoPlayer/VideoPlayer.module.css.d.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
declare const styles: {
readonly "VideoPlayer__container": string;
readonly "VideoPlayer": string;
readonly "VideoPlayer__playButton": string;
readonly "VideoPlayer__playButtonOverlay": string;
readonly "VideoPlayer__playButtonOverlay--transparent": string;
readonly "VideoPlayer__playButton": string;
readonly "VideoPlayer__title": string;
readonly "VideoPlayer__title--hidden": string;
readonly "VideoPlayer__controls": string;
readonly "VideoPlayer__controls--hidden": string;
readonly "VideoPlayer__controlsBar": string;
readonly "VideoPlayer__controlsBar--fade": string;
readonly "VideoPlayer__iconControl": string;
readonly "VideoPlayer__tooltip": string;
readonly "VideoPlayer__seek": string;
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/VideoPlayer/VideoPlayer.stories.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.customPlayIcon {
color: var(--base-color-scale-white-0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
declare const styles: {
readonly "customPlayIcon": string;
};
export = styles;

87 changes: 65 additions & 22 deletions packages/react/src/VideoPlayer/VideoPlayer.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useRef, forwardRef, useContext, type HTMLProps, type FunctionComponent} from 'react'
import React, {useEffect, useState, useRef, forwardRef, useContext, type HTMLProps, type FunctionComponent} from 'react'
import clsx from 'clsx'
import {Text} from '../Text'
import {type AnimateProps} from '../animation'
Expand Down Expand Up @@ -54,7 +54,7 @@ const Root = ({
showMuteButton = true,
showVolumeControl = true,
showFullScreenButton = true,
playIcon: PlayIcon = () => <DefaultPlayIcon className={styles.VideoPlayer__playButtonOverlay} />,
playIcon: PlayIcon = () => <DefaultPlayIcon className={styles.VideoPlayer__playButton} />,
...rest
}: VideoPlayerProps) => {
const videoWrapperRef = useRef<HTMLDivElement>(null)
Expand All @@ -63,41 +63,84 @@ const Root = ({
const useVideoContext = useVideo()
const {ccEnabled, isPlaying, ref, togglePlaying} = useVideoContext

const hideControls = !isPlaying && !showControlsWhenPaused
const [isInteracting, setIsInteracting] = useState(false)

useEffect(() => {
const videoWrapper = videoWrapperRef.current
let hideControlsTimeout: NodeJS.Timeout
const inactivityTimeout = 3000

if (!videoWrapper) {
return
}

const showControls = () => {
setIsInteracting(true)

clearTimeout(hideControlsTimeout)

hideControlsTimeout = setTimeout(() => {
setIsInteracting(false)
}, inactivityTimeout)
}

const hideControls = () => {
setIsInteracting(false)
}

videoWrapper.addEventListener('mousemove', showControls)
videoWrapper.addEventListener('mouseleave', hideControls)
videoWrapper.addEventListener('focusin', showControls)
videoWrapper.addEventListener('focusout', hideControls)

return () => {
videoWrapper.removeEventListener('mousemove', showControls)
videoWrapper.removeEventListener('mouseleave', hideControls)
videoWrapper.removeEventListener('focusin', showControls)
videoWrapper.removeEventListener('focusout', hideControls)

clearTimeout(hideControlsTimeout)
}
}, [videoWrapperRef])

const showControls = isInteracting || (showControlsWhenPaused && !isPlaying)

return (
<div className={styles.VideoPlayer__container} ref={videoWrapperRef}>
<video ref={ref} title={title} controls={false} className={clsx(styles.VideoPlayer, className)} {...rest}>
{children}
<track kind="captions" />
</video>
<div className={styles.VideoPlayer__title}>
{showBranding && <MarkGithubIcon size={40} />}
{!visuallyHiddenTitle && (
<Text size="400" weight="medium" className={styles.VideoPlayer__controlTextColor}>
{title}
</Text>
)}
</div>
{showBranding || !visuallyHiddenTitle ? (
<div className={clsx(styles.VideoPlayer__title, isPlaying && styles['VideoPlayer__title--hidden'])}>
{showBranding && <MarkGithubIcon size={40} />}
{!visuallyHiddenTitle && (
<Text size="400" weight="medium" className={styles.VideoPlayer__controlTextColor}>
{title}
</Text>
)}
</div>
) : null}
<button
className={styles.VideoPlayer__playButton}
className={clsx(
styles.VideoPlayer__playButtonOverlay,
isPlaying && styles['VideoPlayer__playButtonOverlay--transparent'],
)}
onClick={togglePlaying}
aria-label={isPlaying ? 'Pause' : 'Play'}
>
{!isPlaying && <PlayIcon />}
</button>
<div className={styles.VideoPlayer__controls}>
{ccEnabled && <Captions />}
{!hideControls && (
<ControlsBar>
{showPlayPauseButton && <PlayPauseButton />}
{showSeekControl && <SeekControl />}
{showCCButton && <CCButton />}
{showMuteButton && <MuteButton />}
{showVolumeControl && !isSmall && <VolumeControl />}
{showFullScreenButton && <FullScreenButton />}
</ControlsBar>
)}
<ControlsBar className={clsx(!showControls && styles['VideoPlayer__controlsBar--fade'])}>
{showPlayPauseButton && <PlayPauseButton />}
{showSeekControl && <SeekControl />}
{showCCButton && <CCButton />}
{showMuteButton && <MuteButton />}
{showVolumeControl && !isSmall && <VolumeControl />}
{showFullScreenButton && <FullScreenButton />}
</ControlsBar>
</div>
</div>
)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
19 changes: 16 additions & 3 deletions packages/react/src/VideoPlayer/components/Range/Range.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useState, useEffect, DOMAttributes} from 'react'
import React, {useState, useEffect, DOMAttributes, useRef} from 'react'
import clsx from 'clsx'
import {useId} from '@reach/auto-id'

Expand Down Expand Up @@ -30,23 +30,35 @@ export const Range = ({
const [hoverValue, setHoverValue] = useState(0)
const generatedId = useId()
const inputId = id || generatedId
const inputRef = useRef<HTMLInputElement | null>(null)

useEffect(() => {
setValue(startValue)
}, [startValue])

useEffect(() => {
if (!max || !tooltip || !inputRef.current) {
return
}

const handleMouseMove = event => {
if (event.target !== inputRef.current) {
setHoverValue(0)
setMousePos(0)
return
}

setMousePos(event.offsetX)
if (max) setHoverValue((event.offsetX / event.target.clientWidth) * max)

setHoverValue((event.offsetX / event.target.clientWidth) * max)
}

window.addEventListener('mousemove', handleMouseMove)

return () => {
window.removeEventListener('mousemove', handleMouseMove)
}
}, [max])
}, [max, tooltip, inputRef])

const handleKeyDown: DOMAttributes<HTMLInputElement>['onKeyDown'] = event => {
if (typeof value !== 'number') return
Expand Down Expand Up @@ -81,6 +93,7 @@ export const Range = ({
}}
id={inputId}
name={name}
ref={inputRef}
{...props}
/>
</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,15 @@ import {Text} from '../../../Text'
import {useVideo} from '../../hooks/useVideo'

const padTime = (time: number) => time.toString().padStart(2, '0')
const formatTime = (time: number) => {
const formatTime = (time: number, duration?: number) => {
if (isNaN(time) || time < 0) {
return '00:00'
}

if (duration && time > duration) {
time = duration
}

const minutes = padTime(Math.floor((time % 3600) / 60))
const seconds = padTime(Math.floor(time % 60))

Expand Down Expand Up @@ -62,7 +70,7 @@ export const SeekControl = ({className, ...rest}: SeekControlProps) => {
value={currentTime}
className={styles.VideoPlayer__progressBar}
tooltip
tooltipFormatter={formatTime}
tooltipFormatter={time => formatTime(time, duration)}
name="Seek"
/>
<div className={styles.VideoPlayer__progressTime}>
Expand All @@ -71,7 +79,7 @@ export const SeekControl = ({className, ...rest}: SeekControlProps) => {
className={clsx(styles.VideoPlayer__controlTextColor, styles.VideoPlayer__seekTime)}
font="monospace"
>
{formatTime(currentTime)}
{formatTime(currentTime, duration)}
{<span className={styles.VideoPlayer__totalTime}> / {formatTime(duration)}</span>}
</Text>
</div>
Expand Down

0 comments on commit 621d8ee

Please sign in to comment.