Media and media library

"To start, we need screenshots of all the pages/workflow we can have on Media to plan the components."

Sounds like a great task for first-time contributors who are getting to know the issue queues. Adding the 'novice' tag. Also, for keyword search, adding #D4D19

I am attaching some of the screenshots for media and media library pages/workflow in claro experimental theme. Please review.
I hope it will be helpfull.

I am attaching screenshots of adding Media in a node page (single and multifield). Attached is a zip file and a word document referencing the workflow. When we break out the tickets later, I will add the screenshots individually.

In a separate email I will add the Media Library screenshots.

Attached is the Zip drive with the Media Library screenshots and reference doc. These were taken with the Seven theme in 8.8.0 alpha 1.

https://www.drupal.org/files/issues/2019-11-04/Media%20Library%20Screens...

Here is a list of the samples taken:

1a - Seven - Media Library – landing – table view – desktop (with unselected, selected and hover states)
1b - Seven - Media Library – landing – table view – mobile - with unselected and selected states on 2 rows
1c - Seven - Media Library – landing – grid view – desktop
1d - Seven - Media Library – landing – grid view – mobile
2a - Seven - Media Library – landing – table view – desktop (after media attached with success message
2a - Seven - Media Library - add media options – mobile (similar on desktop)
3a - Seven - Media Library – add document – desktop
3b - Seven - Media Library – add image – mobile
3c - Seven - Media Library – add image – mobile
3d - Seven - Media Library - add image metada – mobile
3f - Seven - Media Library – landing – table view – desktop - after img attached success msg
4a - Seven - Media Library – edit image – desktop
4b - Seven - Media Library – edit image – mobile
4c - Seven - Media Library – delete popup – desktop
4d - Seven - Media Library – delete popup – mobile

For entity reference media field styles are missing in popup

I checked issues that was displayed in the https://www.drupal.org/project/drupal/issues/3062751#comment-13457719. Now popup is fine. I fixed buttons positions as well.

Some new screenshots

.
├── 1080p Full HD Television
│   ├── Add media.png
│   ├── Add or select Media.png
│   └── Table Add or select Media.png
├── 4K Ultra HD Television
│   ├── Add media.png
│   ├── Add or select Media.png
│   ├── Select Media.png
│   ├── Table Add or select Media.png
│   └── Table Select Media.png
├── Laptop
│   ├── Add media.png
│   └── Media.png
└── Others
    ├── Add or select media Audio.png
    └── Add or select media Remote Video.png

Here is another screenshots after applying #3023311 patch:

• #3023311: Modal dialog style update

Based on #23, making this a feature request, not a plan.

Here's a patch that covers a lot of the Media Library designs, but it depends on and incorporates the work being done in #3083256: Create smaller variations for form elements (so I think both will need to be applied to test this patch), and defers to bulk operations issues in favor of #3070558: Implement bulk operation designs.

Just did a quick pass of the patch. Will install and do a deeper review soon.

1. +++ b/core/themes/claro/claro.libraries.yml
   @@ -185,6 +185,12 @@ checkbox:
      dependencies:
        - core/drupal
   +    -
   +icon-link:
   +  version: VERSION
   

   Stray dash at 189

2. +++ b/core/themes/claro/src/ClaroPreRender.php
   @@ -25,8 +25,9 @@ public static function managedFile($element) {
   +    // Wrap single-cardinality widgets with a details element. Do not wrap Media
   +    // Library upload element.
   +    $single_file_widget = empty($element['#media_library_upload']) && !empty($element['#cardinality']) && $element['#cardinality'] === 1;
   

   It may be helpful to give the #media_library_upload property a more generic name that more closely describes its purpose, since this could be useful in non-media-library situations. A good name could eliminate the need to add comments to when it is added, too.

3. +++ b/core/themes/claro/claro.theme
   @@ -1291,15 +1294,21 @@ function claro_form_media_library_add_form_oembed_alter(array &$form, FormStateI
   -    $item['value']['remove_button']['#attributes']['class'][] = 'media-library-add-form__remove-button';
   +    $item['value']['remove_button']['#attributes']['class'][] = 'media-library-add-form__remove-button button--extrasmall';
   

   Add each item to the class array on a separate line. This happens in a few spots.

4. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -196,4 +196,22 @@
   +
   +  /**
   +    * Vertical Tabs.
   +    */
   +  --vertical-tabs-margin-vertical: var(--space-s);
   +  --vertical-tabs-border-radius: var(--details-accordion-border-size-radius);
   +  --vertical-tabs-shadow: var(--details-box-shadow);
   +  --vertical-tabs-border-color: var(--details-border-color);
   

   In the variables pcss file, we're not adding whitespace above the comments.

5. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -31,65 +34,149 @@
   +.media-library-menu__item {
   +  overflow: hidden;
   +  margin: calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -2) calc(var(--vertical-tabs-border-size) * -1) calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -1) calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -1); /* LTR */
   +  padding: var(--vertical-tabs-menu-item-shadow-extraspace) 0; /* LTR */
   +}
   +
   +[dir="rtl"] .media-library-menu__item {
   +  margin-right: calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -1);
   +  margin-left: calc(var(--vertical-tabs-border-size) * -1);
   

   There are some missing units in these calc() calls.

6. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -31,65 +34,149 @@
   -  border-left: 1px solid #fcfcfa;
   +  z-index: 3; /* The selected menu link should be on a higher level than the white masking line that hides the grey separator. */
   +  color: var(--color-absolutezero);
   

   The various z-indexes are a good place to use css variables and calc, since their individual values are less important than how they differ from each others

7. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -273,6 +384,37 @@
   + * Ajax throbbers inside a media library item.
   + */
   +.media-library-item .ajax-progress.ajax-progress.ajax-progress {
   +  position: absolute;
   

   If a class is being tripled for what I assume is specificity, it should be noted what it's up against in order to necessitate this. This may help others find a solution that requires less specificity or indicate the need for a followup for whatever is forcing the hoop jumping.

This patch addresses #3062751-27: Media and media library. On #5 and #6, these are mostly copied from vertical-tabs.pcss.css, and I've moved one of the complex calcs and z-index values to variables. Maybe more should be consolidated?

I've combined the patch in #3083256-39: Create smaller variations for form elements so this can be reviewed as it is intended.

The previous patch was incomplete, so disregard #28. I'm adding an interdiff too.

1. +++ b/core/themes/claro/claro.theme
   @@ -1291,15 +1294,21 @@ function claro_form_media_library_add_form_oembed_alter(array &$form, FormStateI
   +  $variables['attributes']['class'][] = 'form--small';
   

   This doesn't seem to target every possible form that can appear in a media library dialog. For example, if you use media_library_test the upload widget on "Type Three" and the embed input for "Type Five" appear in their default sizes

2. +++ b/core/themes/claro/claro.theme
   @@ -1291,15 +1408,22 @@ function claro_form_media_library_add_form_oembed_alter(array &$form, FormStateI
   +    // Set this flag so we can remove the details element.
   +    $fields[$source_field_name]['widget'][0]['#media_library_upload'] = TRUE;
   

   Looks like this got missed in the property renaming.

3. +++ b/core/themes/claro/css/base/variables.pcss.css
   @@ -196,4 +203,23 @@
   +  --vertical-tabs-menu-item--margin: calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -2) calc(var(--vertical-tabs-border-size) * -1) calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -1) calc(var(--vertical-tabs-menu-item-shadow-extraspace) * -1);
   

   This should get separated into variables for top, right, bottom, left since they are all different. The refactoring based on these new variables should then include the RTL rules

4. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -4,9 +4,11 @@
   +  margin: -1em -1.5em -1em -1em;
   

   Right and left have different values, so an RTL version is needed. On RTL the left edge of the form is not flush with the modal container.

5. The next few deal with taming some margins which work fine on a page, but more of an issue in the limited space provided by dialogs.

   There's some unneeded top margins when in the meta form list after adding media
   

6. When dragging a media item to reorder it in the list, the focus outline is partially obscured. This isn't noticeable in Seven because it does not receive an outline -- but technically should since it is focused
7. The upload widgets have a top and bottom margin that seem unnecessary
   
8. Unnecessarily large margin above status messages in the dialog.
   

I think I've addressed everything in #30 except for #1. I'm having trouble duplicating that and may need more information. I removed some top and bottom padding on #5, but we are seeing different things, so I'm not sure if it is addressed.

1. Here's an example of what I was describing in #30.1 and how it'll look with the small variant.
2. The reduced padding looks better. When there are multiple items I notice the remove button is aligned differently than the others. It would also look nicer if the label of the first form element is aligned with the remove button.
   

Thanks for the detail on #1! I think this should cover it.

This also aligns the remove button per #2, and removes the first child exception on the button position.

I noticed that the remove button now overlaps at smaller widths, so per a conversation with @bnjmnm, I tried replacing the remove button with an icon button, which I think is even more consistent with the Media Library styling.

Before:

And after:

This should make the modal close and the remove buttons align.

It seems like the ajax throbber is not visible when media library item is being removed.

Would be great to get confirmation from an UX expert on the change in #35. The icon looks a bit similar to the close dialog button, so it might be possible that they get mixed, especially given the positioning on the first item. As an alternative solution, we probably could just move the bfsmallutton up so that it doesn't overlap with the input.

I'm also adding a reference to #3083256: Create smaller variations for form elements since this issue has a dependency on that.

Would it be possible to get a version of the patch that doesn't include changes from #3083256: Create smaller variations for form elements which would make reviewing easier?

Here it is with the small forms patch removed, and I changed the z-index so the throbber is back.

trying this again + interdiff

After the community Claro meeting and discussion with @ckrina, @lauriii, and others, I've gone back to the prior remove button, but aligned it with the save button, and adjusted the focus border a little so it isn't too close to the buttons.

#40 was no longer applied so rerolled again. Not able to create interdiff as #40 was failed to apply.

I have applied the #41 it seems that save button is not properly aligned with remove button . Added before and after screenshots for the same.

That scrollbar looks like it's an IE issue. I think this should take care of it.

Updating after our Claro sprint conversation.

I've checked the blue border colors around checked and .is-hover states, and the colors are currently the default blue for active and the correct hover color, so this may need additional follow up.

I've added a border-radius and box-shadow on the appropriate elements on the media library thumbnails on the node form, which is a subtle but nice improvement, IMO, so thanks @ckrina for spotting it.

I've removed the green focus border that appears when thumbnail items are clicked on the node form, [edited] and this is not the right answer, but I'm still working on it. :)

