[wp-trac] [WordPress Trac] #38678: Video Headers: Improve Basic Accessibility

[WordPress Trac]

#38678: Video Headers: Improve Basic Accessibility
------------------------------------+----------------------------
 Reporter:  davidakennedy           |       Owner:  joemcgill
     Type:  defect (bug)            |      Status:  assigned
 Priority:  normal                  |   Milestone:  4.7
Component:  Themes                  |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |     Focuses:  accessibility
------------------------------------+----------------------------

Comment (by bradyvercher):

 Replying to [comment:16 joemcgill]:
 > * What are the benefits for switching to constructors for rather than
 methods for handler callbacks?
 > * Why are we triggering play/pause events on the video container rather
 than relying on the play/pause events of the video itself? Is it to
 provide consistent events regardless of the player used (e.g. YouTube) or
 did you have something else in mind?

 There's two way communication going on between the handlers and the header
 object. To keep things DRY, then header object creates the button and
 wires up the click event. When the button is clicked, it calls the
 `play()`, `pause()`, and `paused()` methods on the handler to update the
 videos state.

 On the other hand, each handler triggers an event on the header container
 so it can update the button classes and text based on the video state.
 Without this part, the button could easily get out of sync with the actual
 state of the video.

 It's certainly not the only way to build this out, but it's important that
 the button is updated based on the video state.

 > The YT object needs to be defined somewhere, rather than assuming it's
 available in the window.

 The only code that relies on this is wrapped in a method that only gets
 called when `YT` is available in the global scope. If defining it is to
 prevent linting warnings, it should probably be declared as a global at
 the top of the file using a JSHint globals directive.

 > * CustomHeader has a `header` variable defined that is never used.
 > * "native" is a JavaScript reserved word so we need a different name for
 that handler variable.

 Good catch on those. The whitespace changes don't really follow the
 [https://make.wordpress.org/core/handbook/best-practices/coding-
 standards/javascript/#arrays-and-function-calls examples in the JS coding
 standards]. Should those be updated?

 Replying to [comment:18 afercia]:
 > Not a youtube API expert, but shouldn't this make use of
 `onYouTubeIframeAPIReady()`?

 That's similar to the `YT.ready()` callback, but if we define it, then
 it'll create a conflict with any other code that's also trying to define
 it.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/38678#comment:19>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform