Problem/Motivation

The contextual links appear when rendered entity field widget display is used. So if an unaware user clicks "Edit", actually leaving the editing page and loosing data.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Primsi created an issue. See original summary.

Primsi’s picture

Status: Active » Needs review
FileSize
1.79 KB

Initial patch, not test yet. I think we don't have anything that could be just extended easily.

arpad.rozsa’s picture

+  if ($entity instanceof MediaInterface && isset($build['#entity_browser_suppress_contextual'])) {
+    unset($build['#contextual_links']);
+  }

Entity browser works with entities other than media so this check is not needed.

For the test I found the EntityReferenceWidgetTest would work, so I added the assert there to check if the contextual links don't exist on the page.

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/src/Plugin/EntityBrowser/FieldWidgetDisplay/RenderedEntity.php
    @@ -72,8 +72,10 @@ class RenderedEntity extends FieldWidgetDisplayBase implements ContainerFactoryP
       public function view(EntityInterface $entity) {
    -    return $this->entityTypeManager->getViewBuilder($this->configuration['entity_type'])
    +    $build = $this->entityTypeManager->getViewBuilder($this->configuration['entity_type'])
           ->view($entity, $this->configuration['view_mode']);
    +    $build['#entity_browser_suppress_contextual'] = TRUE;
    +    return $build;
       }
     
    

    What about render-caching?

    If the same view mode is used in a different context than entity browser then it would also not show contextual links there.

    You can fix that by adding an extra 'entity_browser' key to $build['#cache']['keys']

  2. +++ b/tests/src/FunctionalJavascript/EntityReferenceWidgetTest.php
    @@ -16,6 +16,24 @@ use Drupal\user\Entity\Role;
    +   *
    +   * @var array
    +   */
    +  public static $modules = [
    +    'entity_browser_test',
    +    'contextual',
    +    'views',
    +    'block',
    +    'node',
    +    'file',
    +    'image',
    +    'field_ui',
    +    'views_ui',
    +    'system',
    +  ];
    +
    

    you don't need to repeat modules from the parent, just add which module you need additionally.

arpad.rozsa’s picture

Status: Needs work » Needs review
FileSize
3.96 KB
1.97 KB

Added the cache key and removed the modules list from the test that you suggested.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

I guess it could be argued that having those links there is useful in some cases, but unless your users are experienced and know that they need to open these links in a new tab it could easily lead to data loss.. similar to links in node previews and so on.

Setting to RTBC to get the attention of @oknate and whoever else has an opinion on it, if not sure we could maybe add a setting for it?

oknate’s picture

I think if someone wants them, they can use widget alter. I think this is good to go.

oknate’s picture

Actually, we may want to use $build['#contextual_links']['#access'] = FALSE, instead of unset($build['#contextual_links']);

It would be easier to override in other modules in hook_entity_view_alter().

oknate’s picture

FileSize
81.26 KB

Ah, I see, it's not a build array at this point.

contextual link data

  • oknate committed 8f1c589 on 8.x-2.x authored by arpad.rozsa
    Issue #3081971 by arpad.rozsa, Primsi, Berdir: Disable contextual links...

  • oknate committed 9b9511a on 8.x-1.x authored by arpad.rozsa
    Issue #3081971 by arpad.rozsa, Primsi, Berdir: Disable contextual links...
oknate’s picture

Status: Reviewed & tested by the community » Fixed

Committed, thanks!

🎉

Status: Fixed » Closed (fixed)

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