Skip to content

Commit

Permalink
fix(dash): Disable Settings menu when HD is not available
Browse files Browse the repository at this point in the history
  • Loading branch information
Conrad Chan committed Jun 25, 2021
1 parent 68f8502 commit f702e61
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 10 deletions.
4 changes: 3 additions & 1 deletion src/lib/viewers/controls/media/MediaSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ export type Props = Partial<AudioTracksProps> &
Partial<SettingsProps> &
Partial<SubtitlesProps> &
AutoplayProps &
RateProps & { className?: string };
RateProps & { className?: string; isHDSupported?: boolean };

export default function MediaSettings({
audioTrack,
audioTracks = [],
autoplay,
badge,
className,
isHDSupported,
onAudioTrackChange = noop,
onAutoplayChange,
onQualityChange,
Expand Down Expand Up @@ -61,6 +62,7 @@ export default function MediaSettings({
{quality && (
<Settings.MenuItem
data-testid="bp-media-settings-quality"
isDisabled={!isHDSupported}
label={__('media_quality')}
target={Menu.QUALITY}
value={getQualityLabel(quality)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ describe('MediaSettings', () => {
const wrapper = getWrapper({ quality: 'auto', onQualityChange: jest.fn() });
expect(wrapper.exists(MediaSettingsMenuQuality)).toBe(true);
});

test('should render with isDisabled based on isHDSupported prop', () => {
const wrapper = getWrapper({ isHDSupported: false, quality: 'auto', onQualityChange: jest.fn() });
expect(wrapper.find({ target: 'quality' }).prop('isDisabled')).toBe(true);
});
});

describe('subtitles menu', () => {
Expand Down
4 changes: 4 additions & 0 deletions src/lib/viewers/controls/settings/SettingsMenuItem.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
.bp-SettingsMenuItem {
@include bp-SettingsRow;

&[aria-disabled='true'] {
cursor: default;
}

&:hover {
background-color: $hover-blue-background;

Expand Down
12 changes: 9 additions & 3 deletions src/lib/viewers/controls/settings/SettingsMenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,28 @@ import './SettingsMenuItem.scss';
export type Props = React.RefAttributes<HTMLDivElement> & {
className?: string;
label: string;
isDisabled?: boolean;
target: Menu;
value: string;
};
export type Ref = HTMLDivElement;

function SettingsMenuItem(props: Props, ref: React.Ref<Ref>): JSX.Element {
const { className, label, target, value, ...rest } = props;
const { className, label, isDisabled = false, target, value, ...rest } = props;
const { setActiveMenu } = React.useContext(SettingsContext);

const handleClick = (): void => {
if (isDisabled) {
return;
}

setActiveMenu(target);
};

const handleKeydown = (event: React.KeyboardEvent<Ref>): void => {
const key = decodeKeydown(event);

if (key !== 'ArrowRight' && key !== 'Enter' && key !== 'Space') {
if (isDisabled || (key !== 'ArrowRight' && key !== 'Enter' && key !== 'Space')) {
return;
}

Expand All @@ -34,6 +39,7 @@ function SettingsMenuItem(props: Props, ref: React.Ref<Ref>): JSX.Element {
return (
<div
ref={ref}
aria-disabled={isDisabled}
aria-haspopup="true"
className={classNames('bp-SettingsMenuItem', className)}
onClick={handleClick}
Expand All @@ -47,7 +53,7 @@ function SettingsMenuItem(props: Props, ref: React.Ref<Ref>): JSX.Element {
</div>
<div className="bp-SettingsMenuItem-value">{value}</div>
<div className="bp-SettingsMenuItem-arrow">
<IconArrowRight24 height={18} width={18} />
{!isDisabled && <IconArrowRight24 height={18} width={18} />}
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,47 @@ describe('SettingsMenuItem', () => {

expect(context.setActiveMenu).toBeCalledTimes(calledTimes);
});

test('should not set the active menu when clicked while disabled', () => {
const context = getContext();
const wrapper = getWrapper({ isDisabled: true }, context);

wrapper.simulate('click');

expect(context.setActiveMenu).not.toBeCalled();
});

test.each`
key
${'ArrowRight'}
${'Enter'}
${'Space'}
`('should not set the active menu when $key is pressed while disabled', ({ key }) => {
const context = getContext();
const wrapper = getWrapper({ isDisabled: true }, context);

wrapper.simulate('keydown', { key });

expect(context.setActiveMenu).not.toBeCalled();
});
});

describe('render', () => {
test('should return a valid wrapper', () => {
const wrapper = getWrapper();

expect(wrapper.getDOMNode()).toHaveClass('bp-SettingsMenuItem');
expect(wrapper.getDOMNode().getAttribute('aria-disabled')).toBe('false');
expect(wrapper.find('.bp-SettingsMenuItem-label').contains('Speed')).toBe(true);
expect(wrapper.find('.bp-SettingsMenuItem-value').contains('Normal')).toBe(true);
expect(wrapper.exists(IconArrowRight24)).toBe(true);
});

test('should render as disabled when isDisabled is true', () => {
const wrapper = getWrapper({ isDisabled: true });

expect(wrapper.getDOMNode().getAttribute('aria-disabled')).toBe('true');
expect(wrapper.exists(IconArrowRight24)).toBe(false);
});
});
});
2 changes: 2 additions & 0 deletions src/lib/viewers/media/DashControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default function DashControls({
currentTime,
durationTime,
isAutoGeneratedSubtitles,
isHDSupported,
isPlaying,
isPlayingHD,
onAudioTrackChange,
Expand Down Expand Up @@ -74,6 +75,7 @@ export default function DashControls({
autoplay={autoplay}
badge={isPlayingHD ? <HDBadge /> : undefined}
className="bp-DashControls-settings"
isHDSupported={isHDSupported}
onAudioTrackChange={onAudioTrackChange}
onAutoplayChange={onAutoplayChange}
onQualityChange={onQualityChange}
Expand Down
8 changes: 5 additions & 3 deletions src/lib/viewers/media/DashViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ class DashViewer extends VideoBaseViewer {

const isHDSupported = this.hdVideoId !== -1;
this.selectedQuality = isHDSupported ? this.cache.get('media-quality') || 'auto' : 'sd';
this.setQuality(this.selectedQuality);
this.setQuality(this.selectedQuality, false);
}

/**
Expand Down Expand Up @@ -1045,12 +1045,13 @@ class DashViewer extends VideoBaseViewer {
/**
* Updates the selected quality and updates the player accordingly
* @param {string} quality - 'sd', 'hd', or 'auto'
* @param {boolean} [saveToCache] - Whether to save this value to the cache, defaults to true
* @emits qualitychange
* @return {void}
*/
setQuality(quality) {
setQuality(quality, saveToCache = true) {
const newQuality = quality !== 'sd' && quality !== 'hd' ? 'auto' : quality;
this.cache.set('media-quality', newQuality, true);
this.cache.set('media-quality', newQuality, saveToCache);
this.selectedQuality = newQuality;

switch (newQuality) {
Expand Down Expand Up @@ -1147,6 +1148,7 @@ class DashViewer extends VideoBaseViewer {
currentTime={this.mediaEl.currentTime}
durationTime={this.mediaEl.duration}
isAutoGeneratedSubtitles={!!this.autoCaptionDisplayer}
isHDSupported={this.hdVideoId !== -1}
isPlaying={!this.mediaEl.paused}
isPlayingHD={this.isPlayingHD()}
onAudioTrackChange={this.setAudioTrack}
Expand Down
5 changes: 5 additions & 0 deletions src/lib/viewers/media/__tests__/DashControls-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ describe('DashControls', () => {
expect(wrapper.find(VolumeControls).prop('onVolumeChange')).toEqual(onVolumeChange);
});

test.each([true, false])('should set isHDSupported prop on MediaSettings as %s', isHDSupported => {
const wrapper = getWrapper({ isHDSupported });
expect(wrapper.find(MediaSettings).prop('isHDSupported')).toBe(isHDSupported);
});

test('should not pass along badge if not playing HD', () => {
const wrapper = getWrapper({ badge: <CustomBadge /> });
expect(wrapper.find(MediaSettings).prop('badge')).toBeUndefined();
Expand Down
11 changes: 8 additions & 3 deletions src/lib/viewers/media/__tests__/DashViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -752,14 +752,14 @@ describe('lib/viewers/media/DashViewer', () => {
dash.loadUIReact();

expect(dash.selectedQuality).toBe('sd');
expect(dash.setQuality).toBeCalledWith('sd');
expect(dash.setQuality).toBeCalledWith('sd', false);
});

test('should set quality to auto if HD is supported and cache has no entry', () => {
dash.loadUIReact();

expect(dash.selectedQuality).toBe('auto');
expect(dash.setQuality).toBeCalledWith('auto');
expect(dash.setQuality).toBeCalledWith('auto', false);
});

test('should set quality to cache value if HD is supported and cache has an entry', () => {
Expand All @@ -768,7 +768,7 @@ describe('lib/viewers/media/DashViewer', () => {
dash.loadUIReact();

expect(dash.selectedQuality).toBe('hd');
expect(dash.setQuality).toBeCalledWith('hd');
expect(dash.setQuality).toBeCalledWith('hd', false);
});
});

Expand Down Expand Up @@ -1638,6 +1638,11 @@ describe('lib/viewers/media/DashViewer', () => {
expect(dash.emit).toBeCalledWith('qualitychange', 'auto');
expect(dash.renderUI).toBeCalled();
});

test('should not save to cache if saveToCache is false', () => {
dash.setQuality('auto', false);
expect(dash.cache.set).toBeCalledWith('media-quality', 'auto', false);
});
});

describe('initSubtitles()', () => {
Expand Down

0 comments on commit f702e61

Please sign in to comment.