I'm also attaching a version of the patch for manual testing that includes the patch in #3083256: Create smaller variations for form elements, as this patch depends on it.

Oops, this needs a re-roll, but the manual testing patch should apply as-is.

I'm also adding some supporting screenshots.

1. I forgot to mention that I also changed the remove button padding. It's now 1.5rem of left padding, and 0.5rem on the background position, like we discussed on the call.

2. The border-radius and box-shadow on these look much more polished, IMO.

3. The blue border on the checked state is currently --color-absolutezero: #003cc5 and the hover state is --color-absolutezero-hover: #0036b1. I find these hard to distinguish, and this may need more discussion on a future call or in Slack.

#45.3: We could use lighter shade of blue. For example, #b3c9f5 "focus". I don't think hovered items should use color that close to the checked items because it could create confusion around which items are checked.

A patch for the #45.3 @laurii comment.

When hovered + focused, should we prioritize the focus border color or the hover border color?

I think the lighter blue border works, but I think the checkbox background should match, so I picked a different lighter blue that I think might be easier to see because it's not quite so light.

This patch also addresses the green focus border padding problem.

trying that again so it's not missing the icons.

This one should apply. The previous manual testing patch should apply manually for testing.

I'm removing a remnant of another patch from the claro.theme file and some comment changes in the compiled css files, as well as a small styling change to the Media Library grid view action buttons.

