-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Fix keyboard (Space or Enter) operation of buttons. Fixes #2888. Supercedes #2889 #3032
Closed
OwenEdwards
wants to merge
2
commits into
videojs:master
from
OwenEdwards:fix/split-clickables-from-buttons
Closed
Fix keyboard (Space or Enter) operation of buttons. Fixes #2888. Supercedes #2889 #3032
OwenEdwards
wants to merge
2
commits into
videojs:master
from
OwenEdwards:fix/split-clickables-from-buttons
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LGTM |
event.preventDefault(); | ||
this.handleClick(event); | ||
} else { | ||
super.handleKeyPress(event); // Pass keypress handling up for unsupported keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ends up trying to call Component
's handleKeyPress
, which doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want the else to be:
} else if (super.handleKeyPress) {
super.handleKeyPress(event);
}
c0d540c
to
9768e02
Compare
jgubman
added a commit
to jgubman/video.js
that referenced
this pull request
Jan 27, 2016
* 'master' of https://github.com/videojs/video.js: (38 commits) v5.6.0 @gkatsev updated to latest videojs-ie8 shim. closes videojs#3042 @hubdotcom changed URLs in README to be protocol-relative. closes videojs#3040 @CoWinkKeyDinkInc fixed table in Tracks guide. Replaced some single quotes with double quotes. closes videojs#2946 @aril-spetalen added language support for Norwegian (nb and nn). closes videojs#3021 @vitor-faiante updated the guides. closes videojs#2781 @gkatsev checked muted status when updating volume bar level. closes videojs#3037 @OwenEdwards fixed double-localization of mute toggle control text. closes videojs#3017 @mister-ben made $primary-foreground-color a !default sass var. closes videojs#3003 @OwenEdwards Fixed volume menu keyboard access. closes videojs#3034 @OwenEdwards Fixed menu keyboard access and ARIA labeling for screen readers. closes videojs#3033 @OwenEdwards added ClickableComponent. Fixed keyboard operation of buttons. closes videojs#3032 v5.5.3 @rcrooks fixed a couple of docs link and a jsdoc comment. closes videojs#2987 @mister-ben updated CDN urls in setup guide. closes videojs#2984 @gkasev updated vjs to correctly return already created player when given an element. closes videojs#3006 v5.5.2 make sure that styleEl_ is in DOM before removing on dispose. closes videojs#3004 v5.5.1 @gkatsev fixed sass if else for icons. closes videojs#2988 ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Better fix than #2889 for the problem with keyboard operation of buttons which was introduced by changing buttons to
<button>
elements, but leaving other clickable elements (menu buttons, poster image) as<div>
s. By splitting them in different objects, this allows future changes to build on these two distinct bases.