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

'Esc' works for a vertical volume bar and menus #6046

Merged
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d51fb4c
Merge pull request #1 from videojs/master
gjanblaszczyk Sep 3, 2018
fb16dcf
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Sep 28, 2018
34ca2d2
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Sep 29, 2018
4749478
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Oct 5, 2018
b13ea8b
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Oct 12, 2018
7535983
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Oct 24, 2018
136861f
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Oct 26, 2018
262caec
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Nov 1, 2018
63b46f3
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Nov 6, 2018
2c7a3ea
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Nov 30, 2018
79e63f6
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Dec 6, 2018
7388593
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Dec 13, 2018
64936e4
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Dec 21, 2018
b108dc1
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Jan 12, 2019
5da185f
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Feb 1, 2019
fcef720
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Feb 9, 2019
132ec6e
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Feb 18, 2019
7654528
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Apr 27, 2019
2d8e955
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk May 7, 2019
d892b52
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk May 28, 2019
3da996a
feat: 'Esc' key is supported in volume/mute comp.
gjanblaszczyk Jun 10, 2019
5ad2d80
update package-lock.json file
gjanblaszczyk Jun 10, 2019
af5f6be
removes unused mouseover and mouseout handlers
gjanblaszczyk Jun 12, 2019
075a345
adds support for hiding popup menu via Esc/Tab key
gjanblaszczyk Jun 12, 2019
7480293
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Jun 12, 2019
ba687f3
sync branch and resolves conflicts
gjanblaszczyk Jun 12, 2019
294517b
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Jun 18, 2019
5744b00
Merge branch 'master' into 6004-Esc-works-for-a-vertical-volume-bar
gjanblaszczyk Jun 18, 2019
b4d2102
reverts package-lock.json
gjanblaszczyk Jun 18, 2019
382bc74
Merge remote-tracking branch 'upstream/master'
gjanblaszczyk Jun 19, 2019
7036277
Merge branch 'master' into 6004-Esc-works-for-a-vertical-volume-bar
gjanblaszczyk Jun 19, 2019
73bf063
adds dispose methods
gjanblaszczyk Aug 2, 2019
2f9c46e
Merge branch 'master' into 6004-Esc-works-for-a-vertical-volume-bar
gjanblaszczyk Aug 8, 2019
5104fed
improves handlekeyPress handler and fixes comment for this method.
gjanblaszczyk Aug 19, 2019
51c7da0
improves doc comment for the `handleKeyPress` method.
gjanblaszczyk Aug 19, 2019
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
13 changes: 6 additions & 7 deletions src/css/components/_volume.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,33 @@
width: 1px;
height: 1px;
margin-left: -1px;

}

