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
85 changes: 76 additions & 9 deletions packages/core/src/core/ui/time/tests/time-core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('TimeCore', () => {
core.setMedia(createMediaState());
const state = core.getState();
expect(state.type).toBe('current');
expect(TimeCore.defaultProps.toggle).toBe(false);
});

it('accepts custom props', () => {
Expand Down Expand Up @@ -103,21 +104,21 @@ describe('TimeCore', () => {
const core = new TimeCore({ type: 'current' });
core.setMedia(createMediaState());
const state = core.getState();
expect(core.getLabel(state)).toBe('Current time');
expect(core.getLabel(state)).toBe('1 minute, 30 seconds');
});

it('returns default label for duration', () => {
const core = new TimeCore({ type: 'duration' });
core.setMedia(createMediaState());
const state = core.getState();
expect(core.getLabel(state)).toBe('Duration');
expect(core.getLabel(state)).toBe('5 minutes');
});

it('returns default label for remaining', () => {
const core = new TimeCore({ type: 'remaining' });
core.setMedia(createMediaState());
const state = core.getState();
expect(core.getLabel(state)).toBe('Remaining');
expect(core.getLabel(state)).toBe('3 minutes, 30 seconds remaining');
});

it('returns custom string label', () => {
Expand All @@ -136,27 +137,93 @@ describe('TimeCore', () => {
const state = core.getState();
expect(core.getLabel(state)).toBe('Time: 1:30');
});

it('returns toggle label for current', () => {
const core = new TimeCore({ type: 'current', toggle: true });
core.setMedia(createMediaState());
const state = core.getState();
expect(core.getLabel(state)).toBe('1 minute, 30 seconds. Show remaining time.');
});

it('returns toggle label for remaining', () => {
const core = new TimeCore({ type: 'remaining', toggle: true });
core.setMedia(createMediaState());
const state = core.getState();
expect(core.getLabel(state)).toBe('3 minutes, 30 seconds remaining. Show duration.');
});

it('returns elapsed action when remaining toggles from current', () => {
const core = new TimeCore({ type: 'remaining', toggle: true });
core.setMedia(createMediaState());
const state = core.getState();
expect(core.getLabel(state, 'current')).toBe('3 minutes, 30 seconds remaining. Show elapsed time.');
});
});

describe('getAttrs', () => {
it('returns aria attributes', () => {
it('returns aria-label', () => {
const core = new TimeCore({ type: 'current' });
core.setMedia(createMediaState({ currentTime: 90 }));
const state = core.getState();
const attrs = core.getAttrs(state);

expect(attrs['aria-label']).toBe('Current time');
expect(attrs['aria-valuetext']).toBe('1 minute, 30 seconds');
expect(attrs['aria-label']).toBe('1 minute, 30 seconds');
expect(attrs).not.toHaveProperty('aria-valuetext');
});

it('includes remaining suffix in valuetext', () => {
it('includes remaining suffix in label', () => {
const core = new TimeCore({ type: 'remaining' });
core.setMedia(createMediaState({ currentTime: 90, duration: 300 }));
const state = core.getState();
const attrs = core.getAttrs(state);

expect(attrs['aria-label']).toBe('Remaining');
expect(attrs['aria-valuetext']).toBe('3 minutes, 30 seconds remaining');
expect(attrs['aria-label']).toBe('3 minutes, 30 seconds remaining');
expect(attrs).not.toHaveProperty('aria-valuetext');
});

it('returns button attributes when current time is toggleable', () => {
const core = new TimeCore({ type: 'current', toggle: true });
core.setMedia(createMediaState({ currentTime: 90 }));
const state = core.getState();
const attrs = core.getAttrs(state);

expect(attrs.role).toBe('button');
expect(attrs.tabIndex).toBe(0);
expect(attrs['aria-label']).toBe('1 minute, 30 seconds. Show remaining time.');
expect(attrs).not.toHaveProperty('aria-valuetext');
});

it('returns button attributes when remaining time is toggleable', () => {
const core = new TimeCore({ type: 'remaining', toggle: true });
core.setMedia(createMediaState({ currentTime: 90, duration: 300 }));
const state = core.getState();
const attrs = core.getAttrs(state, 'current');

expect(attrs.role).toBe('button');
expect(attrs.tabIndex).toBe(0);
expect(attrs['aria-label']).toBe('3 minutes, 30 seconds remaining. Show elapsed time.');
expect(attrs).not.toHaveProperty('aria-valuetext');
});

it('returns button attributes when duration is toggleable', () => {
const core = new TimeCore({ type: 'duration', toggle: true });
core.setMedia(createMediaState({ duration: 300 }));
const state = core.getState();
const attrs = core.getAttrs(state);

expect(attrs.role).toBe('button');
expect(attrs.tabIndex).toBe(0);
expect(attrs['aria-label']).toBe('5 minutes. Show remaining time.');
});

it('does not return button attributes without toggle', () => {
const core = new TimeCore({ type: 'duration' });
core.setMedia(createMediaState({ duration: 300 }));
const state = core.getState();
const attrs = core.getAttrs(state);

expect(attrs.role).toBeUndefined();
expect(attrs.tabIndex).toBeUndefined();
});
});
});
36 changes: 27 additions & 9 deletions packages/core/src/core/ui/time/time-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export interface TimeProps {
negativeSign?: string | undefined;
/** Custom label for accessibility. */
label?: string | ((state: TimeState) => string) | undefined;
/** Whether the time display can be toggled. */
toggle?: boolean | undefined;
Comment thread
mihar-22 marked this conversation as resolved.
}

export interface TimeState {
Expand All @@ -32,17 +34,18 @@ export interface TimeState {
datetime: string;
}

const DEFAULT_LABELS: Record<TimeType, string> = {
current: 'Current time',
duration: 'Duration',
remaining: 'Remaining',
const TOGGLE_LABELS: Record<TimeType, string> = {
current: 'Show elapsed time',
duration: 'Show duration',
remaining: 'Show remaining time',
};
Comment on lines +37 to +41

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume this would complicate the i18n work. I think the ARIA attributes are already indicating that this can be pressed/toggled. YT seems to only require the addition of role and tabIndex. Ideally we can remove.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've asked in the shared a11y about the best pattern here. I don't think YouTube has this quite right as there's no indication of what the button actually does, the label only informs of the current state. I could be wrong but waiting on clarification. I think we want the current state and next state to be obvious to screen readers.


export class TimeCore {
static readonly defaultProps: NonNullableObject<TimeProps> = {
type: 'current',
negativeSign: '-',
label: '',
toggle: false,
};

#props = { ...TimeCore.defaultProps };
Expand Down Expand Up @@ -98,7 +101,15 @@ export class TimeCore {
return secondsToIsoDuration(Math.abs(seconds));
}

getLabel(state: TimeState): string {
#getToggleType(type: TimeType, currentType: TimeType): TimeType {
if (type === 'current') {
return currentType === 'remaining' ? 'current' : 'remaining';
}

return currentType === 'duration' ? 'remaining' : 'duration';
}

getLabel(state: TimeState, type = this.#props.type): string {
const { label } = this.#props;

if (isFunction(label)) {
Expand All @@ -108,13 +119,20 @@ export class TimeCore {
return label;
}

return DEFAULT_LABELS[this.#props.type];
if (!this.#props.toggle) {
return state.phrase;
}

const toggleType = this.#getToggleType(type, state.type);

return `${state.phrase}. ${TOGGLE_LABELS[toggleType]}.`;
}

getAttrs(state: TimeState) {
getAttrs(state: TimeState, type = this.#props.type) {
return {
'aria-label': this.getLabel(state),
'aria-valuetext': state.phrase,
'aria-label': this.getLabel(state, type),
role: this.#props.toggle ? 'button' : undefined,
tabIndex: this.#props.toggle ? 0 : undefined,
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/minimal-skin.tailwind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function getTemplateHTML() {

<div class="${time.controls}">
<media-time-group class="${time.group}">
<media-time type="current" class="${time.current}"></media-time>
<media-time toggle type="current" class="${time.current}"></media-time>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I only added it to the minimal skin as the default skin felt a bit better with the remaining in place of the duration. We'll revisit this before GA though I guess as we may want to make minimal a bit more minimal in terms of UI. I'm just waiting for the menus stuff to merge so we're reasonably feature stable for the skins.

<media-time-separator class="${time.separator}"></media-time-separator>
<media-time type="duration" class="${time.duration}"></media-time>
</media-time-group>
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/minimal-skin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function getTemplateHTML() {

<div class="media-time-controls">
<media-time-group class="media-time-group">
<media-time type="current" class="media-time media-time--current"></media-time>
<media-time toggle type="current" class="media-time media-time--current"></media-time>
<media-time-separator class="media-time-separator"></media-time-separator>
<media-time type="duration" class="media-time media-time--duration"></media-time>
</media-time-group>
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/skin.tailwind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function getTemplateHTML() {
<media-slider-value type="pointer" class="${slider.value}"></media-slider-value>
</media-slider-preview>
</media-time-slider>
<media-time type="duration" class="${time.duration}"></media-time>
<media-time toggle type="remaining" class="${time.duration}"></media-time>
</div>

<div class="${buttonGroup}">
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/skin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function getTemplateHTML() {
<media-slider-value type="pointer" class="media-slider__value media-time"></media-slider-value>
</media-slider-preview>
</media-time-slider>
<media-time type="duration" class="media-time"></media-time>
<media-time toggle type="remaining" class="media-time"></media-time>
</div>

<div class="media-button-group">
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/video/minimal-skin.tailwind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ function getTemplateHTML() {

<div class="${time.controls}">
<media-time-group class="${time.group}">
<media-time type="current" class="${time.current}"></media-time>
<media-time toggle type="current" class="${time.current}"></media-time>
<media-time-separator class="${time.separator}"></media-time-separator>
<media-time type="duration" class="${time.duration}"></media-time>
</media-time-group>
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/video/minimal-skin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function getTemplateHTML() {

<div class="media-time-controls">
<media-time-group class="media-time-group">
<media-time type="current" class="media-time media-time--current"></media-time>
<media-time toggle type="current" class="media-time media-time--current"></media-time>
<media-time-separator class="media-time-separator"></media-time-separator>
<media-time type="duration" class="media-time media-time--duration"></media-time>
</media-time-group>
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/video/skin.tailwind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function getTemplateHTML() {
<media-slider-value type="pointer" class="${cn(slider.value, time.current)}"></media-slider-value>
</media-slider-preview>
</media-time-slider>
<media-time type="duration" class="${time.duration}"></media-time>
<media-time toggle type="remaining" class="${time.duration}"></media-time>
</div>

<div class="${cn(buttonGroupEnd, menu.settingsGroup)}">
Expand Down
2 changes: 1 addition & 1 deletion packages/html/src/define/video/skin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function getTemplateHTML() {
<media-slider-value type="pointer" class="media-slider__value media-time"></media-slider-value>
</media-slider-preview>
</media-time-slider>
<media-time type="duration" class="media-time"></media-time>
<media-time toggle type="remaining" class="media-time"></media-time>
</div>

<div class="media-button-group">
Expand Down
Loading
Loading