Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keyboard support to the new SettingsPanel design #684

Merged
8 changes: 6 additions & 2 deletions src/scss/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,13 @@
width: $marker-dimension;
}

@mixin focusable {
@mixin focusable($inset: false) {
&:focus {
box-shadow: $focus-element-box-shadow;
@if $inset == false {
box-shadow: $focus-element-box-shadow;
} @else {
box-shadow: inset $focus-element-box-shadow;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The box shadow has to be inset for settings panel items as otherwise it is not visible (they are at the edge of the settings panel and it has the overflow hidden)

}
outline: none;

&:not(.#{$prefix}-focus-visible) {
Expand Down
2 changes: 2 additions & 0 deletions src/scss/components/settings/_settings-panel-item.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
@import '../../variables';

%ui-settings-panel-item {
@include focusable(true);

line-height: 1.5em;
padding: .5em .7em;
white-space: nowrap;
Expand Down
1 change: 0 additions & 1 deletion src/ts/UIFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ function uiLayout(config: UIConfig) {
opener: subtitleSettingsOpenButton,
}),
settingComponent: subtitleSelectBox,
role: 'menubar',
container: settingsPanel,
});
mainSettingsPanelPage.addComponent(subtitleSelectItem);
Expand Down
28 changes: 28 additions & 0 deletions src/ts/components/Component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ export interface ViewModeChangedEventArgs extends NoArgs {
mode: ViewMode;
}

export interface ComponentFocusChangedEventArgs extends NoArgs {
/**
* True is the component is focused, else false.
*/
focused: boolean;
}

/**
* The base class of the UI framework.
* Each component must extend this class and optionally the config interface.
Expand Down Expand Up @@ -208,6 +215,7 @@ export class Component<Config extends ComponentConfig> {
onHoverChanged: new EventDispatcher<Component<Config>, ComponentHoverChangedEventArgs>(),
onEnabled: new EventDispatcher<Component<Config>, NoArgs>(),
onDisabled: new EventDispatcher<Component<Config>, NoArgs>(),
onFocusChanged: new EventDispatcher<Component<Config>, ComponentFocusChangedEventArgs>(),
};

/**
Expand Down Expand Up @@ -273,6 +281,10 @@ export class Component<Config extends ComponentConfig> {
// Track the hovered state of the element
this.getDomElement().on('mouseenter', () => this.onHoverChangedEvent(true));
this.getDomElement().on('mouseleave', () => this.onHoverChangedEvent(false));

// Track the focused state of the element
this.getDomElement().on('focusin', () => this.onFocusChangedEvent(true));
this.getDomElement().on('focusout', () => this.onFocusChangedEvent(false));
}

/**
Expand All @@ -298,6 +310,10 @@ export class Component<Config extends ComponentConfig> {
'role': this.config.role,
}, this);

if (typeof this.config.tabIndex === 'number') {
element.attr('tabindex', this.config.tabIndex.toString());
}

return element;
}

Expand Down Expand Up @@ -531,6 +547,10 @@ export class Component<Config extends ComponentConfig> {
this.componentEvents.onHoverChanged.dispatch(this, { hovered: hovered });
}

protected onFocusChangedEvent(focused: boolean): void {
this.componentEvents.onFocusChanged.dispatch(this, { focused: focused });
}

/**
* Gets the event that is fired when the component is showing.
* See the detailed explanation on event architecture on the {@link #componentEvents events list}.
Expand Down Expand Up @@ -582,4 +602,12 @@ export class Component<Config extends ComponentConfig> {
get onViewModeChanged(): Event<Component<Config>, ViewModeChangedEventArgs> {
return this.componentEvents.onViewModeChanged.getEvent();
}

/**
* Gets the event that is fired when the component's focus-state is changing.
* @returns {Event<Component<Config>, ComponentFocusChangedEventArgs>}
*/
get onFocusedChanged(): Event<Component<Config>, ComponentFocusChangedEventArgs> {
return this.componentEvents.onFocusChanged.getEvent();
}
}
4 changes: 4 additions & 0 deletions src/ts/components/Container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ export class Container<Config extends ContainerConfig> extends Component<Config>
'aria-label': i18n.performLocalization(this.config.ariaLabel),
}, this);

