Implementation of the initial Media Management Proposal

Created on 14 November 2024, about 1 month ago

Implement the recipe in code from [META] Track 15: Proposal for media management 📌 [META] Track 15: Proposal for media management Active

📌 Task
Status

Active

Component

Track: Media Management

Created by

🇬🇧United Kingdom tonypaulbarker Leeds

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @tonypaulbarker
  • Merge request !188Issue #3487391 "Media mgmt implementation" → (Merged) created by tonypaulbarker
  • Pipeline finished with Failed
    about 1 month ago
    Total: 597s
    #338235
  • Pipeline finished with Failed
    about 1 month ago
    Total: 44s
    #339472
  • Pipeline finished with Failed
    about 1 month ago
    Total: 44s
    #339473
  • 🇭🇺Hungary Gábor Hojtsy Hungary

    Although you are linking 📌 [META] Track 15: Proposal for media management Active , I believe the description of your intended result is on 🌱 Proposal for Media Management in early versions of Drupal CMS Active , right? Do you have a checklist/summary of how much did you implement from that in this MR?

  • 🇬🇧United Kingdom tonypaulbarker Leeds

    @gábor-hojtsy thanks, I have added that issue to the description. I believe we have implemented everything described there once snagging is done and in addition some of the maybes and nice to haves.

    This issue / MR will contain

    MVP:

    • Image styles and responsive image styles optimised for Olivero and plumbed in with view modes. Formats as described and using focal point for crops.
    • Remote video with small, medium and large view modes
    • Documents

    Additionally:

    • Strong media support in CKEditor including selecting view modes for size / dimensions
    • SVGs with view modes for small, medium and large scaling options
    • optional local audio
    • optional local video

    Note:

    The privacy compliant embedded media described there is already included, coming from the privacy track.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 64s
    #340101
  • Pipeline finished with Failed
    about 1 month ago
    Total: 64s
    #340100
  • Pipeline finished with Failed
    about 1 month ago
    Total: 68s
    #340113
  • Pipeline finished with Failed
    about 1 month ago
    Total: 103s
    #340112
  • Pipeline finished with Failed
    about 1 month ago
    Total: 67s
    #340132
  • Pipeline finished with Failed
    about 1 month ago
    Total: 65s
    #340133
  • Pipeline finished with Failed
    about 1 month ago
    Total: 69s
    #340190
  • Pipeline finished with Failed
    about 1 month ago
    Total: 75s
    #340191
  • Pipeline finished with Failed
    about 1 month ago
    Total: 66s
    #340335
  • Pipeline finished with Failed
    about 1 month ago
    Total: 69s
    #340336
  • Pipeline finished with Failed
    about 1 month ago
    Total: 102s
    #342673
  • Pipeline finished with Failed
    about 1 month ago
    Total: 106s
    #342672
  • Pipeline finished with Failed
    about 1 month ago
    Total: 96s
    #342687
  • Pipeline finished with Failed
    about 1 month ago
    Total: 96s
    #342686
  • Pipeline finished with Failed
    about 1 month ago
    Total: 619s
    #342708
  • Pipeline finished with Failed
    about 1 month ago
    Total: 742s
    #342707
  • 🇺🇸United States phenaproxima Massachusetts

    Reviewed this and I have two overarching concerns, one of which is commit-blocking and one of which isn't.

    The blocking one is that media_base is constructed as an anti-pattern. It should not include any media types of its own; only things which may be needed by all media types. We probably will want the image media type to have its own recipe, built on top of media_base.

    The non-blocking one is ZOMG I have never seen this many image styles in my entire life. Won't marketers find this overwhelming? This is an enormous amount of config, choices, and (seemingly) UI complexity to be introducing. How do we expect people to use this? I don't build sites for a living so I may very well be coming at this wrong, but this seems like it will be very intimidating for people. Should we streamline it?

  • 🇬🇧United Kingdom tonypaulbarker Leeds

    @phenaproxima SVG image is a separate media type because focal point can't apply any crops. We could combine it for ease of use but if you use a portrait aspect image style with a landscape aspect SVG image you will render a small image with a gap. Therefore, when an SVG image is selected in CKEditor, the editor has a choice of small, medium or large to select from, but not aspect ratios. The editor profile is not in this MR because we were waiting for https://www.drupal.org/project/drupal_cms/issues/3486284 📌 Create a new text format/editor to replace core's Basic HTML Active .

    We could split up the recipe by media type. There is not much shared (only enabling responsive_image and the small, medium and large view modes so the base would be small. Shall we go ahead and do that?

    Unfortunately, interacting with the image styles will not be user friendly if editors try to do that, but out of the box this will give them good options to use in their content. These are the common options that we found are presented in competitor CMS.

  • 🇺🇸United States phenaproxima Massachusetts

    @tonypaulbarker and I discussed this recipe on Zoom and how we think it should proceed. Here's what we landed on:

    • We don't really need a media_base recipe. The only things that are shared between images and remote videos are the small, medium, and large view modes. I think it's perfectly okay for those to be duplicated if necessary.
    • Let's have two recipes here: drupal_cms_image and drupal_cms_video.
    • drupal_cms_images should be a maximalist recipe that contains everything needed for images to work as editors would expect them to. That means all image styles, crop types, responsive styles, view modes, etc. are contained in this recipe. It provides both the Image and SVG Image media types. Ideally those would be only one media type, but Tony explained that there are technical limitations around cropping that prevent that from working as we'd like. It's the kind of thing we could evolve in future versions of this recipe.
    • drupal_cms_video will be a wrapper around core's remote_video_media_type recipe, adding the small, medium, and large view modes.
    • The starter recipe will include core's audio_media_type recipe as-is, along with drupal_cms_image and drupal_cms_video.
    • The starter recipe will also expose core's local_video_media_type recipe to Project Browser, for sites that want locally hosted video.

    So, kicking back to "needs work" to reorganize this as we agreed. But truly this is magnificent and robust - I'm amazed by the diligence and thoughtfulness that went into this.

  • 🇬🇧United Kingdom tonypaulbarker Leeds

    @phenaproxima

    Thanks so much for your time it was great to chat this through.

    Just couple of suggested adjustments to the above - let’s call the recipe ‘drupal_cms_remote_video’ to distinguish from the other video media type that uses files
    Shall we make hosted audio from core optional the same way as hosted video? Not everyone is using that either.

  • 🇺🇸United States phenaproxima Massachusetts

    Sure, that sounds good. I'll adjust my comment.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 443s
    #344957
  • Pipeline finished with Failed
    about 1 month ago
    Total: 467s
    #344956
  • Pipeline finished with Failed
    about 1 month ago
    Total: 519s
    #344987
  • Pipeline finished with Failed
    about 1 month ago
    Total: 632s
    #344988
  • Pipeline finished with Failed
    about 1 month ago
    Total: 128s
    #345018
  • Pipeline finished with Failed
    about 1 month ago
    Total: 132s
    #345017
  • Pipeline finished with Failed
    about 1 month ago
    Total: 34s
    #345021
  • Pipeline finished with Failed
    about 1 month ago
    Total: 34s
    #345022
  • Pipeline finished with Failed
    about 1 month ago
    Total: 34s
    #345028
  • Pipeline finished with Failed
    about 1 month ago
    Total: 34s
    #345029
  • Pipeline finished with Failed
    about 1 month ago
    Total: 34s
    #345047
  • Pipeline finished with Failed
    about 1 month ago
    Total: 34s
    #345048
  • Pipeline finished with Failed
    about 1 month ago
    Total: 605s
    #345062
  • Pipeline finished with Failed
    about 1 month ago
    Total: 611s
    #345063
  • Pipeline finished with Failed
    about 1 month ago
    Total: 414s
    #345142
  • Pipeline finished with Failed
    about 1 month ago
    Total: 492s
    #345143
  • Pipeline finished with Failed
    about 1 month ago
    Total: 429s
    #345277
  • 🇬🇧United Kingdom tonypaulbarker Leeds

    Known blockers are resolved.

    We didn't get to complete this yet, because we need to work out how to show these up in CKEditor once they have been added: "The starter recipe will also expose core's local_video_media_type and audio_media_type recipes to Project Browser, for sites that want locally hosted video and audio." I will create a new issue to follow up with that.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1485s
    #345278
  • Pipeline finished with Failed
    about 1 month ago
    Total: 607s
    #345661
  • Pipeline finished with Failed
    about 1 month ago
    Total: 693s
    #345660
  • 🇺🇸United States phenaproxima Massachusetts

    I think this looks really good overall. It's super thorough and the display options it will provide make sense and have a good variety of options for editors. I think the Image recipe will be widely adopted outside of Drupal CMS, thanks to its robustness and coherence. I feel like a thoughtful, careful approach was taken to this architecture, and it shows. Great work!

    The problems, such as they are, seem to mostly be inherited from core's limitations. The coupling to Olivero is a problem, and it means that the recipe needs to outright install and depend on Olivero. Either that, or we need to move all the Olivero-specific stuff into the starter recipe, which will both reduce the usefulness of the Image recipe and blow up the size of the starter -- both bad outcomes. So for the moment, as long as we file a follow-up to fix it, I think it's okay for this recipe to land with a hard dependency on Olivero. IMHO what we need to do is make the image recipe dynamic -- either by a recipe input or a specialized config action -- so that it sets up all the responsive image styles with the proper theme and breakpoints. Chances are we can make that happen with a polyfill module (which we are likely going to need to create anyway, for unrelated reasons).

  • 🇺🇸United States phenaproxima Massachusetts

    This needs a follow-up to discuss how best to decouple of the responsive image styles from Olivero.

  • 🇬🇧United Kingdom tonypaulbarker Leeds

    @phenaproxima one of the options on this issue is to move and improve the breakpoints specifically for Drupal CMS https://www.drupal.org/project/drupal_cms/issues/3488359 Support retina devices and improve responsive image styles on mobile Needs review

  • 🇺🇸United States phenaproxima Massachusetts

    Adding that as a related issue; should we maybe rescope it to be about decoupling the responsive image styles from Olivero altogether? That way we don't need a separate follow-up issue to commit this.

  • Pipeline finished with Failed
    30 days ago
    Total: 552s
    #346736
  • Pipeline finished with Failed
    30 days ago
    Total: 1582s
    #346737
  • Pipeline finished with Failed
    30 days ago
    Total: 603s
    #346751
  • Pipeline finished with Failed
    30 days ago
    Total: 608s
    #346752
  • Pipeline finished with Failed
    29 days ago
    Total: 1457s
    #347281
  • Pipeline finished with Failed
    29 days ago
    Total: 1834s
    #347282
  • Pipeline finished with Failed
    24 days ago
    Total: 385s
    #352279
  • Pipeline finished with Failed
    24 days ago
    Total: 1164s
    #352284
  • Pipeline finished with Failed
    24 days ago
    Total: 1595s
    #352308
  • Pipeline finished with Running
    23 days ago
    #353675
  • Pipeline finished with Failed
    23 days ago
    Total: 1038s
    #353674
  • Pipeline finished with Canceled
    23 days ago
    Total: 373s
    #353685
  • Pipeline finished with Failed
    23 days ago
    Total: 376s
    #353686
  • Pipeline finished with Failed
    23 days ago
    Total: 601s
    #353696
  • Pipeline finished with Failed
    23 days ago
    Total: 808s
    #353695
  • Pipeline finished with Failed
    22 days ago
    Total: 1270s
    #354652
  • Pipeline finished with Failed
    22 days ago
    Total: 1373s
    #354651
  • 🇬🇧United Kingdom tonypaulbarker Leeds
  • 🇺🇸United States phenaproxima Massachusetts

    Looks like there are some outstanding merge conflicts...?

  • Pipeline finished with Success
    19 days ago
    Total: 504s
    #356729
  • Pipeline finished with Failed
    19 days ago
    Total: 652s
    #356730
  • Pipeline finished with Success
    19 days ago
    Total: 493s
    #356744
  • Pipeline finished with Failed
    19 days ago
    Total: 673s
    #356743
  • 🇬🇧United Kingdom tonypaulbarker Leeds

    @phenaproxima current conflicts are resolved. Re-tested some things and all looks well.

  • 🇬🇧United Kingdom tonypaulbarker Leeds
  • 🇺🇸United States phenaproxima Massachusetts

    Re-titling for clarity.

  • 🇺🇸United States phenaproxima Massachusetts

    Only three small points, two of which are probably just an oversight. Then this is RTBC.

  • Pipeline finished with Failed
    19 days ago
    Total: 818s
    #356777
  • Pipeline finished with Failed
    19 days ago
    Total: 861s
    #356776
  • Pipeline finished with Failed
    19 days ago
    Total: 317s
    #356820
  • Pipeline finished with Failed
    19 days ago
    Total: 369s
    #356819
  • Pipeline finished with Failed
    19 days ago
    Total: 682s
    #356823
  • Pipeline finished with Failed
    19 days ago
    Total: 1870s
    #356824
  • 🇺🇸United States phenaproxima Massachusetts

    Follow-ups are opened and this is RTBC to me from a code perspective. Assigning to @pameeela for final validation.

  • Pipeline finished with Failed
    19 days ago
    Total: 861s
    #356852
  • Pipeline finished with Success
    19 days ago
    Total: 971s
    #356853
  • 🇺🇸United States phenaproxima Massachusetts

    I spoke to Pam over Zoom and she authorized me to merge this as-is. She will manually test it once it's in and we can make tweaks as we need to.

    I'm okay with that, so this is a-goin' in.

  • Pipeline finished with Failed
    19 days ago
    Total: 727s
    #356883
  • Pipeline finished with Skipped
    19 days ago
    #356891
  • Pipeline finished with Skipped
    19 days ago
    #356892
  • 🇺🇸United States phenaproxima Massachusetts

    It's been a long road, getting from there to here, but we've finally taken this first, monumental step. Merged into 0.x!

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

Production build 0.71.5 2024