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

feat(controls): Add media settings control item components #1341

Merged
merged 2 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
@import './styles';

.bp-MediaSettingsMenuBack {
@include bp-MediaSettingsRow;

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

.bp-MediaSettingsMenuBack-label {
color: lighten($blue-steel, 50%);
}
}

.bp-is-focused &:focus {
background-color: $box-blue;

.bp-MediaSettingsMenuBack-arrow {
fill: $white;
}

.bp-MediaSettingsMenuBack-label {
color: $white;
}
}
}

.bp-MediaSettingsMenuBack-arrow {
@include bp-MediaSettingsRow-cell;
}

.bp-MediaSettingsMenuBack-label {
@include bp-MediaSettingsRow-label;

text-align: center;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React from 'react';
import IconArrowLeft24 from '../../icons/IconArrowLeft24';
import MediaSettingsContext, { Menu } from './MediaSettingsContext';
import { decodeKeydown } from '../../../../util';
import './MediaSettingsMenuBack.scss';

export type Props = {
label: string;
};
export type Ref = HTMLDivElement;

function MediaSettingsMenuBack({ label }: Props, ref: React.Ref<Ref>): JSX.Element {
const { setActiveMenu } = React.useContext(MediaSettingsContext);

const handleClick = (): void => {
setActiveMenu(Menu.MAIN);
};

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

if (key !== 'ArrowLeft' && key !== 'Enter' && key !== 'Space') {
return;
}

setActiveMenu(Menu.MAIN);
};

return (
<div
ref={ref}
className="bp-MediaSettingsMenuBack"
onClick={handleClick}
onKeyDown={handleKeydown}
role="menuitem"
tabIndex={0}
>
<div className="bp-MediaSettingsMenuBack-arrow">
<IconArrowLeft24 height={18} width={18} />
</div>
<div aria-label={label} className="bp-MediaSettingsMenuBack-label">
{label}
</div>
</div>
);
}

export default React.forwardRef(MediaSettingsMenuBack);
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
@import './styles';

.bp-MediaSettingsMenuItem {
@include bp-MediaSettingsRow;

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

.bp-MediaSettingsMenuItem-label {
color: lighten($blue-steel, 50%);
}

.bp-MediaSettingsMenuItem-value {
color: $blue-steel;
}
}

.bp-is-focused &:focus {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is bp-is-focused needed? I don't see where that class is applied to the MediaSettingsMenuItem

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a holdover from the current experience in production where focus via keyboard has a different visual treatment than focus via mouse. The class is added by the MediaSettingsControls component.

background-color: $box-blue;

.bp-MediaSettingsMenuItem-arrow {
fill: $white;
}

.bp-MediaSettingsMenuItem-label,
.bp-MediaSettingsMenuItem-value {
color: $white;
}
}
}

.bp-MediaSettingsMenuItem-arrow {
@include bp-MediaSettingsRow-cell;
}

.bp-MediaSettingsMenuItem-label {
@include bp-MediaSettingsRow-label;
}

.bp-MediaSettingsMenuItem-value {
@include bp-MediaSettingsRow-value;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from 'react';
import classNames from 'classnames';
import IconArrowRight24 from '../../icons/IconArrowRight24';
import MediaSettingsContext, { Menu } from './MediaSettingsContext';
import { decodeKeydown } from '../../../../util';
import './MediaSettingsMenuItem.scss';

export type Props = {
className?: string;
label: string;
target: Menu;
value: string;
};
export type Ref = HTMLDivElement;

function MediaSettingsMenuItem(props: Props, ref: React.Ref<Ref>): JSX.Element {
const { className, label, target, value } = props;
const { setActiveMenu } = React.useContext(MediaSettingsContext);

const handleClick = (): void => {
setActiveMenu(target);
};

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

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

setActiveMenu(target);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to worry about stopPropagation for these key events?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, we let them bubble up to be handled by MediaSettingsControls.

};

return (
<div
ref={ref}
aria-haspopup="true"
className={classNames('bp-MediaSettingsMenuItem', className)}
onClick={handleClick}
onKeyDown={handleKeydown}
role="menuitem"
tabIndex={0}
>
<div aria-label={label} className="bp-MediaSettingsMenuItem-label">
{label}
</div>
<div className="bp-MediaSettingsMenuItem-value">{value}</div>
<div className="bp-MediaSettingsMenuItem-arrow">
<IconArrowRight24 height={18} width={18} />
</div>
</div>
);
}

export default React.forwardRef(MediaSettingsMenuItem);
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
@import './styles';

