Problem/Motivation

Add formatter for https://www.npr.org/ videos.

Remaining tasks

Review and test

CommentFileSizeAuthor
#4 3265320-2.patch10.18 KBgtr18
#2 3265320-1.patch10.34 KBgtr18

Comments

GTR18 created an issue. See original summary.

gtr18’s picture

StatusFileSize
new10.34 KB
marcoscano’s picture

Version: 1.1.0 » 1.x-dev
Status: Needs review » Needs work
Issue tags: -Needs tests, -features

Thank you for contributing! I think we are almost there, I just found a few nitpicks with the code but after that we'll be good to go.

  1. +++ b/media_remote.module
    @@ -65,6 +65,14 @@ function media_remote_theme($existing, $type, $theme, $path) {
    +        'type' => NULL,
    

    Are we using this `type` variable or is it just leftover from other formatters?

  2. +++ b/src/Plugin/Field/FieldFormatter/MediaRemoteNPRFormatter.php
    @@ -0,0 +1,127 @@
    +        '#type' => $matches[1][0],
    

    Again, do we need the type here?

  3. +++ b/templates/media-remote-npr.html.twig
    @@ -0,0 +1,14 @@
    + * Template implementation for the media_remote_buzzsprout theme hook.
    

    Copy/paste leftover?

  4. +++ b/templates/media-remote-npr.html.twig
    @@ -0,0 +1,14 @@
    + * - type: (string) The type of this content: document, spreadsheets or
    

    We don't seem to be using the type, we could just remove it here I think.

gtr18’s picture

StatusFileSize
new10.18 KB
gtr18’s picture

Issue tags: +Needs review
gtr18’s picture

Status: Needs work » Needs review

  • marcoscano committed 6d98369 on 1.x authored by GTR18
    Issue #3265320 by GTR18, marcoscano: Add support for NPR
    
marcoscano’s picture

Status: Needs review » Fixed
Issue tags: -Needs review

Looks great, thanks for contributing!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.