Video Player Community Code Review - Jan. 27, 2012

make as much public as possible: anything that an integrator might want to tinker with; keep stateful things as methods
if possible, don't pass the full model around from the parent component to its subcomponents: only the part it needs
each component should be responsible for rendering its own sources and responding to its own inputs and events
- controllers responsible for keybindings
- media element responsible for rendering sources and responding to the html5 video element events, and anything else specific to the html 5 media element
in general, add code comments to areas that seem to need improvement but aren't going to be addressed immediately
try latest Infusion in master (now that the subcomponent refactoring branch is merged)
move the feature detection (media element) into the Framework's ProgressiveEnhancement code; check whether or not mediaelement.js passes the test.

move all strings into a single bundle in the top-level component's options
in VideoPlayer.js bindKeyboardControl():
- there's redundency with the defaultKeys list: Use fluid.each() to simplify this
in bindVideoPlayerModel() in VideoPlayer.js: inconsistency: one handler is direct, one is wrapped in an anonymous function
rename fullscreen() to toggleFullScreen() or something else more appropriate
VideoPlayer's preInit() function needs reworking
- methods are peculiar
- validation should happen in changeApplier guards, not in top-level functions
- don't need two guards for increasing and decreasing, only one
- reexamine some of the math used for validation

rename the adapters to "conductors" (consider including 'interval' in the name?)
one conductor per interval list (e.g. caption, transcript...)
each conductor will have its own timer subcomponent
times injected through demands blocks
the conductors will process the interval list and fire start/end events for each interval

in bindScrubberModel() (in VideoPlayer_controllers.js), listeners could be added declaratively (once cindy's branch is merged and we update the framework)
bindCaptionModel() in VideoPlayer_controllers.js: try restructuring the model to avoid the need for code logic: perhaps an array of booleans
toggleButton's toggleState() method in VideoPlayer_controllers.js
- rename rename to something more indicative of the fact that it only requests a change of state
- it shouldn't take an event
double-check whether or not we need to add ARIA to sliders ourselves
use the keyboard-a11y plugin for keyboard control of the volume slider and scrub-bar. If the plugin doesn't do what's necessary, update the plugin
use the keyboard-a11y plugin for the various hot-keys, if possible. At least comment the hard-coded keyCodes if there are no jQuery code available
bind keyboard controls in the controllers subcomponent, not the main VideoPlayer component