Drupal 8 version needed

pferlito,

Thanks for your work on this so far!

Been rather busy lately, but I can't wait to look through it as soon as I get the chance.

Gus

1. +++ b/lib/Drupal/youtube/Plugin/Field/FieldFormatter/YouTubeFormatter.php
   @@ -0,0 +1,154 @@
   +    );
   +
   

   Nit pick... extra space compared to the other code.

2. +++ b/lib/Drupal/youtube/Plugin/Field/FieldFormatter/YouTubeFormatter.php
   @@ -0,0 +1,154 @@
   +    foreach ($parameters as $parameter) {
   +    if ($parameter) {
   +        $cp = t(', custom parameters');
   +        break;
   +      }
   +    }
   +    $summary[] = t('YouTube video: @youtube_size@cp',array('@youtube_size' => $youtube_size,'@cp' => $cp));
   

   Don't think this will do what is intended... it will break out on the first parameter.

3. +++ b/lib/Drupal/youtube/Plugin/Field/FieldFormatter/YouTubeFormatter.php
   @@ -0,0 +1,154 @@
   +  public function prepareView(array $entities_items) {
   +  }
   

   Is this intentionally blank? If so, perhaps add a comment? Not sure what is standard in core.

4. +++ b/lib/Drupal/youtube/Plugin/Field/FieldFormatter/YouTubeThumbFormatter.php
   @@ -0,0 +1,114 @@
   +    $image_link = $this->getSetting('image_link');
   +    $image_link = !empty($image_link) ? t(" linked to @image_link",array('@image_link' => $image_link)) : "";
   

   Extra space before "linked"?

   Add space before array().

5. +++ b/lib/Drupal/youtube/Plugin/Field/FieldFormatter/YouTubeThumbFormatter.php
   @@ -0,0 +1,114 @@
   +  public function prepareView(array $entities_items) {}
   +
   

   Is this the standard way to have an empty function? This is different than the other one above.

6. +++ b/lib/Drupal/youtube/Plugin/Field/FieldType/YouTubeItem.php
   @@ -0,0 +1,80 @@
   +    if (!isset(static::$propertyDefinitions)) {
   +      static::$propertyDefinitions['input'] = array(
   +        'type' => 'string',
   +        'label' => t('video url'),
   +      );
   +      static::$propertyDefinitions['video_id'] = array(
   +        'type' => 'string',
   +        'label' => t('Video id'),
   +      );
   +    }
   

   "video url" is lowercase and "Video id" starts with uppercase. Not sure what is intended.

7. +++ b/lib/Drupal/youtube/Plugin/Field/FieldWidget/YouTubeDefaultWidget.php
   @@ -0,0 +1,99 @@
   +    } elseif (!empty($input)) {
   +      form_error($element,'Invalid URL format');
   +    }
   

   elseif should be on new line.

8. +++ b/lib/Drupal/youtube/Tests/YouTubeTest.php
   @@ -0,0 +1,180 @@
   +
   +    // Verify that the image markup is displayed
   

   Nit pick: Period missing at end of sentence.

9. +++ b/lib/Drupal/youtube/Tests/YouTubeTest.php
   @@ -0,0 +1,180 @@
   +    // Verify that the remote image is created
   

   Nit pick: Period missing at end of sentence.

10. +++ b/youtube.module
    @@ -29,468 +29,517 @@ function youtube_permission() {
    -    return $summary;
    +  // Check if an url path is provided
    

    Nit pick: Period missing at end of sentence.

11. +++ b/youtube.module
    @@ -29,468 +29,517 @@ function youtube_permission() {
    +/**
    + * Get images by building correctly formed URL - kinda hackey.
    

    I think it's "hacky" :)

Tested the D7 and D8 sites this evening.

For the most part things work well and identically. I did notice that in D8, when editing a node, a YouTube Field is not labeled with the label given the field in the content type. It instead is labeled "Video URL". This may be intended behavior in D8?

In both D7 and D8 we noticed that the System-level "Do not show YouTube logo on video player (modestbranding)." option ONLY took effect after some Field-level configurations of the YouTube field had been toggled.
AFTER changing some field-level configuration options this System-level config item DID take effect.

Some notes, I got the patched module running on a vagrant box.

@ http://drupal8.scdug.local/admin/structure/types/manage/article/display - sometimes after editing options under youtube video, the gear icon won't open until a refresh. this can also be fixed by switching between video and thumbnail display modes. This might just be some weird core behavior? It is alpha 6 after all.

@ http://drupal8.scdug.local/admin/config/media/youtube I'd recommend a change in wording to make it more explicit about what is happening (as there is a youtube logo that will load itself over the video).

• orig: Do not show YouTube logo on video player (modestbranding).
• option1: Do not show YouTube logo on video player control bar (modest branding).
• option2: Do not show YouTube logo on video player controls (modest branding).

after switching to responsive display mode @ http://drupal8.scdug.local/admin/structure/types/manage/article/display the following error occurred when viewing the piece of content I had made with the embedded video image.

Twig_Error_Runtime: An exception has been thrown during the rendering of a template ("The PHP cURL extension must be installed to use Guzzle.") in "core/themes/bartik/templates/page.html.twig" at line 167. in Twig_Template->displayWithErrorHandling() (line 279 of /var/www/core/vendor/twig/twig/lib/Twig/Template.php).

This persisted throughout cache clears, but going back to http://drupal8.scdug.local/admin/structure/types/manage/article/display and changing it to another setting fixed it. Changing it back to responsive again did not yeild the same result and it displayed fine.

Thanks so much, pferlito, for your work on this and many thanks to those who have reviewed it as well. I have committed the attached zip from #18 to a new 8.x-1.x branch. I have also made mention of this effort on the project page in hopes to better track progress towards an 8.x-1.0 release.

Future folks: please open new issues for all features, bugs, etc. regarding 8.x-1.x rather than re-opening this one.

Great job on the early 8.x version everyone! :)