Allow passing existing selection from field widget

Yes, I think that we shouldn't make any assumptions about that. Entity reference field supports that and we should too in my opinion.

#2768585: Add option for prepending of fresh added items seems related.

Thanks! This looks great! First review:

1. +++ b/js/entity_browser.common.js
   @@ -21,49 +21,44 @@
   +    var entity_reference_options = drupalSettings['entity_browser'][uuid];
   

   This is common code, which can be used outside of the field scope. We shouldn't refer to the entity reference.

   I am personally a bit against this kind of assignments as I think they decrease readability of code.

2. +++ b/js/entity_browser.common.js
   @@ -21,49 +21,44 @@
   +    // In case that edit selection mode is used,
   +    // overwrite of selection will be made.
   

   Comment should use all 80 characters.

3. +++ b/src/Annotation/EntityBrowserSelectionDisplay.php
   @@ -40,4 +40,11 @@ class EntityBrowserSelectionDisplay extends Plugin {
   +   * Information does Selection Display support preselection.
   

   This comment reads a bit strange.

4. +++ b/src/Element/EntityBrowserElement.php
   @@ -62,11 +98,25 @@ class EntityBrowserElement extends FormElement {
   +      // Check does Entity Browser Selection Display supports preselection.
   +      $entity_browser_selection_display_definition = $entity_browser->getSelectionDisplay()
   +        ->getPluginDefinition();
   +
   +      if (!$entity_browser_selection_display_definition['acceptPreselection']) {
   

   It would probably make sense to have a helper function to check that. Would improve DX in my opinion.

5. +++ b/src/Plugin/EntityBrowser/SelectionDisplay/MultiStepDisplay.php
   @@ -186,9 +190,11 @@ class MultiStepDisplay extends SelectionDisplayBase {
   +      $diff_selected_size = count($selected_entities) - count($weights);
   +      if ($diff_selected_size > 0) {
   

   Do we need this extra variable?

6. +++ b/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php
   @@ -325,8 +341,14 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
   +      $stored_ids = $form_state->get(['entity_browser_widget', $this->getFormStateKey($items)]);
   +      $indexed_entities = $entity_storage->loadMultiple($stored_ids);
   +
   +      foreach ($stored_ids as $entity_id) {
   +        if (isset($indexed_entities[$entity_id])) {
   +          $entities[] = $indexed_entities[$entity_id];
   +        }
   +      }
   

   We load entities and then we iterate over their initial ids. Seems redundant. What am I missing?

7. +++ b/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php
   @@ -381,11 +412,18 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
   +    $selection_mode = $this->getSetting('field_widget_selection_mode');
   +    if (
   +      $cardinality == FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED
   +      || count($ids) < $cardinality
   +      || $selection_mode == EntityBrowserElement::SELECTION_MODE_EDIT
   +    ) {
   

   We need to figure out how to refactor this condition. Probably with some logic before the actual diff.

8. +++ b/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php
   @@ -381,11 +412,18 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
   +        '#default_value' => $entities,
   

   #default_value should only be set if edit mode is active.

9. +++ b/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php
   @@ -472,9 +511,10 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
   +        function ($item, $index) use ($id, $row_id) {
   +          return ($item != $id) || ($index != $row_id);
   +        },
   +        ARRAY_FILTER_USE_BOTH
   

   This will raise PHP requirement to 5.6 (which I am fine with). Core still depends on >= 5.5.9. We will need to document this requirement in README and check it in hook_requirements() if we want to do this right.

10. +++ b/tests/modules/entity_browser_test/config/install/entity_browser.browser.test_entity_browser_iframe.yml
    @@ -9,8 +9,13 @@ display_configuration:
    -selection_display: no_display
    -selection_display_configuration: {  }
    +selection_display: multi_step_display
    +selection_display_configuration:
    +  entity_type: file
    +  display: label
    +  display_settings: {  }
    +  select_text: 'Use selected'
    +  selection_hidden: false
    
    +++ b/tests/modules/entity_browser_test/src/Form/FormElementTest.php
    @@ -28,6 +28,7 @@ class FormElementTest extends FormBase {
    +      '#selection_mode' => EntityBrowserElement::SELECTION_MODE_EDIT,
    

    Do we need to update those since we're not actually testing this?

11. There are quite some changes in the field widget class. In the past we've seen a lot of bugs in the area of nested widgets (Paragraphs, Inline entity form). We need to thoroughly test that to be really sure that we didn't introduce any regressions.

This looks almost ready. Thank you for your hard work! Few nitpiks:

1. +++ b/src/Element/EntityBrowserElement.php
   @@ -33,9 +36,35 @@ use Drupal\Core\Entity\EntityInterface;
   +   * Selection from entity browser will be prepend to existing list.
   

   s/prepend/prepended

2. +++ b/src/Element/EntityBrowserElement.php
   @@ -33,9 +36,35 @@ use Drupal\Core\Entity\EntityInterface;
   +   * When this selection mode is sued, then entity browser will be populated
   +   * with existing selection and returned selected list will replace existing
   +   * selection. Also used selection display has to support preselection.
   

   s/sued/used (noticed by @primsi :))

   Last sentence reads a bit strange.

3. +++ b/src/Element/EntityBrowserElement.php
   @@ -33,9 +36,35 @@ use Drupal\Core\Entity\EntityInterface;
   -  const CARDINALITY_UNLIMITED = -1;
   +  const SELECTION_MODE_EDIT = 'selection_edit';
   

   Cardinality constant was put here on purpose. The reason for that was the fact that the Entity browser doesn't depend on fields at all. Fields are just one possible usage of it.

   However, this doesn't seem that important. While I prefer the current solution I am open for other solutions too.

4. +++ b/src/Element/EntityBrowserElement.php
   @@ -45,16 +74,76 @@ class EntityBrowserElement extends FormElement {
   +  protected static function checkPreselectionSupport(EntityBrowserInterface $entity_browser) {
   

   I think that this function belongs to the Selection display plugin class.

5. Ad 6: Understood. Let's add a comment that explains that.
6. Ad 8: Good point. Docs for #default_value on EntityBrowserElement are not correct in that case. Let's update them.

Committed. Thanks!