Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions packages/core/core/src/components/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ export interface SliderState {
_pointerRatio: number;
_hovering: boolean;
_dragging: boolean;
_keying: boolean;
_fillWidth: number;
_pointerWidth: number;
_stepSize: number;
}

export class Slider {
Expand All @@ -19,8 +21,10 @@ export class Slider {
_pointerRatio: 0,
_hovering: false,
_dragging: false,
_keying: false,
_fillWidth: 0,
_pointerWidth: 0,
_stepSize: 0.01,
});

attach(target: HTMLElement): void {
Expand All @@ -34,6 +38,8 @@ export class Slider {
this.#element.addEventListener('pointerup', this, { signal });
this.#element.addEventListener('pointerenter', this, { signal });
this.#element.addEventListener('pointerleave', this, { signal });
this.#element.addEventListener('keydown', this, { signal });
this.#element.addEventListener('keyup', this, { signal });
}

detach(): void {
Expand Down Expand Up @@ -80,6 +86,12 @@ export class Slider {
case 'pointerleave':
this.#handlePointerLeave(event as PointerEvent);
break;
case 'keydown':
this.#handleKeyDown(event as KeyboardEvent);
break;
case 'keyup':
this.#handleKeyUp(event as KeyboardEvent);
break;
}
}

Expand Down Expand Up @@ -120,6 +132,46 @@ export class Slider {
#handlePointerLeave(_event: PointerEvent) {
this.setState({ _hovering: false });
}

#handleKeyDown(event: KeyboardEvent) {
const { key } = event;
const { _pointerRatio, _stepSize } = this.#state.get();

let newRatio = _pointerRatio;

switch (key) {
case 'ArrowLeft':
case 'ArrowDown':
event.preventDefault();
newRatio = Math.max(0, _pointerRatio - _stepSize);
break;
case 'ArrowRight':
case 'ArrowUp':
event.preventDefault();
newRatio = Math.min(1, _pointerRatio + _stepSize);
break;
case 'Home':
event.preventDefault();
newRatio = 0;
break;
case 'End':
event.preventDefault();
newRatio = 1;
break;
default:
return; // Don't update state for other keys
}

this.setState({ _pointerRatio: newRatio, _keying: true });
}

#handleKeyUp(_event: KeyboardEvent) {
this.setState({ _keying: false });
}

setStepSize(stepSize: number): void {
this.setState({ _stepSize: Math.max(0.001, Math.min(1, stepSize)) });
}
}

