Contrib.social

Provide option to display contextual links on embedded entities

Open on Drupal.org →
Created on 1 October 2020, over 3 years ago
Updated 25 March 2024, 3 months ago

Problem/Motivation

The Media Embed filter plugin currently removes contextual links from embedded media. The code mentions:

// - Contextual Links do not make sense for embedded entities; we only allow
//   the host entity to be contextually managed.

However, I think the ability to contextually manage items is exactly why I'd rather embed media than insert an image directly in my content. For example, I could have an "Attribution" field on an image bundle, and want to provide users the ability to edit that attribution wherever that image appears. Same idea with some image style implementations that use cropping, if you are editing content, and see a weird crop in an embedded image, it's a much better UX to edit the image in place, than to click over to the media library, brainstorm what title search might correspond to this image, page thru results...

Proposed resolution

Add a checkbox to the settings form for the Media Embed filter that allows users to opt in to contextual links on embedded media.

Remaining tasks

User interface changes

New option on Media Embed filter settings form, defaulting to current behavior.

API changes

NA

Data model changes

Add new field to filter_settings.media_embed
File: core/modules/media/config/schema/media.schema.yml

    show_contextual_links:
      type: boolean
      label: 'Show contextual links for embedded media'
✨ Feature request
Status

Needs work

Version

11.0 🔥

Component
Media  →

Last updated about 15 hours ago

Created by

🇺🇸United States xaqrox Washington, D.C.

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    Tagging for tests as that will need to happen.
    But also tagging for subsystem review as they may have different thoughts about if this should be implemented.

  • Comment 7 months ago →
    🇩🇪Germany e.r.n.i.e

    I applied the patch #3 ✨ Provide option to display contextual links on embedded entities Needs work successfully (Drupal 10.1).

    👏 Thank you @xaqrox! This is an important UI improvement for my use cases.

  • Comment 7 months ago →
    🇩🇪Germany 4kant

    Patch #3 applied cleanly in D 9.5.10 and in D 10.1.5
    That´s exactly what I was looking for. Webmasters are now able to edit and therefore also to crop images...
    Thanks a lot @xaqrox!

  • First commit to issue fork.
  • Merge request !5626Moved patch to merge request. Fixed bugs schema config missing. Added test to... → (Open) created by phthlaap
  • Pipeline finished with Failed
    7 months ago
    Total: 509s
    #57525
  • Pipeline finished with Success
    7 months ago
    Total: 621s
    #57670
  • Status changed to Needs review 7 months ago
  • Comment 7 months ago →
    🇻🇳Vietnam phthlaap

    Added tests and fix some issues of config.

  • Status changed to Needs work 7 months ago
  • Comment 7 months ago →
    🇺🇸United States smustgrave

    Still needs submaintainer review but relooking and seeing the schema change realize will need an upgrade path + tests

  • Comment 7 months ago →
    🇻🇳Vietnam phthlaap

    Tests already there. Can you help to suggest what is the upgrade path?

  • Comment 7 months ago →
    🇺🇸United States smustgrave

    When making a schema change that would appear in config export an upgrade path needs to be included that will add that setting to existing sites, setting to null. Then a simple test for the upgrade path that
    - checks config doesn't exist
    - run updates
    - checks config now exists with default vlaue

  • Comment 4 months ago →
    🇻🇳Vietnam phthlaap

    Can you please help provide sample code in any module?
    Thanks.

  • Pipeline finished with Failed
    3 months ago
    Total: 167s
    #120457
  • Pipeline finished with Failed
    3 months ago
    Total: 191s
    #120479
  • Pipeline finished with Success
    3 months ago
    Total: 554s
    #120484
  • Pipeline finished with Success
    3 months ago
    Total: 492s
    #121609
  • Status changed to Needs review 3 months ago
  • Comment 3 months ago →
    🇻🇳Vietnam phthlaap
  • Status changed to Needs work 3 months ago
  • Comment 3 months ago →
    🇺🇸United States smustgrave

    Left a comment.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024