I updated the manual testing patch, which includes the small form elements patch on issue #3083256.

I gave this a quick manual test and I think it. looks. great. It takes everything that is good about the media library in Seven and transfers it into Claro -- a big step forward towards Claro being the default admin theme in Drupal.

I observed a few things while testing, but I'm guessing they are all out of scope. Maybe some could use follow-ups, maybe others don't need it.

First, after adding a couple of media items in the media library, I saw that they are not the same width in the administrative grid view, and are a little small at that:

That said, it was still clear and usable, so maybe not a big deal.

I also noticed that, when selecting media items in the administrative table view, the selected rows are highlighted, like so:

However, selecting media items in the media library's table view does not highlight them:

There is also no column click-sorting for this table, but that's definitely out of scope here and I think it's due to a known bug in Views, for which a dedicated issue exists.

This next thing is probably also well out of scope. But, when I add a media item in the media library, focus is returned to a...weird effin' place. Halfway between the gutter and the stars:

I highly doubt Claro is responsible for this, nor am I sure that Claro can do anything about it even if it is causing it, but I thought I'd call it out, since I'm actively trying to turn up issues. :)

Finally -- and I'm pretty sure this isn't something Claro is doing, but Claro might have control over it to some extent -- the phrasing of "0 of 1 item selected" is weird. Shouldn't that be "items"? Or is that weird too?

I believe there is a dedicated issue out there to make this text themeable, if it's not already. In which case, once it lands, Claro could certainly improve on this if it wanted to :)

So there you go. Them's my findings. I think these are all pretty minor, and most likely outside of Claro's control. The current patch is still head-and-shoulders above what's in HEAD, so I'm +1 for getting it landed as soon as possible!

Thanks for the thorough review. :)

I'm unable to duplicate the first point in #54. This is what I see:

Seven and Claro both show the inconsistent background color described in the second point, as the selected Media Library item is not given the selected class. I think this is a Media Library issue, rather than a Claro issue, so I opened a new issue. #3169841: Media Library selected row background color is inconsistent with Media table view.

And I believe all the remaining points are also present in both Seven and Claro, so out of scope as mentioned.

Let's open follow-up issues for the issues discovered in #54.

I opened a new issue for the focus location feedback: #3170150: Media library focus returns to unexpected place after media upload.

It looks like the question of item/items was resolved in this issue: #3033943: MediaLibraryWidget selected items count is not conveyed to assistive tech correctly
I think we're seeing the intended behavior.
+1 to future themeability!

Did a bit of manual testing and reviewed the code. Overall this looks great! Here's couple of things I noticed:

1. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -104,25 +199,62 @@
   +/* This Media Library form is an exception to the extrasmall button pattern. */
   

   Can we expand this comment to explain why this is needed?

2. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -104,25 +199,62 @@
   +.media-library-add-form--upload.media-library-add-form--with-input .form-managed-file_meta {
   +  margin-top: 0;
   +}
   

   Is this rule needed because it seems like there's a typo in the selector because .form-managed-file_meta should probably be .form-managed-file__meta?

3. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -104,25 +199,62 @@
   +.media-library-add-form--upload.media-library-add-form--with-input .form-managed-file__main,
   +.media-library-add-form--upload.media-library-add-form--with-input .form-managed-file.no-upload {
   +  display: block;
   +}
   

   How can I reproduce this scenario?

4. 
   The remove button icon should use the same gray color as the text when the button is disabled. This state is applied to the button when the ajax request to remove an item is being processed.

1. Done, good idea.
2. It's not needed. Good eye. :)
3. You can see this after uploading a new file to the media library.

before:

after:

4. Changed.

1. Done, good idea.
2. It's not needed. Good eye. :)
3. You can see this after uploading a new file to the media library.

