Having an element would allow us to use entity browser outside of the field api with more easily.

Comments

Primsi created an issue. See original summary.

primsi’s picture

Category: Feature request » Task
primsi’s picture

Still a work in progress: https://github.com/drupal-media/entity_browser/pull/118

I am not sure about what to do with the field widget display. Adding that to the element would mean

  • more configuration for the element
  • (if I am not mistaken) require that additional configuration for the widget to be available before implementing the element
slashrsm’s picture

slashrsm’s picture

Issue tags: +Media Initiative
slashrsm’s picture

Status: Active » Needs review
StatusFileSize
new17.09 KB

Based on PR that @Primsi started. Few modifications and added a test.

slashrsm’s picture

StatusFileSize
new31.93 KB

Ready for review.

slashrsm’s picture

samuel.mortenson’s picture

Status: Needs review » Needs work

1.

+  if (empty($widget->getThirdPartySetting('entity_browser_entity_form', 'entity_browser_id'))) {
+    return;
+  }
 
-      // Add cardinality for this widget so that we can handle it later.
-      $reference_form['#attached']['drupalSettings']['entity_browser'][$display->getUuid()]['cardinality'] = $instance->getFieldStorageDefinition()->getCardinality();
-    }
+  if ($widget->getThirdPartySetting('entity_browser_entity_form', 'entity_browser_id') === '_none') {
+    return;
   }
-}

"$widget->getThirdPartySetting('entity_browser_entity_form', 'entity_browser_id')" is called three times in this function - can we call it once at the top of the function?

2.

@@ -95,7 +76,14 @@ function entity_browser_entity_form_reference_form_validate(&$reference_form, Fo
   $ief_id = $reference_form['#ief_id'];
   $labels = $reference_form['#ief_labels'];
   $storage = \Drupal::entityTypeManager()->getStorage($reference_form['#entity_type']);
-  $attach_entities = $storage->loadMultiple(explode(' ', $form_values['entity_id']));
+  $attach_entities = $storage->loadMultiple(
+    array_map(
+      function ($item) {
+        return explode(':', $item)[1];
+      },
+      explode(' ', $form_values['entity_browser']['entity_ids'])
+    )
+  );

This sort of routine is used a couple of times, I feel like individual functions shouldn't have to process $form_values['entity_browser']['entity_ids'], it would be nice if they all just called a utility function like _entity_browser_process_enity_ids(string or array). It's also worth noting that the current implementation only works for one Entity Type, even though each entity ID contains an Entity Type.

3.

+/**
+ * Provides an Entity Browser form element.
+ *
+ * Properties:
+ * - #entity_browser: Entity browser or ID of the Entity browser to be used.
+ * - #cardinality: (optional) Maximum number of items that are expected from
+ *     the entity browser. Unlimited by default.

Cardinality is a WidgetValidation plugin now - do we ever need to explicitly support it as a unique setting? This might be worth filing a new issue to track and refactor all instances of cardinality.

4.

@@ -507,7 +522,7 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
             '#theme_wrappers' => ['container'],
             '#attributes' => [
               'class' => ['item-container', Html::getClass($field_widget_display->getPluginId())],
-              'data-entity-id' => $entity->id(),
+              'data-entity-id' => $entity->getEntityTypeId() . ':' . $entity->id(),
             ],
             'display' => $display,
             'remove_button' => [
@@ -520,7 +535,7 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
               '#submit' => [[get_class($this), 'removeItemSubmit']],
               '#name' => $this->fieldDefinition->getName() . '_remove_' . $entity->id(),
               '#limit_validation_errors' => [array_merge($field_parents, [$this->fieldDefinition->getName()])],
-              '#attributes' => ['data-entity-id' => $entity->id()],
+              '#attributes' => ['data-entity-id' => $entity->getEntityTypeId() . ':' . $entity->id()],

Will we need to make the same change in the FileBrowserWidget Field Widget?

slashrsm’s picture

Status: Needs work » Needs review
StatusFileSize
new34.48 KB
new14.36 KB

Fixed all items from #9. I think I found all parts of code that are related to #9.3. It would probably make sense to also support #entity_types limit. I guess this can be a follow-up, since we need to update validator too (to support multiple types).

Also changed order of arguments in DisplayInterface::displayEntityBrowser() to follow core pattern.

  • slashrsm committed 729d7db on 8.x-1.x authored by Primsi
    Issue #2624874 by slashrsm, Primsi, samuel.mortenson: Provide the Entity...
slashrsm’s picture

Status: Needs review » Fixed

Committed. Created a follow-up in entity embed: #2767573: [Follow-up] Use Entity browser's form element

Status: Fixed » Closed (fixed)

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

gábor hojtsy’s picture

Issue tags: -Media Initiative