Skip to content

Commit

Permalink
Show VideoPlayer tooltips when associated control receives focus (#872)
Browse files Browse the repository at this point in the history
* show VideoTooltip when the parent is focused (or contains focus) and allow dismissal with Esc key

* remove duplicate labels

* reduce scope of mousemove listener

* add changeset

* add interaction test to test tooltip focus visibility

* update snapshots
  • Loading branch information
joshfarrant authored Jan 13, 2025
1 parent bbdf7f2 commit 872bdcf
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/wet-plants-melt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react-brand': patch
---

`VideoPlayer` tooltips now show when the associated control receives focus.
18 changes: 18 additions & 0 deletions packages/react/src/VideoPlayer/VideoPlayer.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Stack} from '../Stack'
import {Button} from '../Button'
import {useVideo} from './hooks'
import styles from './VideoPlayer.stories.module.css'
import {expect, userEvent, waitFor, within} from '@storybook/test'

export default {
title: 'Components/VideoPlayer/Features',
Expand Down Expand Up @@ -104,3 +105,20 @@ export const CustomPlayIcon = () => (
<VideoPlayer.Track src="./example.vtt" default />
</VideoPlayer>
)

export const TooltipVisibleOnFocus = () => (
<VideoPlayer title="GitHub media player">
<VideoPlayer.Source src="./example.mp4" type="video/mp4" />
<VideoPlayer.Track src="./example.vtt" default />
</VideoPlayer>
)
TooltipVisibleOnFocus.play = async ({canvasElement}) => {
const {getByText} = within(canvasElement)

await waitFor(() => expect(getByText('Play video')).not.toBeVisible())

await userEvent.tab()
await userEvent.tab()

await waitFor(() => expect(getByText('Play video')).toBeVisible())
}
4 changes: 4 additions & 0 deletions packages/react/src/VideoPlayer/VideoPlayer.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@
color: var(--base-color-scale-gray-9);
}

.VideoPlayer__tooltip-visible {
opacity: 1;
}

.VideoPlayer__controlTextColor {
color: var(--base-color-scale-white-0);
}
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/VideoPlayer/VideoPlayer.module.css.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ declare const styles: {
readonly "VideoPlayer__rangeProgress": string;
readonly "VideoPlayer__tooltipContent": string;
readonly "VideoPlayer__tooltipText": string;
readonly "VideoPlayer__tooltip-visible": string;
readonly "VideoPlayer__controlTextColor": string;
readonly "VideoPlayer__seekTime": string;
};
Expand Down
9 changes: 9 additions & 0 deletions packages/react/src/VideoPlayer/VideoPlayer.visual.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,13 @@ test.describe('Visual Comparison: VideoPlayer', () => {
await page.waitForTimeout(500)
expect(await page.screenshot({fullPage: true})).toMatchSnapshot()
})

test('VideoPlayer / Tooltip Visible On Focus', async ({page}) => {
await page.goto(
'http://localhost:6006/iframe.html?args=&id=components-videoplayer-features--tooltip-visible-on-focus&viewMode=story',
)

await page.waitForTimeout(500)
expect(await page.screenshot({fullPage: true})).toMatchSnapshot()
})
})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ export const CCButton = ({className, ...props}: HTMLAttributes<HTMLButtonElement
!ccEnabled && styles.VideoPlayer__ccOff,
)}
onClick={toggleCC}
aria-label={ccEnabled ? 'Disable captions' : 'Enable captions'}
{...props}
>
<Text className={styles.VideoPlayer__ccText}>CC</Text>
<Text className={styles.VideoPlayer__ccText} aria-hidden="true">
CC
</Text>
<VideoTooltip>{ccEnabled ? 'Disable captions' : 'Enable captions'}</VideoTooltip>
</button>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ type IconControlProps = {
} & HTMLAttributes<HTMLButtonElement>

export const IconControl = ({tooltip, children, className, ...rest}: IconControlProps) => (
<button className={clsx(styles.VideoPlayer__iconControl, className)} {...rest} aria-label={tooltip}>
<button className={clsx(styles.VideoPlayer__iconControl, className)} {...rest}>
{children}
<span className="visually-hidden">{tooltip}</span>
<VideoTooltip>{tooltip}</VideoTooltip>
</button>
)
10 changes: 6 additions & 4 deletions packages/react/src/VideoPlayer/components/Range/Range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ export const Range = ({
}, [startValue])

useEffect(() => {
if (!max || !tooltip || !inputRef.current) {
const input = inputRef.current

if (!max || !tooltip || !input) {
return
}

const handleMouseMove = event => {
if (event.target !== inputRef.current) {
if (event.target !== input) {
setHoverValue(0)
setMousePos(0)
return
Expand All @@ -53,10 +55,10 @@ export const Range = ({
setHoverValue((event.offsetX / event.target.clientWidth) * max)
}

window.addEventListener('mousemove', handleMouseMove)
input.addEventListener('mousemove', handleMouseMove)

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,60 @@
import React, {type HTMLAttributes} from 'react'
import React, {useEffect, useRef, useState, type HTMLAttributes} from 'react'
import clsx from 'clsx'

import {Text} from '../../../Text'
import styles from '../../VideoPlayer.module.css'

type VideoTooltipProps = HTMLAttributes<HTMLDivElement>

export const VideoTooltip = ({children, className, ...rest}: VideoTooltipProps) => (
<div className={clsx(styles.VideoPlayer__tooltip, className)} {...rest}>
<span className={styles.VideoPlayer__tooltipContent}>
<Text className={styles.VideoPlayer__tooltipText} weight="medium">
{children}
</Text>
</span>
</div>
)
export const VideoTooltip = ({children, className, ...rest}: VideoTooltipProps) => {
const tooltipRef = useRef<HTMLDivElement>(null)
const [hasFocus, setHasFocus] = useState(false)

useEffect(() => {
const tooltip = tooltipRef.current
const parent = tooltip?.parentElement

if (!tooltip || !parent) {
return
}

const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
setHasFocus(false)
}
}

const checkFocus = () => {
const isFocused = parent === document.activeElement || parent.contains(document.activeElement)
setHasFocus(isFocused)
}

parent.addEventListener('focus', checkFocus)
parent.addEventListener('blur', checkFocus)
parent.addEventListener('focusin', checkFocus)
parent.addEventListener('focusout', checkFocus)
window.addEventListener('keydown', handleKeyDown)

return () => {
parent.removeEventListener('focus', checkFocus)
parent.removeEventListener('blur', checkFocus)
parent.removeEventListener('focusin', checkFocus)
parent.removeEventListener('focusout', checkFocus)
window.removeEventListener('keydown', handleKeyDown)
}
}, [tooltipRef])

return (
<div
className={clsx(styles.VideoPlayer__tooltip, hasFocus && styles['VideoPlayer__tooltip-visible'], className)}
ref={tooltipRef}
{...rest}
>
<span className={styles.VideoPlayer__tooltipContent}>
<Text className={styles.VideoPlayer__tooltipText} weight="medium">
{children}
</Text>
</span>
</div>
)
}

0 comments on commit 872bdcf

Please sign in to comment.