Skip to content

Commit 273f650

Browse files
shinaBR2claude
andauthored
Implement Video Player Hotkey Controls (#266)
* feat: implement comprehensive video player hotkeys Add full keyboard shortcut support for react-player video component: - Space/K: Play/Pause toggle - M: Toggle mute - F: Toggle fullscreen - Arrow keys: Seek (left/right 5s) and volume (up/down 5%) - J/L: Seek backward/forward 10 seconds (YouTube style) - 0-9: Jump to video percentages (0=start, 5=50%, 9=90%) - Home/End: Jump to beginning/end of video - </> (Shift+comma/period): Adjust playback speed by 0.25x Fixed commented-out arrow key seek functionality by using proper ReactPlayer API methods (seekTo() instead of direct currentTime). All hotkeys are disabled when typing in input fields to prevent conflicts with text entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: prevent native controls from interfering with arrow key seeking Use event capture phase and stopImmediatePropagation to intercept keyboard events before they reach the native HTML5 video controls. This fixes the issue where arrow keys would seek 30+ seconds instead of the intended 5 seconds due to both handlers responding. Changes: - Add { capture: true } to addEventListener for capture phase handling - Add stopImmediatePropagation() to all hotkey cases - Add explanatory comment about capture phase usage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: intercept keyboard events on wrapper element to prevent native controls interference Changed approach to fix arrow key double-seeking issue: - Added wrapperRef to Box container element - Made wrapper focusable with tabindex="0" and auto-focus on mount - Attached keyboard event listeners to both wrapper (capture phase) and document - This intercepts events before they reach the native video element The wrapper-level capture ensures our custom hotkeys fully override the native HTML5 video keyboard shortcuts, preventing the 30s jump when only 5s was intended. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix * f * test: fix video player unit tests for new hotkey implementation Updated ReactPlayer mock to include methods required by the new keyboard shortcut implementation: - Added mockGetCurrentTime(): Returns 10 seconds (mock current time) - Added mockGetDuration(): Returns 100 seconds (mock duration) - Added mockSeekTo(): Mock function for seeking operations - Added wrapper element mock for fullscreen functionality These methods are now called in the keyboard event handler to get the current playback position before seeking, which was causing tests to fail with "getCurrentTime is not a function" errors. All 12 tests now pass successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: prevent duplicate handlePlay/handlePause calls on keyboard toggle Previously, pressing Space or K to toggle play/pause would call handlePlay()/handlePause() twice: 1. Manual call in keyboard handler 2. Automatic call via ReactPlayer's onPlay/onPause events This caused duplicate progress tracking updates and other side effects. Changes: - Removed manual handlePlay()/handlePause() calls from keyboard handler - Now just toggles `playing` state and lets ReactPlayer's event handlers (onPlay/onPause) trigger the callbacks once - Updated useEffect dependencies to remove unused handlePlay/handlePause - Added explanatory comment about ReactPlayer handling side effects All tests still pass (12/12). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: improve accessibility and scope keyboard event listeners Accessibility improvements: - Made wrapper focusable declaratively with tabIndex={0} in JSX - Removed imperative auto-focus that steals focus from user on mount - Restored visible focus indicator (2px primary color outline) - Used :focus-visible to hide outline for mouse clicks Event listener scoping improvements: - Removed always-active document-level keyboard listener - Scoped keyboard shortcuts to wrapper element (requires focus) - Added fullscreenchange handler to dynamically add/remove document listener when entering/exiting fullscreen - Replaced stopImmediatePropagation() with stopPropagation() Benefits: - Better accessibility for screen readers and keyboard users - More predictable behavior (shortcuts require focus outside fullscreen) - No conflicts with other page elements - Cleaner architecture (declarative vs imperative) - Global shortcuts only in fullscreen when it makes UX sense Test updates: - Updated all keyboard tests to fire events on wrapper instead of document - Added data-testid="video-player-wrapper" for testing - All 12 tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a3d2233 commit 273f650

3 files changed

Lines changed: 222 additions & 51 deletions

File tree

apps/watch/src/routeTree.gen.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/* eslint-disable */
2+
13
// @ts-nocheck
24

35
// noinspection JSUnusedGlobalSymbols
@@ -27,19 +29,19 @@ const HistoryLazyRoute = HistoryLazyImport.update({
2729
id: '/history',
2830
path: '/history',
2931
getParentRoute: () => rootRoute,
30-
} as const).lazy(() => import('./routes/history.lazy').then((d) => d.Route));
32+
} as any).lazy(() => import('./routes/history.lazy').then((d) => d.Route));
3133

