[wp-trac] [WordPress Trac] #32417: Add new core media widget

[WordPress Trac]

#32417: Add new core media widget
-------------------------------------------------+-------------------------
 Reporter:  melchoyce                            |       Owner:  melchoyce
     Type:  feature request                      |      Status:  assigned
 Priority:  normal                               |   Milestone:  4.8
Component:  Widgets                              |     Version:  4.3
 Severity:  normal                               |  Resolution:
 Keywords:  needs-unit-tests has-patch needs-    |     Focuses:  ui,
  refresh                                        |  administration
-------------------------------------------------+-------------------------

Comment (by sirbrillig):

 And here's a few notes on the otherwise excellent PHP in
 `WP_Media_Widget`:

 - `$selectedLink` should be snake-case, like `$selected_link`.
 - There are a few cases where we see `.'</a>'`, which is missing a space
 between the concat operator and the string.
 - Do any instances of concatenated output need to be escaped in
 `widget()`? Such as `align` and `description` here: `'<p class
 ="attachment-description align' . $instance['align'] . '">' .
 $instance['description'] . '</p>'`? Or `$title` just above that? Maybe
 that's all handled when data is saved, but it's unclear here.
 - Very minor but you could save a few lines by removing the final `else`
 clause when assigning `$selected_link` and setting an initial empty value
 (or just allowing an undefined value since it is checked with `empty()`
 everywhere it is used, but I always like using defined variables). In
 addition, currently `$selected_link` is undefined if `$instance['link']`
 is empty, so if we want to use an initial value it should be set outside
 of any conditions.
 - Inside `widget()`, there's two nested conditionals `if ( ! empty(
 $instance['id'] ) ) { if ( $attachment = get_post( $instance['id'] ) ) {`
 that could be one condition instead to reduce indentation: `if ( ! empty(
 $instance['id'] ) && $attachment = get_post( $instance['id'] ) ) {`.
 - Similar to my suggestion for the JS, generating markup from the the
 different media types in `widget()` could be split so that each has its
 own function, making `widget()` smaller and easier to follow.
 - There's two semi-colons after the first line of `render_audio()`.
 - Why do the private methods `get_attachment_audio()` and
 `get_attachment_video()` accept an attribute array, but not use it for
 anything?
 - `get_responsive_style`, `get_attachment_image`, `get_attachment_audio`,
 and `get_attachment_video` are missing a `@return` docblock.

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