export interface Point {
Expand Down
25 changes: 23 additions & 2 deletions packages/core/core/src/components/time-slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ export class TimeSlider extends Slider {
getState(): TimeSliderState {
const state = super.getState() as TimeSliderState;

// When dragging, use pointer position for immediate feedback;
// When dragging or keying, use pointer position for immediate feedback;
// While seeking, use seeking time so it doesn't jump back to the current time;
// Otherwise, use current time;
let _fillWidth = 0;
if (state._dragging) {
if (state._dragging || state._keying) {
_fillWidth = state._pointerRatio * 100;
} else if (state.duration > 0) {
if (this.#seekingTime !== null && this.#oldCurrentTime === state.currentTime) {
Expand All @@ -41,6 +41,15 @@ export class TimeSlider extends Slider {
return { ...state, _fillWidth, _currentTimeText, _durationText };
}

setState(state: Partial<TimeSliderState>): void {
// When not dragging or keying, set pointer ratio to current time / duration.
if (!state._dragging && !state._keying && state.currentTime && state.duration) {
super.setState({ ...state, _pointerRatio: state.currentTime / state.duration });
return;
}
super.setState(state);
}

handleEvent(event: Event): void {
const { type } = event;
switch (type) {
Expand All @@ -53,6 +62,9 @@ export class TimeSlider extends Slider {
case 'pointerup':
this.#handlePointerUp(event as PointerEvent);
break;
case 'keydown':
this.#handleKeyDown(event as KeyboardEvent);
break;
default:
super.handleEvent(event);
break;
Expand Down Expand Up @@ -92,6 +104,15 @@ export class TimeSlider extends Slider {

super.handleEvent(event);
}

#handleKeyDown(event: KeyboardEvent) {
super.handleEvent(event);

const { _pointerRatio, duration, requestSeek } = super.getState() as TimeSliderState;

this.#seekingTime = _pointerRatio * duration;
requestSeek(this.#seekingTime);
}
Comment on lines +108 to +115
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as in VolumeSlider: calling getState() immediately after super.handleEvent(event) may not capture the updated _pointerRatio value if state updates are asynchronous. This aligns with the PR description's noted bug about 'relative value bug in time slider and volume slider'.

Copilot uses AI. Check for mistakes.
}

function formatTime(time: number): string {
Expand Down
28 changes: 26 additions & 2 deletions packages/core/core/src/components/volume-slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,18 @@ export interface VolumeSliderState extends SliderState {
}

export class VolumeSlider extends Slider {
constructor() {
super();
this.setStepSize(0.1);
}

getState(): VolumeSliderState {
const state = super.getState() as VolumeSliderState;

// When dragging, use pointer position for immediate feedback;
// When dragging or keying, use pointer position for immediate feedback;
// Otherwise, use current volume;
let _fillWidth = 0;
if (state._dragging) {
if (state._dragging || state._keying) {
_fillWidth = state._pointerRatio * 100;
} else {
_fillWidth = state.muted ? 0 : (state.volume || 0) * 100;
Expand All @@ -28,6 +33,15 @@ export class VolumeSlider extends Slider {
return { ...state, _fillWidth, _volumeText };
}

setState(state: Partial<VolumeSliderState>): void {
// When not dragging or keying, set pointer ratio to current volume.
if (!state._dragging && !state._keying && state.volume) {
super.setState({ ...state, _pointerRatio: state.volume });
return;
}
super.setState(state);
}

handleEvent(event: Event): void {
const { type } = event;
switch (type) {
Expand All @@ -40,6 +54,9 @@ export class VolumeSlider extends Slider {
case 'pointerup':
this.#handlePointerUp(event as PointerEvent);
break;
case 'keydown':
this.#handleKeyDown(event as KeyboardEvent);
break;
default:
super.handleEvent(event);
break;
Expand Down Expand Up @@ -72,6 +89,13 @@ export class VolumeSlider extends Slider {

super.handleEvent(event);
}

#handleKeyDown(event: KeyboardEvent) {
super.handleEvent(event);

const { _pointerRatio, requestVolumeChange } = super.getState() as VolumeSliderState;
requestVolumeChange(_pointerRatio);
Comment on lines +94 to +97
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keyboard event is passed to super.handleEvent(event) which updates _pointerRatio, but then getState() is called immediately after. Due to potential asynchronous state updates, _pointerRatio may not reflect the updated value from the keyboard handler. Consider getting the updated ratio directly from the parent's keyboard handler return value or accessing the updated state differently.

Suggested change
super.handleEvent(event);
const { _pointerRatio, requestVolumeChange } = super.getState() as VolumeSliderState;
requestVolumeChange(_pointerRatio);
// Compute the new pointer ratio synchronously based on the key event
const state = super.getState() as VolumeSliderState;
let newPointerRatio = state._pointerRatio;
// Example logic: adjust ratio based on arrow keys
if (event.key === 'ArrowLeft' || event.key === 'ArrowDown') {
newPointerRatio = Math.max(0, newPointerRatio - this.getStepSize());
} else if (event.key === 'ArrowRight' || event.key === 'ArrowUp') {
newPointerRatio = Math.min(1, newPointerRatio + this.getStepSize());
}
const { requestVolumeChange } = state;
requestVolumeChange(newPointerRatio);
// Optionally, update the pointer ratio in state if needed
// (depends on parent class implementation)
// super.setPointerRatio(newPointerRatio);

Copilot uses AI. Check for mistakes.
}
}

function formatVolume(volume: number): string {
Expand Down
7 changes: 2 additions & 5 deletions packages/core/media-store/src/factory.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { getKey, map, subscribeKeys } from 'nanostores';
import type { StateOwners } from './types';

export interface StateOwners {
media?: any;
container?: any;
}
import { getKey, map, subscribeKeys } from 'nanostores';

export interface EventOrAction<D = undefined> {
type: string;
Expand Down
18 changes: 10 additions & 8 deletions packages/core/media-store/src/state-mediators/audible.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import type { StateOwners } from '../types';

export const audible = {
muted: {
get(stateOwners: any): boolean {
get(stateOwners: StateOwners): boolean {
const { media } = stateOwners;
return media?.muted ?? false;
},
set(value: boolean, stateOwners: any): void {
set(value: boolean, stateOwners: StateOwners): void {
const { media } = stateOwners;
if (!media) return;
media.muted = value;
Expand All @@ -13,7 +15,7 @@ export const audible = {
}
},
stateOwnersUpdateHandlers: [
(handler: (value?: boolean) => void, stateOwners: any): (() => void) | void => {
(handler: (value?: boolean) => void, stateOwners: StateOwners): (() => void) | void => {
const { media } = stateOwners;
if (!media) return;

Expand All @@ -30,22 +32,22 @@ export const audible = {
},
},
volume: {
get(stateOwners: any): number {
get(stateOwners: StateOwners): number {
const { media } = stateOwners;
return media?.volume ?? 1.0;
},
set(value: number, stateOwners: any): void {
set(value: number, stateOwners: StateOwners): void {
const { media } = stateOwners;
if (!media) return;
const numericValue = +value;
if (!Number.isFinite(numericValue)) return;
media.volume = numericValue;
if (numericValue > 0) {
media.mute = false;
media.muted = false;
}
},
stateOwnersUpdateHandlers: [
(handler: (value?: number) => void, stateOwners: any): (() => void) | void => {
(handler: (value?: number) => void, stateOwners: StateOwners): (() => void) | void => {
const { media } = stateOwners;
if (!media) return;

Expand All @@ -62,7 +64,7 @@ export const audible = {
},
// NOTE: This could be (re)implemented as "derived state" in some manner (e.g. selectors but also other patterns/conventions) if preferred. (CJP)
volumeLevel: {
get(stateOwners: any): 'high' | 'medium' | 'low' | 'off' {
get(stateOwners: StateOwners): 'high' | 'medium' | 'low' | 'off' {
const { media } = stateOwners;
if (typeof media?.volume == 'undefined') return 'high';
if (media.muted || media.volume === 0) return 'off';
Expand Down
4 changes: 4 additions & 0 deletions packages/core/media-store/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface StateOwners {
media?: HTMLMediaElement;
container?: HTMLElement;
}
47 changes: 38 additions & 9 deletions packages/html/html/src/components/media-popover.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
import type { Placement } from '@floating-ui/dom';
import { autoUpdate, computePosition, flip, offset, shift } from '@floating-ui/dom';

import { getNextTabbable, getPreviousTabbable, isOutsideEvent, uniqueId } from '../utils/element-utils';
import { getDocument, getNextTabbable, getPreviousTabbable, isOutsideEvent, uniqueId } from '../utils/element-utils';

export class MediaPopoverRoot extends HTMLElement {
#open = false;
#hoverTimeout: ReturnType<typeof setTimeout> | null = null;
#cleanup: (() => void) | null = null;
#transitionStatus: 'initial' | 'open' | 'close' | 'unmounted' = 'initial';

constructor() {
super();
this.addEventListener('mouseenter', this.#handleMouseEnter.bind(this));
this.addEventListener('mouseleave', this.#handleMouseLeave.bind(this));
this.addEventListener('focusin', this.#handleFocusIn.bind(this));
this.addEventListener('focusout', this.#handleFocusOut.bind(this));
}
#abortController: AbortController | null = null;

connectedCallback(): void {
this.#updateVisibility();

this.#abortController ??= new AbortController();
const { signal } = this.#abortController;

this.addEventListener('mouseenter', this, { signal });
this.addEventListener('mouseleave', this, { signal });
this.addEventListener('focusin', this, { signal });
this.addEventListener('focusout', this, { signal });

getDocument(this).documentElement.addEventListener('mouseleave', this, { signal });
}

disconnectedCallback(): void {
Expand All @@ -27,6 +30,28 @@ export class MediaPopoverRoot extends HTMLElement {

this.#transitionStatus = 'unmounted';
this.#updateVisibility();

this.#abortController?.abort();
this.#abortController = null;
}

handleEvent(event: Event): void {
switch (event.type) {
case 'mouseenter':
this.#handleMouseEnter();
break;
case 'mouseleave':
this.#handleMouseLeave(event as MouseEvent);
break;
case 'focusin':
this.#handleFocusIn(event as FocusEvent);
break;
case 'focusout':
this.#handleFocusOut(event as FocusEvent);
break;
default:
break;
}
}

static get observedAttributes(): string[] {
Expand Down Expand Up @@ -99,6 +124,10 @@ export class MediaPopoverRoot extends HTMLElement {
this.#popupElement.toggleAttribute('data-open', this.#transitionStatus === 'initial' || this.#transitionStatus === 'open');
this.#popupElement.toggleAttribute('data-ending-style', this.#transitionStatus === 'close' || this.#transitionStatus === 'unmounted');
this.#popupElement.toggleAttribute('data-closed', this.#transitionStatus === 'close' || this.#transitionStatus === 'unmounted');

this.#abortController ??= new AbortController();
const { signal } = this.#abortController;
this.#popupElement.addEventListener('mouseleave', this, { signal });
}

const triggerElement = this.#triggerElement?.firstElementChild as HTMLElement;
Expand Down
7 changes: 4 additions & 3 deletions packages/react/react/src/components/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ function PopoverRoot({ openOnHover = false, delay = 0, closeDelay = 0, children
const hover = useHover(context, {
enabled: openOnHover,
mouseOnly: true,
move: false,
delay: {
open: delay,
close: closeDelay,
Expand Down Expand Up @@ -161,7 +162,7 @@ function PopoverPositioner({ side = 'top', sideOffset = 5, children }: PopoverPo
}

function PopoverPopup({ className, children }: PopoverPopupProps): JSX.Element {
const { getFloatingProps, context, openReason, transitionStatus } = usePopoverContext();
const { getFloatingProps, context, transitionStatus } = usePopoverContext();
Comment thread
luwes marked this conversation as resolved.
const { refs, placement } = context;
const triggerElement = refs.reference.current as HTMLElement | null;

Expand All @@ -176,10 +177,10 @@ function PopoverPopup({ className, children }: PopoverPopupProps): JSX.Element {

return (
<FloatingFocusManager
disabled={openReason === 'hover'}
context={context}
modal={false}
initialFocus={context.refs.reference as MutableRefObject<HTMLElement>}
initialFocus={-1}
returnFocus={false}
>
<div
className={className}
Expand Down