.bp-MediaSettingsRadioItem {
@include bp-MediaSettingsRow;

&.bp-is-selected {
.bp-MediaSettingsRadioItem-check-icon {
visibility: visible;
}

.bp-MediaSettingsRadioItem-value {
color: $box-blue;
}
}

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

.bp-MediaSettingsRadioItem-check-icon {
fill: $blue-steel;
}

.bp-MediaSettingsRadioItem-value {
color: $blue-steel;
}
}

.bp-is-focused &:focus {
background-color: $box-blue;

.bp-MediaSettingsRadioItem-check-icon {
fill: $white;
}

.bp-MediaSettingsRadioItem-value {
color: $white;
}
}
}

.bp-MediaSettingsRadioItem-check {
@include bp-MediaSettingsRow-cell;
}

.bp-MediaSettingsRadioItem-check-icon {
visibility: hidden;
}

.bp-MediaSettingsRadioItem-value {
@include bp-MediaSettingsRow-value;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import React from 'react';
import classNames from 'classnames';
import IconCheckMark24 from '../../icons/IconCheckMark24';
import { decodeKeydown } from '../../../../util';
import './MediaSettingsRadioItem.scss';

export type Props<V extends Value> = {
className?: string;
isSelected?: boolean;
label?: string;
onChange: (value: V) => void;
value: V;
};
export type Ref = HTMLDivElement;
export type Value = boolean | number | string;

function MediaSettingsRadioItem<V extends Value>(props: Props<V>, ref: React.Ref<Ref>): JSX.Element {
const { className, isSelected, label, onChange, value } = props;

const handleClick = (): void => {
onChange(value);
};

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

if (key !== 'ArrowLeft' && key !== 'Enter' && key !== 'Space') {
return;
}

onChange(value);
};

return (
<div
ref={ref}
aria-checked={isSelected ? 'true' : 'false'}
className={classNames('bp-MediaSettingsRadioItem', className, {
'bp-is-selected': isSelected,
})}
onClick={handleClick}
onKeyDown={handleKeydown}
role="menuitemradio"
tabIndex={0}
>
<div className="bp-MediaSettingsRadioItem-check">
<IconCheckMark24 className="bp-MediaSettingsRadioItem-check-icon" height={16} width={16} />
</div>
<div className="bp-MediaSettingsRadioItem-value">{label || value}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of having both label and value for this component?

Copy link
Collaborator Author

@jstoffan jstoffan Mar 15, 2021

Choose a reason for hiding this comment

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

It will be relevant for the Autoplay menu, which shows the value true as "Enabled" to the user. The Rate menu shows most values as their own label, such as 1.5.

</div>
);
}

export default React.forwardRef(MediaSettingsRadioItem) as typeof MediaSettingsRadioItem;
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import MediaSettingsContext, { Context, Menu } from '../MediaSettingsContext';
describe('MediaSettingsContext', () => {
const getContext = (): Context => ({
activeMenu: Menu.MAIN,
activeRect: undefined,
setActiveMenu: jest.fn(),
setActiveRect: jest.fn(),
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from 'react';
import { mount, ReactWrapper } from 'enzyme';
import IconArrowLeft24 from '../../../icons/IconArrowLeft24';
import MediaSettingsContext, { Context } from '../MediaSettingsContext';
import MediaSettingsMenuBack from '../MediaSettingsMenuBack';

describe('MediaSettingsMenuBack', () => {
const getContext = (): Partial<Context> => ({ setActiveMenu: jest.fn() });
const getWrapper = (props = {}, context = {}): ReactWrapper =>
mount(<MediaSettingsMenuBack label="Autoplay" {...props} />, {
wrappingComponent: MediaSettingsContext.Provider,
wrappingComponentProps: { value: context },
});

describe('event handlers', () => {
test('should set the active menu when clicked', () => {
const context = getContext();
const wrapper = getWrapper({}, context);

wrapper.simulate('click');

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

test.each`
key | calledTimes
${'ArrowLeft'} | ${1}
${'Enter'} | ${1}
${'Escape'} | ${0}
${'Space'} | ${1}
`('should set the active menu $calledTimes times when $key is pressed', ({ key, calledTimes }) => {
const context = getContext();
const wrapper = getWrapper({}, context);

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

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

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

expect(wrapper.getDOMNode()).toHaveClass('bp-MediaSettingsMenuBack');
expect(wrapper.find('.bp-MediaSettingsMenuBack-label').contains('Autoplay')).toBe(true);
expect(wrapper.exists(IconArrowLeft24)).toBe(true);
});
});
});
Loading