Support Media core module (Drupal core 8.4.x-dev)

Great work. Thanks. Haven't had time to test/check yet. It would be great if we got some more testing feedback too.

Regarding on how not to break support for contrib, I think other modules in the ecosystem created a new branch for that. We could probably do the same.

I tested it functional and it works quite well so far.

I have two suggestions:

1. Rename all variable occurrences from media_entity to media
2. We have a configuration key media_entity_bundle. Would be nice if we could rename it to media_type. Then we need an update hook as well

I have adjusted few things, but I didn't test it extensively.

The update hook should live in eb_widget/dropzonejs_eb_widget.install

I have moved update hook to the submodule.

Hopefully, uploading the patch file with all changes. :>

I checked the upgrade path and it worked fine.

Only thing that was not working was the type filtering on the config form. I removed that part, because it was an improvement of this patch.
We can do that in a proper way in a follow-up.

I would mark it as RTBC, but now i changed a bit.

There are a few other places where we could maybe do some renaming:

./modules/eb_widget/dropzonejs_eb_widget.install:33:        if (!empty($config['settings']['media_entity_bundle']) && empty($config['settings']['media_type'])) {
./modules/eb_widget/dropzonejs_eb_widget.install:34:          $config['settings']['media_type'] = $config['settings']['media_entity_bundle'];
./modules/eb_widget/dropzonejs_eb_widget.install:35:          unset($config['settings']['media_entity_bundle']);

Another code style nitpick: camel case variables are only allowed for class members.

Apart from that looks good to me.

@Primsi - I have uploaded a new patch with the renaming of variables.

And code that you have marked is correct since media_entity_bundle key is used for configuration by version 1.x and with 2.x we are renaming that key, then update hook has to migrate configuration from old key to new key. So, that's how it should work.

Patch looks good to me. All I found were nitpicks.

1. +++ b/modules/eb_widget/dropzonejs_eb_widget.install
   @@ -0,0 +1,52 @@
   +  /** @var \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager */
   +  $entity_type_manager = \Drupal::service('entity_type.manager');
   

   If we use the static (procedural) style of loading the entity browsers, this is not needed. If we decide to keep it, though, \Drupal::entityTypeManager() is preferable, and the type hint is not needed.

2. +++ b/modules/eb_widget/dropzonejs_eb_widget.install
   @@ -0,0 +1,52 @@
   +  /** @var \Psr\Log\LoggerInterface $logger */
   

   This type hint is not necessary; \Drupal::logger() documents the return type.

3. +++ b/modules/eb_widget/dropzonejs_eb_widget.install
   @@ -0,0 +1,52 @@
   +  $entity_browsers = $entity_type_manager->getStorage('entity_browser')
   +    ->loadMultiple();
   

   Supernit: Since this code is procedural, why not simply do EntityBrowser::loadMultiple(), to shave off a few lines?

4. +++ b/modules/eb_widget/dropzonejs_eb_widget.install
   @@ -0,0 +1,52 @@
   +          $logger->warning(sprintf('Unable to changed configuration for widget (%s) of entity browser (%s)', $widget->label(), $entity_browser->label()));
   

   Nit: Should be "unable to change configuration..."

5. +++ b/modules/eb_widget/src/Plugin/EntityBrowser/Widget/InlineEntityFormMediaWidget.php
   @@ -171,7 +171,7 @@ class InlineEntityFormMediaWidget extends MediaEntityDropzoneJsEbWidget {
   +    $source_field = $this->getType()->getSource()->getConfiguration()['source_field'];
   

   Nit: You can also do $this->getType()->getSource()->getSourceFieldDefinition(), to keep the abstraction sealed :)

6. +++ b/modules/eb_widget/src/Plugin/EntityBrowser/Widget/MediaEntityDropzoneJsEbWidget.php
   @@ -111,29 +111,29 @@ class MediaEntityDropzoneJsEbWidget extends DropzoneJsEbWidget {
   +    if (!empty($media_types)) {
   +      foreach ($media_types as $media_type) {
   +        $form['media_type']['#options'][$media_type->id()] = $media_type->label();
   

   Nit: The !empty() check is not needed; loadMultiple() returns an empty array if there are no media types.

Fixed all the nits except from 6. If we remove the !empty() check we can't provide a help message.

Alllll-righty then! Looks great. Thanks, @chr.fritsch!

Thanks everyone. Committed, pushed and created a new branch.

Hmm, shouldn't the info file have entity_browser (>=8.x-2.x) as dependency? Right now it automatically gets version 1.x.