3234
const IndexLazyRoute = IndexLazyImport.update({
3335
id: '/',
3436
path: '/',
3537
getParentRoute: () => rootRoute,
36-
} as const).lazy(() => import('./routes/index.lazy').then((d) => d.Route));
38+
} as any).lazy(() => import('./routes/index.lazy').then((d) => d.Route));
3739

3840
const VideoSlugIdLazyRoute = VideoSlugIdLazyImport.update({
3941
id: '/video/$slug/$id',
4042
path: '/video/$slug/$id',
4143
getParentRoute: () => rootRoute,
42-
} as const).lazy(() =>
44+
} as any).lazy(() =>
4345
import('./routes/video.$slug.$id.lazy').then((d) => d.Route),
4446
);
4547

@@ -48,7 +50,7 @@ const PlaylistSlugPlaylistIdVideoIdLazyRoute =
4850
id: '/playlist/$slug/$playlistId/$videoId',
4951
path: '/playlist/$slug/$playlistId/$videoId',
5052
getParentRoute: () => rootRoute,
51-
} as const).lazy(() =>
53+
} as any).lazy(() =>
5254
import('./routes/playlist.$slug.$playlistId.$videoId.lazy').then(
5355
(d) => d.Route,
5456
),

packages/ui/src/watch/videos/video-player/index.test.tsx

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import { VideoPlayer } from './index';
66

77
// Mock ReactPlayer
88
const mockReactPlayerRef = vi.fn();
9+
const mockGetCurrentTime = vi.fn(() => 10); // Mock current time at 10 seconds
10+
const mockGetDuration = vi.fn(() => 100); // Mock duration at 100 seconds
11+
const mockSeekTo = vi.fn();
12+
913
vi.mock('react-player', () => ({
1014
__esModule: true,
1115
default: React.forwardRef(({ url, onReady, ...props }, ref) => {
@@ -16,6 +20,10 @@ vi.mock('react-player', () => ({
1620
ref({
1721
paused: false,
1822
requestFullscreen: vi.fn(),
23+
getCurrentTime: mockGetCurrentTime,
24+
getDuration: mockGetDuration,
25+
seekTo: mockSeekTo,
26+
wrapper: document.createElement('div'), // Mock wrapper element for fullscreen
1927
});
2028
}
2129
return React.createElement(
@@ -191,69 +199,78 @@ describe('VideoPlayer', () => {
191199
describe('VideoPlayer keyboard hotkeys', () => {
192200
beforeEach(() => {
193201
vi.clearAllMocks();
202+
// Reset mock implementations for each test
203+
mockGetCurrentTime.mockReturnValue(10);
204+
mockGetDuration.mockReturnValue(100);
194205
});
195206

196207
it('should handle keyboard events for play/pause (k key)', async () => {
197208
render(<VideoPlayer video={mockVideo} />);
198-
await screen.findByTestId('mock-react-player');
209+
const wrapper = await screen.findByTestId('video-player-wrapper');
199210

200-
// Simulate 'k' key press
201-
const kEvent = new KeyboardEvent('keydown', { key: 'k' });
211+
// Simulate 'k' key press on wrapper (scoped listener)
212+
const kEvent = new KeyboardEvent('keydown', { key: 'k', bubbles: true });
202213
Object.defineProperty(kEvent, 'preventDefault', { value: vi.fn() });
203214

204-
fireEvent(document, kEvent);
215+
fireEvent(wrapper, kEvent);
205216

206217
expect(kEvent.preventDefault).toHaveBeenCalled();
207218
});
208219

209220
it('should handle keyboard events for mute (m key)', async () => {
210221
render(<VideoPlayer video={mockVideo} />);
211-
await screen.findByTestId('mock-react-player');
222+
const wrapper = await screen.findByTestId('video-player-wrapper');
212223

213-
// Simulate 'm' key press
214-
const mEvent = new KeyboardEvent('keydown', { key: 'm' });
224+
// Simulate 'm' key press on wrapper (scoped listener)
225+
const mEvent = new KeyboardEvent('keydown', { key: 'm', bubbles: true });
215226
Object.defineProperty(mEvent, 'preventDefault', { value: vi.fn() });
216227

217-
fireEvent(document, mEvent);
228+
fireEvent(wrapper, mEvent);
218229

219230
expect(mEvent.preventDefault).toHaveBeenCalled();
220231
});
221232

222233
it('should handle keyboard events for volume up (ArrowUp key)', async () => {
223234
render(<VideoPlayer video={mockVideo} />);
224-
await screen.findByTestId('mock-react-player');
235+
const wrapper = await screen.findByTestId('video-player-wrapper');
225236

226-
// Simulate 'ArrowUp' key press
227-
const upEvent = new KeyboardEvent('keydown', { key: 'ArrowUp' });
237+
// Simulate 'ArrowUp' key press on wrapper (scoped listener)
238+
const upEvent = new KeyboardEvent('keydown', {
239+
key: 'ArrowUp',
240+
bubbles: true,
241+
});
228242
Object.defineProperty(upEvent, 'preventDefault', { value: vi.fn() });
229243

230-
fireEvent(document, upEvent);
244+
fireEvent(wrapper, upEvent);
231245

232246
expect(upEvent.preventDefault).toHaveBeenCalled();
233247
});
234248

235249
it('should handle keyboard events for volume down (ArrowDown key)', async () => {
236250
render(<VideoPlayer video={mockVideo} />);
237-
await screen.findByTestId('mock-react-player');
251+
const wrapper = await screen.findByTestId('video-player-wrapper');
238252

239-
// Simulate 'ArrowDown' key press
240-
const downEvent = new KeyboardEvent('keydown', { key: 'ArrowDown' });
253+
// Simulate 'ArrowDown' key press on wrapper (scoped listener)
254+
const downEvent = new KeyboardEvent('keydown', {
255+
key: 'ArrowDown',
256+
bubbles: true,
257+
});
241258
Object.defineProperty(downEvent, 'preventDefault', { value: vi.fn() });
242259

243-
fireEvent(document, downEvent);
260+
fireEvent(wrapper, downEvent);
244261

245262
expect(downEvent.preventDefault).toHaveBeenCalled();
246263
});
247264

248265
it('should handle keyboard events for fullscreen (f key)', async () => {
249266
render(<VideoPlayer video={mockVideo} />);
250-
await screen.findByTestId('mock-react-player');
267+
const wrapper = await screen.findByTestId('video-player-wrapper');
251268

252-
// Simulate 'f' key press
253-
const fEvent = new KeyboardEvent('keydown', { key: 'f' });
269+
// Simulate 'f' key press on wrapper (scoped listener)
270+
const fEvent = new KeyboardEvent('keydown', { key: 'f', bubbles: true });
254271
Object.defineProperty(fEvent, 'preventDefault', { value: vi.fn() });
255272

256-
fireEvent(document, fEvent);
273+
fireEvent(wrapper, fEvent);
257274

258275
expect(fEvent.preventDefault).toHaveBeenCalled();
259276
});

0 commit comments

Comments
 (0)