.video-js .vjs-volume-panel {
&:hover .vjs-volume-control,
&.vjs-hover .vjs-volume-control,
&:active .vjs-volume-control,
&:focus .vjs-volume-control,
& .vjs-volume-control:hover ,
& .vjs-volume-control:active ,
& .vjs-mute-control:hover ~ .vjs-volume-control,
& .vjs-volume-control:active,
&.vjs-hover .vjs-mute-control ~ .vjs-volume-control,
& .vjs-volume-control.vjs-slider-active {
&.vjs-volume-horizontal {
width: 5em;
height: 3em;
}

visibility: visible;
opacity: 1;
position: relative;

&.vjs-volume-vertical {
left: -3.5em;
@include transition(left 0s);
}
$transition-property: visibility 0.1s, opacity 0.1s, height 0.1s, width 0.1s, left 0s, top 0s;
@include transition($transition-property);
}

&.vjs-volume-panel-horizontal {
&:hover,
&.vjs-hover,
&:active,
&.vjs-slider-active {
width: 9em;
Expand All @@ -83,6 +81,7 @@
$transition-property: visibility 1s, opacity 1s, height 1s 1s, width 1s 1s, left 1s 1s, top 1s 1s;
@include transition($transition-property)
}

.video-js .vjs-volume-panel .vjs-volume-control.vjs-volume-horizontal {
$transition-property: visibility 1s, opacity 1s, height 1s 1s, width 1s, left 1s 1s, top 1s 1s;
@include transition($transition-property)
Expand Down
2 changes: 1 addition & 1 deletion src/css/components/menu/_menu-popup.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
max-height: 25em;
}

.vjs-workinghover .vjs-menu-button-popup:hover .vjs-menu,
.vjs-workinghover .vjs-menu-button-popup.vjs-hover .vjs-menu,
.vjs-menu-button-popup .vjs-menu.vjs-lock-showing {
display: block;
}
2 changes: 2 additions & 0 deletions src/js/clickable-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ class ClickableComponent extends Component {
if (typeof this.tabIndex_ !== 'undefined') {
this.el_.removeAttribute('tabIndex');
}
this.off('mouseover', this.handleMouseOver);
this.off('mouseout', this.handleMouseOut);
this.off(['tap', 'click'], this.handleClick);
this.off('keydown', this.handleKeyDown);
}
Expand Down
76 changes: 76 additions & 0 deletions src/js/control-bar/volume-panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
*/
import Component from '../component.js';
import {isPlain} from '../utils/obj';
import * as Events from '../utils/events.js';
import * as Fn from '../utils/fn.js';
import keycode from 'keycode';
import document from 'global/document';

// Required children
import './volume-control/volume-control.js';
Expand Down Expand Up @@ -42,6 +46,11 @@ class VolumePanel extends Component {
super(player, options);

this.on(player, ['loadstart'], this.volumePanelState_);
this.on(this.muteToggle, 'keyup', this.handleKeyPress);
this.on(this.volumeControl, 'keyup', this.handleVolumeControlKeyUp);
this.on('keydown', this.handleKeyPress);
this.on('mouseover', this.handleMouseOver);
this.on('mouseout', this.handleMouseOut);

// while the slider is active (the mouse has been pressed down and
// is dragging) we do not want to hide the VolumeBar
Expand Down Expand Up @@ -109,6 +118,73 @@ class VolumePanel extends Component {
});
}

/**
* Dispose of the `volume-panel` and all child components.
*/
dispose() {
this.handleMouseOut();
super.dispose();
}

/**
* Handles `keyup` events on the `VolumeControl`, looking for ESC, which closes
* the volume panel and sets focus on `MuteToggle`.
*
* @param {EventTarget~Event} event
* The `keyup` event that caused this function to be called.
*
* @listens keyup
*/
handleVolumeControlKeyUp(event) {
if (keycode.isEventKey(event, 'Esc')) {
this.muteToggle.focus();
}
}

/**
* This gets called when a `VolumePanel` gains hover via a `mouseover` event.
* Turns on listening for `mouseover` event. When they happen it
* calls `this.handleMouseOver`.
*
* @param {EventTarget~Event} event
* The `mouseover` event that caused this function to be called.
*
* @listens mouseover
*/
handleMouseOver(event) {
this.addClass('vjs-hover');
Events.on(document, 'keyup', Fn.bind(this, this.handleKeyPress));
}

/**
* This gets called when a `VolumePanel` gains hover via a `mouseout` event.
* Turns on listening for `mouseout` event. When they happen it
* calls `this.handleMouseOut`.
*
* @param {EventTarget~Event} event
* The `mouseout` event that caused this function to be called.
*
* @listens mouseout
*/
handleMouseOut(event) {
this.removeClass('vjs-hover');
Events.off(document, 'keyup', Fn.bind(this, this.handleKeyPress));
gkatsev marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Handles `keydown|keyup` events on the document, looking for ESC, which closes
* the volume panel.
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a copy/paste issue here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, nice catch! I have just fixed the comment.

*
* @param {EventTarget~Event} event
* The keypress that triggered this event.
*
* @listens keydown | keyup
*/
handleKeyPress(event) {
if (keycode.isEventKey(event, 'Esc')) {
this.removeClass('vjs-hover');
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need to remove the document-level listener here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I have just improved the method following your comment.

}
}
}

/**
Expand Down
44 changes: 43 additions & 1 deletion src/js/menu/menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import Button from '../button.js';
import Component from '../component.js';
import Menu from './menu.js';
import * as Dom from '../utils/dom.js';
import * as Fn from '../utils/fn.js';
import * as Events from '../utils/events.js';
import {toTitleCase} from '../utils/string-cases.js';
import { IS_IOS } from '../utils/browser.js';
import document from 'global/document';
import keycode from 'keycode';

/**
Expand Down Expand Up @@ -49,9 +52,11 @@ class MenuButton extends Component {
this.on(this.menuButton_, 'click', this.handleClick);
this.on(this.menuButton_, 'keydown', this.handleKeyDown);
this.on(this.menuButton_, 'mouseenter', () => {
this.addClass('vjs-hover');
this.menu.show();
Events.on(document, 'keyup', Fn.bind(this, this.handleMenuKeyUp));
gkatsev marked this conversation as resolved.
Show resolved Hide resolved
});

this.on('mouseleave', this.handleMouseLeave);
gkatsev marked this conversation as resolved.
Show resolved Hide resolved
this.on('keydown', this.handleSubmenuKeyDown);
}

Expand Down Expand Up @@ -210,6 +215,14 @@ class MenuButton extends Component {
return this.menuButton_.controlText(text, el);
}

/**
* Dispose of the `menu-button` and all child components.
*/
dispose() {
this.handleMouseLeave();
super.dispose();
}

/**
* Handle a click on a `MenuButton`.
* See {@link ClickableComponent#handleClick} for instances where this is called.
Expand All @@ -229,6 +242,19 @@ class MenuButton extends Component {
}
}

/**
* Handle `mouseleave` for `MenuButton`.
*
* @param {EventTarget~Event} event
* The `mouseleave` event that caused this function to be called.
*
* @listens mouseleave
*/
handleMouseLeave(event) {
this.removeClass('vjs-hover');
Events.off(document, 'keyup', Fn.bind(this, this.handleMenuKeyUp));
}

/**
* Set the focus to the actual button, not to this element
*/
Expand Down Expand Up @@ -275,6 +301,22 @@ class MenuButton extends Component {
}
}

/**
* Handle a `keyup` event on a `MenuButton`. The listener for this is added in
* the constructor.
*
* @param {EventTarget~Event} event
* Key press event
*
* @listens keyup
*/
handleMenuKeyUp(event) {
// Escape hides popup menu
if (keycode.isEventKey(event, 'Esc') || keycode.isEventKey(event, 'Tab')) {
this.removeClass('vjs-hover');
}
}

/**
* This method name now delegates to `handleSubmenuKeyDown`. This means
* anyone calling `handleSubmenuKeyPress` will not see their method calls
Expand Down