Add an upload to YouTube Widget

Hey Jakimus,

This sounds awesome, but it is probably beyond the intended scope of the original module. That said, it could either be created as another module that lives within the same directory/repository or as an entirely new module altogether. That way users could avoid enabling it and messing with API keys unless they wanted that specific functionality.

The 7.x version of the YouTube Field module has a YouTube Colorbox module living within it (youtube/modules/youtube_colorbox). (The 8.x version doesn't yet because it hasn't yet been ported.) I'd imagine something similar here (youtube/modules/youtube_upload). If this makes sense to you, please create it like that in your repository and then create a patch that you attach to this issue (using these instructions). I can then review that patch, provide feedback, and ultimately commit it should it make sense to do so. Let me know if you need more help beyond that link.

Or you could attempt to register the project at drupal.org/project/youtube_upload (or whatever you wanted to name it), but it'd have to go through the module approval process. I could help point you in the right direction if this is the path you'd like to take.

Thanks!
Gus

Hi Jakimus,

My apologies for the delay in getting to this. Thanks for following the initial suggestions!

First, I really liked the approach to only adding the initial youtube library when the field was being rendered. I actually went ahead and committed a variation of that improvement: 3cefacd. Please pull the latest branch before creating another patch.

Next, I will admit that I am not yet up to speed with a lot of Drupal 8 code. My suggestions and thoughts on the remainder of the patch will be my best attempt. :)

I've noticed the 2 JS files are copyrighted by Google. For that reason I think they cannot technically be committed to the repo and hosted on Drupal.org. It looks like they'll have to instead by added to the library as external. I see the plusone.js file in the libraries file being listed as external - can the same not be done for the other two? If you didn't already see it, this documentation may help.

As a part of the previously mentioned commit I renamed the drupal.youtube library to drupal.youtube.responsive and did the same with the CSS file. That format seems to be what core modules use. Can we do the same here? Perhaps the library is drupal.youtube.upload and the CSS file is youtube.upload.css? Or perhaps because the module is youtube_upload, they can be drupal.youtube_upload and youtube_upload.css. Perhaps that would be more consistent.

As for the CSS file, why are these elements being hidden? Could we document that at the top of the file (and add a @file docblock too) If they should continue to be hidden, the code could be simplified a bit:

.post-sign-in,
.during-upload,
.post-upload,
.youtube-upload-wrapper {
  display: none;
}

As for youtube_upload.routing.yml, I'm not seeing the link on admin/config? I do see the settings form when I go to the URL though. Instead of getting it to appear on admin/config, I'm wondering if it should be something like /admin/config/media/youtube/upload and be a local task (assuming that's what they're still called in Drupal 8 - a tab, essentially) that can be navigated to from /admin/config/media/youtube. Thoughts on that?

I like your initial README.txt, but I do think you're right in that the Google API instructions could be slightly improved. I think the step-by-step instructions could be moved from "Configuration" to "Installation". I think they could also be a numbered list. Picky, but could you also wrap the lines at 80 characters? That'd follow the module's main README and standards.

For the .info.yml file, please remove # Information added by Drupal.org packaging script on 2015-11-11 and everything below it. That's added by Drupal.org and I'm assuming it came from a copy/paste of youtube.info.yml.

In the config, can we prefix the variable name with youtube_upload_. I'm assuming that standard hasn't changed from 7 to 8. Also wondering if there is another module to handle Google auth stuff? Then you wouldn't have to worry about storing variables or writing setup instructions for that yourself. At a glance I'm seeing the gauth module but it doesn't mention Drupal 8 at all so who knows.

In YoutubeUploadSettingsForm.php:

• please rename getFormID() to getFormId(). This was an issue that was fixed in YoutubeSettingsForm.php earlier today.
• please wrap description text at 80 characters. Also, perhaps instead of having the same text as the README, you could simply instruct people to read the README for more information on how to obtain the ID.

In YoutubeUploadWidget.php I see there are "FIX" comments and code style issues. Could you run PHP CodeSniffer or Coder on this file to clean these up? I also noticed a lot of markup in the form items. I'm wondering if that can somehow be done a better way. Is there perhaps a way to provide a template for a field widget?

Again, I appreciate the patch. This seems like something that could be very useful for others so I'd love to see it continued. I hope my delays haven't dampened your interest. This was more of a visual code review, but when the next patch is submitted I'll follow the README steps and test the uploading. Let me know if you need help or have any questions.

Please work with the new YouTube Import module for uploading functionality.

My mistake. Uploading videos is not what the module mentioned in the previous comment does.

Closing after 10 months with no response to the patch feedback and no interest expressed by others. Please reopen if anyone would like to pick up where we left off.