before:

after:

4. Changed.

I'm leaving out the patch that incorporates small form elements for now, as that issue has diverged a bit from the changes in this patch. The form size variations will need to change based on the outcome of #3083256: Create smaller variations for form elements

Looks great, thanks so much for your attention to detail. The media library needs these improvements. I have already added css to override claro in a recent project and with these improvements I can back off my custom css overrides.

Thanks for the review @joseph.olstad! Moving this to postponed for #3083256: Create smaller variations for form elements. This can be moved back to RTBC after it has been committed. 😇

Reroll in #63 was missing a few files

Found a few small things

1. +++ b/core/themes/claro/css/theme/media-library.css
   @@ -41,68 +53,164 @@
   +  padding: 0.5rem 0; /* LTR */
   

   This has an /* LTR */ without a corresponding RTL style

2. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -4,9 +4,21 @@
   +  margin: -1em -1.5em -1em -1em;
   

   Needs /* LTR */

3. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -14,16 +26,17 @@
    .media-library-menu {
   +  position: relative;
      display: block;
   -  width: 600px;
   -  max-width: 20%;
   -  margin: 0; /* LTR */
   -  padding: 0;
   -  border-bottom: 1px solid #ccc;
   -  background-color: #e6e5e1;
   -  line-height: 1;
   +  float: left; /* LTR */
   +  width: var(--vertical-tabs-menu-width);
   +  margin: 0;
   +  padding-top: var(--vertical-tabs-menu-item-shadow-extraspace);
   +  list-style: none;
   +  color: var(--color-text);
    }
    [dir="rtl"] .media-library-menu {
   +  float: right;
      margin: 0;
    }
   

   The floats don't seem to be needed since they are wrapped in a flex container. Maybe I'm missing something?

4. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -31,65 +44,147 @@
     */
   

   This comment can be removed. It's using a class instead of the li element.

5. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -104,25 +199,62 @@
   +/* This Media Library form is an exception to the extrasmall button pattern.
   + * Additional padding is needed to accommodate the remove button icon. The
   + * margin is adjusted for alignment within the media library dialog.
   + */
   

   Multiline comments should be formatted a bit differently https://www.drupal.org/docs/develop/standards/css/css-formatting-guideli...

6. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -104,25 +199,62 @@
   +  /* background: #fcfcfa; */
   

   Commented out declaration should be removed.

1. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -273,6 +404,37 @@
    /**
    * The media library item container receives screen reader focus when items are
    * removed. Since it is not an interactive element, it does not need an
   

   This comment seems like it may be outdated. From what I can tell the focus is no longer hidden, but displayed in a child element.

2. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -666,20 +847,20 @@
   +  right: 0;
   

   Needs an /* LTR */

Thanks!

65.1 Removed, unnecessary.
65.2 Added.
65.3 Correct, not necessary.
65.4 Removed.
65.5 Fixed.
65.6 Removed.
66.1 Replaced.
66.2 Added.

As an alternative to postponing on #3083256: Create smaller variations for form elements, I'd like to propose considering a patch with the references to .form--small removed. I've included such a patch here. This would allow for faster compliance with the Claro designs in the Media Library interface. We could then open a separate issue to then incorporate smaller form elements when that issue is resolved.

In my testing of this patch, I do not see any regressions from the current Media Library implementation.

I think it's fine to move forward even though #3083256: Create smaller variations for form elements is not done yet. Let's just add some updated screenshots to the issue summary and open a follow-up to make a decision how media library will utilize small form elements.

I've updated the issue summary with new screenshots, and the follow-up is here: https://www.drupal.org/project/drupal/issues/3177181

Patch needs reroll

