Skip to content

Conversation

@jmclaus
Copy link

@jmclaus jmclaus commented Sep 3, 2013

This PR fixes the following issues in the video player:

The speed button was behaving differently than the volume button when tabbing through its elements (and was broken when tabbing backwards):
When going forward, we would traverse as follows: play/pause button, highest speed to lowest speed, volume button
When going backwards volume button, speed button, highest speed, speed button, highest speed etc.

We now have the following:
Forward: play/pause button, speed button, highest speed to lowest speed, speed button, volume button.
Backwards: volume button, speed button, lowest speed to highest speed, speed button, play/pause button.

This mirrors the behavior of the volume button.

@ghost ghost assigned valera-rozuvan and auraz Sep 3, 2013
@valera-rozuvan
Copy link
Contributor

@jmclaus I have checkout your branch, and ran it on the example course content-demos found at https://github.com/MITx/content-demos . Looking at a video (YouTube source) in both Chrome and Firefox I was not able to tab further than the speeds control. It kept going through all of the speeds, then closing, and the opening again and going through all of the speeds.

The same happens in Studio.

@jmclaus
Copy link
Author

jmclaus commented Sep 4, 2013

@valera-rozuvan Commit 3cc2cc9 fixes all the issues. Tested in Chrome/Firefox with LMS/CMS.

@valera-rozuvan
Copy link
Contributor

@jmclaus Verified manually - it works very well!

@valera-rozuvan
Copy link
Contributor

The code is well documented and easy to follow. There is however one issue that if more controls will be added, or the ordering of controls will change, then things might break. For the developer to know that he broke something I think that this particular fix should be covered by Jasmine tests.

@jmclaus
Copy link
Author

jmclaus commented Sep 4, 2013

Very good point. I will take care of it.

@jmclaus
Copy link
Author

jmclaus commented Sep 5, 2013

@valera-rozuvan I added a Jasmine test that checks out the order of the controls and if there was an insertion of a new one.

@valera-rozuvan
Copy link
Contributor

👍 Good to merge. Please do a git rebase master first though.

jmclaus added 3 commits September 6, 2013 09:22
… tabbing, in certain condition, would get stuck on speed button.
… tabbing on the speed control will malfunction if it is not the case.
jmclaus pushed a commit that referenced this pull request Sep 6, 2013
…ontrol

Speed button know behaves like the volume button when tabbing forward or...
@jmclaus jmclaus merged commit faf1f54 into master Sep 6, 2013
@jmclaus jmclaus deleted the jmclaus/bugfix_tabbing_video_speed_control branch September 6, 2013 08:10
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
Added an indicator on Progress page when there are no problem scores in a section.
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Aug 9, 2016
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Aug 9, 2016
…ix-requirement-version

Fix requirements version openedx#854
cgopalan pushed a commit to open-craft/openedx-platform that referenced this pull request Aug 24, 2017
yonk-729: urban airship push notification support
jfavellar90 added a commit to eduNEXT/edx-platform that referenced this pull request Aug 23, 2018
caesar2164 pushed a commit to caesar2164/edx-platform that referenced this pull request Dec 15, 2018
* stv/content-visibility-warning:
  Move CONTENT_VISIBILITY_NOTICE to theme
DanielVZ96 pushed a commit to open-craft/openedx-platform that referenced this pull request Jan 31, 2024
Abdul-Muqadim-Arbisoft pushed a commit to edly-io/edx-platform that referenced this pull request May 8, 2024
Bump the default image reference for MongoDB from 4.2.17 to 4.2.24, to
address a critical issue present in versions 4.2.0 through 4.2.23.

References:
https://www.mongodb.com/docs/manual/release-notes/4.2/#patch-releases
https://jira.mongodb.org/browse/WT-10461

Fixes openedx#854.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants