Closed
Bug 892815
Opened 11 years ago
Closed 10 years ago
[meta] Keyboard Interactions Plugin
Categories
(Webmaker Graveyard :: Popcorn Maker, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mjschranz, Assigned: sedge)
Details
(Whiteboard: fivel-bugs)
Attachments
(1 file, 2 obsolete files)
Technical requirements notes: • This plugin is an extended pause plugin that works identically to the pause plugin with the exception that you can select a keyboard key press combination to advance through the pause and resume the video • Example: pause and wait for CTRL+C to be pressed. The video may say something like “now you try it” and pause and wait for the user to press the key event • The plugin should also allow for a specific image to optionally be displayed if it is provided so we can create a poster slide that displays for the user to see (CSS class applied to the div would allow us to set an image as the background) • The plugin should also allow for specific text to be displayed over the image that we can display to the user. This text may need to be localized in the future and account for support for that. • This plugin should be supported in IE8
Assignee | ||
Comment 1•11 years ago
|
||
WIP patch
Assignee | ||
Updated•11 years ago
|
Summary: Keyboard Interactions Plugin → [meta] Keyboard Interactions Plugin
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #799070 -
Flags: review?(schranz.m)
Reporter | ||
Updated•11 years ago
|
Attachment #775858 -
Attachment is obsolete: true
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 Comments. I actually can't run this right now.
Attachment #799070 -
Flags: review?(schranz.m) → review-
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 This is complete relative to my reasonable attempts to fight against technical complications that prevent full functionality. This needs testing and thorough code review for style and efficiency would be appreciated!
Attachment #799070 -
Flags: review- → review?(schranz.m)
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 Matt knows what to look for in testing, but both feel free to do code review as well - this is likely to be pretty dusty.
Attachment #799070 -
Flags: review?(scott)
Comment 6•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 Some initial line comments, but it seems that popcorn.interaction.less never made it into the commit, so I don't see a keyboard.
Attachment #799070 -
Flags: review?(scott) → review-
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 I added the LESS file in this time ;)
Attachment #799070 -
Flags: review- → review?(scott)
Comment 8•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 Hm, having it on is blocking normal popcorn maker functionality too. This is very confusing. We need to come up with a better idea of whe it goes into input mode. Right now it just goes into input mode on start, but that's too agressive. Also, still some nits in the pull request.
Attachment #799070 -
Flags: review?(scott) → review-
Reporter | ||
Comment 9•11 years ago
|
||
Comment on attachment 799070 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/192 Browser defaults still seem to get through. For example, I register meta+c as a key combo. I tried to meta+t but that opened a new tab rather than blocking it. If I change the key combo in the editor while it's active with a keycombo already the keyboard slides away.
Attachment #799070 -
Flags: review?(schranz.m) → review-
Assignee | ||
Comment 10•11 years ago
|
||
This patch isn't rebased from master because it's going into Fivel's repo.
Attachment #799070 -
Attachment is obsolete: true
Attachment #831531 -
Flags: review?(schranz.m)
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 831531 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/315 I'd really value a code review from you Humph, especially since you've worked with keyboard events before.
Attachment #831531 -
Flags: review?(david.humphrey)
Assignee | ||
Comment 12•11 years ago
|
||
:humph this part specifically: https://github.com/sedge/popcorn.webmaker.org/blob/24e973f6b675cece4f8415dd146c3ff3582832ed/public/templates/assets/plugins/interaction/popcorn.interaction.js#L12-297
Comment 13•11 years ago
|
||
Comment on attachment 831531 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/315 This was a big job, now that I read the code. You really had to deal with a lot of stuff. Nice work on it. Matt needs to lead the review of this, since he knows all this code much better than I do. I've made some initial comments on the code in general. Overall it looks pretty good, just some things you can do to improve it for maintainability.
Attachment #831531 -
Flags: review?(david.humphrey) → review-
Assignee | ||
Comment 14•11 years ago
|
||
Comment on attachment 831531 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/315 I'm gonna push review fixes, then flag you again.
Attachment #831531 -
Flags: review?(schranz.m)
Assignee | ||
Comment 15•11 years ago
|
||
Comment on attachment 831531 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/315 Your turn! Code + testing please
Attachment #831531 -
Flags: review?(schranz.m)
Reporter | ||
Comment 16•11 years ago
|
||
Comment on attachment 831531 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/315 Haven't tested yet, but plenty of comments in the PR now.
Attachment #831531 -
Flags: review?(schranz.m) → review-
Reporter | ||
Comment 17•11 years ago
|
||
https://www.dropbox.com/s/4kwglv76hdbkgi2/Screenshot%202013-11-15%2012.54.48.png The keyboard shifts down when the message indicating the correct key sequence is displayed. Other thoughts considerations: - Can we UPPERCASE the entire valid key combo? I mean https://www.dropbox.com/s/0i1yalu6ae9821o/Screenshot%202013-11-15%2012.59.15.png so the CMD+E. - There might be cases where TAB and ENTER are keys that should be part of the sequence but these are both natural ways to update fields/input boxes. As it stands the only way to update a key sequence is to click out of it. Can we change to this to be some sort of APPLY button style? - For multiple key sequences perhaps we should consider listing them all in the message. Right now it appears to only show the last one registered.
Assignee | ||
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•