1. +++ b/core/themes/claro/css/theme/media-library.css
   @@ -13,7 +13,18 @@
    .media-library-wrapper {
   
   +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -4,9 +4,21 @@
    .media-library-wrapper {
   ...
   +  margin: -1em -1.5em -1em -1em; /* LTR */
   

   These can get changed to rems for consistency (I don't believe it depends on it being em), and would benefit from a comment explaining that these negative margins exist to both offset the dialog padding AND compensate for positioning that would otherwise hide the active tab indicator on the left.

2. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -14,82 +26,159 @@
     */
   

   This little deal here is part of a comment that is a @todo that is not relevant to Claro (and it references a closed issue), so that should get removed. To find more easily, this is the comment to remove

   /**
    * @todo Reuse or build on vertical tabs styling for the media library menu.
    *   https://www.drupal.org/project/drupal/issues/3023767
    */

3. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -104,25 +193,62 @@
   +  align-items: center;
   

   Because of this addition there's a rule + comment a bit further down that appears to be no longer needed. Would be good to double check that.

   /**
    * @todo Remove .button when styles are moved to the seven theme in
    *   https://www.drupal.org/project/drupal/issues/2980769
    */
   .button.media-library-add-form-oembed-submit {
     align-self: center;
   }

4. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -523,14 +704,15 @@
    }
    
   

   There's another set of styles that aren't needed just prior to here. These styles are the same as default button styling, so they + the @todo can be removed.

5. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -590,8 +770,15 @@
   +  background-image: url("../../../../misc/icons/545560/pencil.svg");
   

   Item not found at this address

6. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -600,17 +787,20 @@
   +  background-image: url("../../../../misc/icons/545560/ex.svg");
   

   Item not found at this address.

7. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -623,26 +813,10 @@
    .media-library-add-form__media {
      position: relative;
      display: flex;
   -  padding: 1em 0;
      border-bottom: 1px solid #c0c0c0;
      outline: none;
    }
   

   The outline rule in this style is not relevant to Claro for multiple reasons and can be removed. Among the reasons it isn't relevant: focus rings in Claro include box-shadow, so outline: none wouldn't address it | another big one: we also shouldn't be hiding a focus ring from anything that receives focus anyway.

   The comment above The added media container receives screen reader focus since.... can also be removed.

8. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -666,20 +840,20 @@
   +  background-image: url("../../images/icons/8e929c/ex.svg");
   

   No image at this path.

1. Changed and comment added.
2. Removed.
3. Yep, no longer needed, so removed.
4. Removed.
5., 6. , and 8. It looks like the new files were left out of the reroll, so three icons and the two icon-link css files. Added here.
7. Removed.

Thank you!

Hope the repeated kickbacks aren't too discouraging 😊, it's tough with larger patches.

1. --- /dev/null
   +++ b/core/misc/icons/545560/ex.svg
   
   +++ b/core/misc/icons/545560/ex.svg
   @@ -0,0 +1 @@
   
   @@ -0,0 +1 @@
   diff --git a/core/misc/icons/545560/pencil.svg b/core/misc/icons/545560/pencil.svg
   
   diff --git a/core/misc/icons/545560/pencil.svg b/core/misc/icons/545560/pencil.svg
   new file mode 100644
   
   new file mode 100644
   index 0000000000..2808993ddf
   
   index 0000000000..2808993ddf
   --- /dev/null
   
   --- /dev/null
   +++ b/core/misc/icons/545560/pencil.svg
   
   +++ b/core/misc/icons/545560/pencil.svg
   @@ -0,0 +1 @@
   

   I ran a few of the added SVGs through the optimizer at https://jakearchibald.github.io/svgomg/ to confirm they weren't already fully optimized. Both of them can have their filesize reduced by using that service.

   In other recent Claro issues, we've been adding Claro-only SVGs to the Claro images directory instead of misc/icons. It may be worth a second opinion, but since there's already a mix of core and claro SVGs requested in this file, keeping ones only used by Claro in Claro is a nice way to know what items can participate in a move/remove of Claro several years from now.

2. +++ b/core/themes/claro/claro.theme
   @@ -1370,11 +1373,15 @@ function claro_preprocess_item_list__media_library_add_form_media_list(array &$v
        $item['value']['preview']['#attributes']['class'][] = 'media-library-add-form__preview';
        $item['value']['fields']['#attributes']['class'][] = 'media-library-add-form__fields';
        $item['value']['remove_button']['#attributes']['class'][] = 'media-library-add-form__remove-button';
   -
   +    $item['value']['remove_button']['#attributes']['class'][] = 'button--extrasmall';
        // #source_field_name is set by AddFormBase::buildEntityFormElement()
        // to help themes and form_alter hooks identify the source field.
        $fields = &$item['value']['fields'];
   

   The whitespace that got removed here was intentionally there, to precede the comment block beginning with #source_field_name is...

3. +++ b/core/themes/claro/claro.theme
   @@ -1370,11 +1373,15 @@ function claro_preprocess_item_list__media_library_add_form_media_list(array &$v
   +    // Set this flag so we can remove the details element.
   

   This comment isn't accurate, the flag doesn't remove the details element, it prevents it from being added in \Drupal\claro\ClaroPreRender::managedFile

4. +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -4,92 +4,181 @@
   -.media-library-menu__link:active,
   
   @@ -298,6 +441,22 @@
   +.media-library-item--grid:focus > article {
   +  outline: var(--focus-outline);
   +  box-shadow: var(--focus-box-shadow);
    }
   

   Lets add a class in media--media-library.html.twig so we can use a BEM selector instead of > article (and you'll need to move the template and update ConfirmClassyCopiesTest)

@katherined , great work on this patch.

I looked into the comments by bnjmnm ,

1. Did the optimisation (optimised svg images) however I did not move the .svg images to the claro folder.
   Reason: it was suggested to move these icons from core/misc/icons/545560 to core/themes/claro/images/icons however I noticed there already is an ex.svg image in the core/themes/images/icons/545560 folder , so to move the ex.svg to this folder if it is a different file we'd have to rename it. pencil.svg was not present in that folder however so we could easily move that.
   with that said, @katherined approach is consistent with the rest of the pcss.css /.css
2. Done see interdiff
3. Done see interdiff
4. REMAINING TODO:
   

   +++ b/core/themes/claro/css/theme/media-library.pcss.css
   @@ -4,92 +4,181 @@
   -.media-library-menu__link:active,
   
   @@ -298,6 +441,22 @@
   +.media-library-item--grid:focus > article {
   +  outline: var(--focus-outline);
   +  box-shadow: var(--focus-box-shadow);
    }
   

   add a class in media--media-library.html.twig so we can use a BEM selector instead of > article (and you'll need to move the template and update ConfirmClassyCopiesTest)

unfortunately for me to jump in and help out here more than this is difficult for me because I'm not familiar with the core standards for pcss.css and I don't know if my version of compass even recognizes these files for compiling.

With that said, I did do as many changes as I could here for the svg optimisation and have attached the interdiff here for the other minor changes.

Optimizing the svg images, this part is fairly easy using the hyperlink provided by @bnjmnm https://jakearchibald.github.io/svgomg/
however @bnjmnm, what options should be selected? just the default options? I used the default options.

I hope either @katherined or @bnjmnm can drive this one home , I pitched in a bit hope this helps.

I think we really need these changes and there's been a lot of great work here, really rooting for the team win here. Thanks and good luck.

I went ahead and moved the pencil icon to the claro directory, but left the new ex variants in the core directory to avoid having two ex variants in Claro, but that may require some more thought, possibly in a follow up issue?

I addressed the BEM point in #78-4 as well.

Thank you @bnjmnm and @joseph.olstad for your help!

This also required a reroll, so no interdiff this time.

I think this is ready. During this last review all I spotted were potential refinements maybe be worth discussing, but none that need to happen here. This is a clear + significant improvement to the Media Library UI that should be made available to users ASAP. If minor adjustments are desired, they can happen in a future issue.

Pushed to 9.2.x and 9.1.x.

Thanks @effulgentsia and everyone else (especially @katherined and @bnjmnm)!