if (typeof this.config.tabIndex === 'number') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The ComponentConfig already has the tabIndex property but it's not consistently applied when creating the dom elements.

containerElement.attr('tabindex', this.config.tabIndex.toString());
}

// Create the inner container element (the inner <div>) that will contain the components
let innerContainer = new DOM(this.config.tag, {
'class': this.prefixCss('container-wrapper'),
Expand Down
4 changes: 4 additions & 0 deletions src/ts/components/buttons/Button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ export class Button<Config extends ButtonConfig> extends Component<Config> {
this.onClickEvent();
});

buttonElement.on('focusin focusout', (e) => {
e.stopPropagation();
});
Comment on lines +80 to +82
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is needed to stop bubbling of the focusin event to parents. This is a problem in the new settings panel where the subtitle settings button is inside a focusable settings panel item. Without that the focusin fires on both elements which is undesired.


return buttonElement;
}

Expand Down
13 changes: 5 additions & 8 deletions src/ts/components/settings/DynamicSettingsPanelItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { SubtitleSettingsLabel } from './subtitlesettings/SubtitleSettingsLabel'
import { SettingsPanel } from './SettingsPanel';
import { SettingsPanelPageBackButton } from './SettingsPanelPageBackButton';
import { SubtitleSettingSelectBox } from './subtitlesettings/SubtitleSettingSelectBox';
import { InteractiveSettingsPanelItem } from './InteractiveSettingsPanelItem';

/**
* Configuration interface for a {@link DynamicSettingsPanelItem}.
Expand All @@ -35,10 +36,10 @@ export interface DynamicSettingsPanelItemConfig extends SettingsPanelItemConfig
/**
* A dynamic settings panel item which can build a sub page with the items of a {@link ListSelector}.
* The page will be dynamically added and removed from the {@link SettingsPanel}.
*
*
* @category Components
*/
export class DynamicSettingsPanelItem extends SettingsPanelItem<DynamicSettingsPanelItemConfig> {
export class DynamicSettingsPanelItem extends InteractiveSettingsPanelItem<DynamicSettingsPanelItemConfig> {
private selectedOptionLabel: Label<LabelConfig>;
protected settingComponent: ListSelector<ListSelectorConfig>;

Expand All @@ -63,6 +64,7 @@ export class DynamicSettingsPanelItem extends SettingsPanelItem<DynamicSettingsP
cssClass: 'ui-settings-panel-item',
role: 'menuitem',
addSettingAsComponent: false,
tabIndex: 0,
}, this.config);
}

Expand Down Expand Up @@ -99,13 +101,8 @@ export class DynamicSettingsPanelItem extends SettingsPanelItem<DynamicSettingsP

handleSelectedItemChanged();

