fix(embedded-mpv): address native-view dock review feedback

Resolves the actionable P2 review comments on the dock rework:

- Inline volume no longer clips the dock actions. At sidebar-constrained
  player widths (~480-660px, viewport wider than the 720px breakpoint) the
  new in-flow volume slider widened the non-shrinking actions column and,
  under overflow:hidden, clipped the fullscreen button. Add min-width:0 to
  .embedded-mpv-player__actions and __volume-group so the inline volume (a
  scroll container) compresses its own slider instead of pushing neighbors
  off-edge. Verified in Chromium: fullscreen stays visible down to 480px.
- Space now selects a focused chip. onPanelKeydown stops Space/Enter from
  bubbling to the global shortcut handler (whose Space case preventDefault'd
  the button's native activation and toggled playback) without calling
  preventDefault itself, so the menuitemradio chip activates and emits
  chipSelected. Matches the WAI-ARIA menu activation-key expectation.
- Simplify dock-panel opener tracking: always remember the toggled kind so
  focus restoration is correct if in-panel switching ever becomes reachable
  (currently unreachable — the toggle buttons are removed from the DOM while
  a panel is open); restoreOpenerFocus still no-ops unless focus fell to body.

Not changed: the "closePanels no-ops when unavailable" comment — verified
unreachable (chip selection closes via menus.close() directly, not through
closePopovers; isAvailable() is engine-bound and the native dock only renders
while it is true, with engine handoff calling menus.closeAll()).

Regression test added for Space/Enter chip activation. The volume-overflow
fix is CSS layout (no jsdom layout engine) and was validated in a real
browser.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-07-19 17:33:40 +02:00
1 parent 4bc6f7e919
commit 93e6f60582
4 files changed
+40 -1

No files matched your search

@@ -114,6 +114,26 @@ describe('EmbeddedMpvDockPanelComponent', () => {
document.removeEventListener('keydown', documentKeydown);
});
it('lets Space/Enter activate a chip without leaking to global shortcuts', () => {
// The global shortcut handler's Space case calls preventDefault() +
// togglePaused(); preventing the keydown default would suppress the
// chip button's native activation. The panel must stop Space/Enter
// from reaching the document without cancelling their default action.
const documentKeydown = jest.fn();
document.addEventListener('keydown', documentKeydown);
const space = dispatchPanelKey(' ');
const enter = dispatchPanelKey('Enter');
// Not defaultPrevented → the focused chip button still activates.
expect(space.defaultPrevented).toBe(false);
expect(enter.defaultPrevented).toBe(false);
// stopPropagation → the global handler never sees them.
expect(documentKeydown).not.toHaveBeenCalled();
document.removeEventListener('keydown', documentKeydown);
});
it('maps vertical wheel deltas to horizontal ribbon scrolling', () => {
const ribbonEl = ribbon();
ribbonEl.scrollLeft = 0;
@@ -106,6 +106,15 @@ export class EmbeddedMpvDockPanelComponent implements OnDestroy {
onPanelKeydown(event: KeyboardEvent): void {
const key = event.key;
if (key === ' ' || key === 'Enter') {
// Let the focused chip button activate natively (Space/Enter →
// click → chipSelected), but stop the keydown before it reaches
// the global shortcut handler, whose Space case would otherwise
// preventDefault() the activation and toggle playback instead.
// No preventDefault here — that would re-suppress the click.
event.stopPropagation();
return;
}
if (key === 'ArrowUp' || key === 'ArrowDown') {
// The panel owns the keyboard: never let the global volume
// shortcuts fire while a chip panel is open.
@@ -66,7 +66,11 @@ export class EmbeddedMpvDockPanelState {
}
toggle(kind: EmbeddedMpvDockPanelKind): void {
this.openerKind = this.deps.menus.dockPanelOpen() ? null : kind;
// Always remember the button that opened a panel so focus returns to
// it when the panel closes. restoreOpenerFocus() only acts when focus
// has fallen to document.body, so re-toggling a panel with focus still
// on its button never steals focus.
this.openerKind = kind;
this.deps.menus.toggle(kind);
this.deps.revealControls();
}
@@ -294,11 +294,17 @@
display: flex;
align-items: center;
gap: 8px;
// Let the actions cell shrink below its content so the inline volume
// (a scroll container) clips its own slider instead of pushing the
// fullscreen button past the overflow:hidden player edge on narrow
// sidebar-constrained widths (viewport wider than the 720px breakpoint).
min-width: 0;
}
.embedded-mpv-player__volume-group {
display: flex;
align-items: center;
min-width: 0;
}
.embedded-mpv-player__control-button {