Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

Status: Active » Needs review
FileSize
3.81 KB

Initial patch.

phenaproxima’s picture

A small rephrase in the add form stuff, added @see lines for additional examples in core, and @defgroup.

Wim Leers’s picture

Status: Needs review » Needs work

Great start! I found a few nits and a few paragraphs that with some refinement will make it much easier to understand :)

  1. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * reuse and add media in entity reference fields, or in text editors, using
    

    🤓 or in text editorsor in text fields

    (Yes, the media library opener lives on a text editor, but the reusing of media happens in text fields.)

  2. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * Interaction with the modal media library dialog is mediated by special
    + * services, called "openers". All openers must implement
    

    by special services, called "openers"by "opener" services

  3. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * in the library. Essentially, an opener is a "bridge" between the opinionated
    + * media library modal dialog, and the "thing" that is trying to use the media
    + * library to select media items. Examples of openers include entity reference
    + * fields and text editors.
    

    Essentially […] text editors.

    Openers enable different consumers of the Media Library to trigger a media library modal dialog in a way that makes sense for that particular consumer. Examples in Drupal core include entity reference fields and text editors.

  4. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * @section state Modal dialog state
    + * When the media library modal is used, its configuration and status (such as
    

    "state" vs "status"

    Should be "state" always.

  5. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * It is possible for users who have the appropriate access to add media to the
    + * library from directly within the modal dialog. This interaction is handled
    + * almost entirely by form classes, which in turn can be provided by modules.
    
    1. "who have the appropriate access" sounds a bit odd?
    2. "handled almost entirely by form classes" sounds weirdly vague — "almost entirely" lacks precision. We need to be either precise or omit this IMHO.
    3. "which in turn can be provided by modules" → so there is an API here? I see this is sort of explained later. I understand this paragraph is painting the high-level picture. But I think it'd be better to explain why forms are used: because it allows this customization elegantly. So: I think framing this differently will make it much easier to understand :)
phenaproxima’s picture

Status: Needs work » Needs review
FileSize
4.04 KB
3.42 KB

Tried to make these changes. How does this sound?

phenaproxima’s picture

Ugh, missed a word. I hate when I do that.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Media Initiative

Looks much better, thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  1. This looks helpful - nice work
  2. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * reuse and add media in entity reference fields, or in text fields, using a
    + * modal dialog.
    

    I think this sentence is a bit confusing. I the the media library provides an alternate selection for entity reference fields AND provides another of select media to place in text fields. I think the re-use of the word in is odd. You don't reuse and add media in entity reference fields - you're selecting the reference.

  3. +++ b/core/modules/media_library/media_library.api.php
    @@ -0,0 +1,93 @@
    + * within the modal dialog. This interaction is handled by form classes,
    + * allowing modules to customize the process of adding media.
    

    This interaction is handled by form classes... I think this needs more detail. Feels a bit hand-wavy. I think this really relates to the next section. I think this sentence belongs there and needs to be re-wrriten a bit.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
4.03 KB
2.22 KB

How's this? I tried to streamline it and explain it a bit more clearly.

starshaped’s picture

Status: Needs review » Reviewed & tested by the community

This documentation is clear, concise, and makes sense to me. Great job!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 2607278f2c to 9.0.x and 376a846edf to 8.9.x and 48229e83d2 to 8.8.x. Thanks!

  • alexpott committed 2607278 on 9.0.x
    Issue #3088476 by phenaproxima, Wim Leers: Document Media Library's API...

  • alexpott committed 376a846 on 8.9.x
    Issue #3088476 by phenaproxima, Wim Leers: Document Media Library's API...

  • alexpott committed 48229e8 on 8.8.x
    Issue #3088476 by phenaproxima, Wim Leers: Document Media Library's API...
xjm’s picture

Crediting myself for this issue as well; it wasn't documented here, but I contributed to it in Slack. :P

xjm’s picture

We are in commit freeze though so this shouldn't have been backported to 8.8.x. Docs aren't going to break anything, but just FYI.

Status: Fixed » Closed (fixed)

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