const handleItemClick = () => {
this.onClick.subscribe(() => {
this.displayItemsSubPage();
};
this.getDomElement().on('click', (e) => {
e.preventDefault();
e.stopPropagation();
handleItemClick();
});
}

Expand Down
65 changes: 65 additions & 0 deletions src/ts/components/settings/InteractiveSettingsPanelItem.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { SettingsPanelItem, SettingsPanelItemConfig } from './SettingsPanelItem';
import { Event, EventDispatcher, NoArgs } from '../../EventDispatcher';
import { PlayerAPI } from 'bitmovin-player';
import { UIInstanceManager } from '../../UIManager';
import { getKeyMapForPlatform } from '../../spatialnavigation/getKeyMapForPlatform';
import { Action } from '../../spatialnavigation/types';

/**
* A settings panel item that can be interacted with using the keyboard or mouse.
* Can be used when no interactive element is present as child item.
*/
export class InteractiveSettingsPanelItem<Config extends SettingsPanelItemConfig> extends SettingsPanelItem<Config> {
private events = {
onClick: new EventDispatcher<InteractiveSettingsPanelItem<Config>, NoArgs>(),
};

constructor(config: Config) {
super(config);
}

configure(player: PlayerAPI, uimanager: UIInstanceManager) {
super.configure(player, uimanager);

const handleClickEvent = (event: UIEvent) => {
event.preventDefault();
event.stopPropagation();
this.onClickEvent();
};

this.getDomElement().on('click touchend', handleClickEvent);

// Listen to keyboard events and trigger the click event when a select key is detected
const handleKeyDown = (event: KeyboardEvent) => {
const action = getKeyMapForPlatform()[event.keyCode];
const acceptedKeys = ['Enter', ' '];
const acceptedCodes = ['Enter', 'Space'];

if (action === Action.SELECT || acceptedKeys.includes(event.key) || acceptedCodes.includes(event.code)) {
handleClickEvent(event);
}
};

this.onFocusedChanged.subscribe((_, args) => {
if (args.focused) {
// Only listen to keyboard events when the element is focused
this.getDomElement().on('keydown', handleKeyDown);
} else {
// Unregister the keyboard event listener when the element loses focus
this.getDomElement().off('keydown', handleKeyDown);
}
});
}

protected onClickEvent() {
this.events.onClick.dispatch(this);
}

/**
* Gets the event that is fired when the SettingsPanelItem is clicked.
* @returns {Event<InteractiveSettingsPanelItem<Config>, NoArgs>}
*/
get onClick(): Event<InteractiveSettingsPanelItem<Config>, NoArgs> {
return this.events.onClick.getEvent();
}
}
7 changes: 6 additions & 1 deletion src/ts/components/settings/SettingsPanelPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {UIInstanceManager} from '../../UIManager';
import {Event, EventDispatcher, NoArgs} from '../../EventDispatcher';
import { PlayerAPI } from 'bitmovin-player';
import { BrowserUtils } from '../../utils/BrowserUtils';
import { InteractiveSettingsPanelItem } from './InteractiveSettingsPanelItem';

/**
* Configuration interface for a {@link SettingsPanelPage}
Expand Down Expand Up @@ -94,7 +95,11 @@ export class SettingsPanelPage extends Container<SettingsPanelPageConfig> {
this.settingsPanelPageEvents.onActive.dispatch(this);
// Disable focus for iOS and iPadOS 13. They open select boxes automatically on focus and we want to avoid that.
if (activeItems.length > 0 && !BrowserUtils.isIOS && !(BrowserUtils.isMacIntel && BrowserUtils.isTouchSupported)) {
activeItems[0].getDomElement().focusToFirstInput();
if (activeItems[0] instanceof InteractiveSettingsPanelItem) {
activeItems[0].getDomElement().get(0).focus();
} else {
activeItems[0].getDomElement().focusToFirstInput();
}
}
}

Expand Down
13 changes: 5 additions & 8 deletions src/ts/components/settings/SettingsPanelSelectOption.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { SettingsPanelItem, SettingsPanelItemConfig } from './SettingsPanelItem';
import { SettingsPanelItemConfig } from './SettingsPanelItem';
import { PlayerAPI } from 'bitmovin-player';
import { UIInstanceManager } from '../../UIManager';
import { ListSelector, ListSelectorConfig } from '../lists/ListSelector';
import { InteractiveSettingsPanelItem } from './InteractiveSettingsPanelItem';

/**
* Configuration interface for a {@link SettingsPanelSelectOption}.
Expand All @@ -25,7 +26,7 @@ export interface SettingsPanelSelectOptionConfig extends SettingsPanelItemConfig
*
* @category Components
*/
export class SettingsPanelSelectOption extends SettingsPanelItem<SettingsPanelSelectOptionConfig> {
export class SettingsPanelSelectOption extends InteractiveSettingsPanelItem<SettingsPanelSelectOptionConfig> {
private settingsValue: string | undefined;
protected settingComponent: ListSelector<ListSelectorConfig>;

Expand All @@ -37,6 +38,7 @@ export class SettingsPanelSelectOption extends SettingsPanelItem<SettingsPanelSe
this.config = this.mergeConfig(config, {
cssClasses: ['ui-settings-panel-item-select-option'],
role: 'menuitem',
tabIndex: 0,
} as SettingsPanelSelectOptionConfig, this.config);
}

Expand All @@ -54,13 +56,8 @@ export class SettingsPanelSelectOption extends SettingsPanelItem<SettingsPanelSe
};
this.settingComponent.onItemSelected.subscribe(handleSelectedOptionChanged);

const handleItemClick = () => {
this.onClick.subscribe(() => {
this.settingComponent.selectItem(this.settingsValue);
};
this.getDomElement().on('click', (e) => {
e.preventDefault();
e.stopPropagation();
handleItemClick();
});

// Initial state
Expand Down
Loading