Problem/Motivation

Upon committing #2981044-90: Unify the grid/table views of the media library, Gábor had this to say:

I believe the overall visual coherence of this window needs work (relative placement, visual weights, etc.) but we can refine that later on as well, especially once the dropzone for uploads lands as well.

This is issue is where we can work on that.

Proposed resolution

TBD

Remaining tasks

Determine what needs fixing, visually, and fix it.

User interface changes

Yes; it will get nicer!

API changes

None.

Data model changes

None.

Release notes snippet

TBD; probably none.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

Gábor Hojtsy’s picture

Issue summary: View changes
FileSize
329.28 KB

Some things I noted earlier:

(1) The "Image" tab top white margin/padding seems to be smaller than the bottom padding, also possibly true for the gray tabs, even those that don't have drop shadows dropping on them.
(2) the border of Add files does not line up with anything, eg. maybe same amount of padding should be applied within this tab as is for the tab title(?)
(3) the side of the grid view does not align with the side of the tab content, as evidenced by the grid/tab switcher and the add files border
(4) The top of the grid/table switcher does not align to anyting
(5) The boldness and font size, etc. of the grid/table switcher does not seem to be consistent with anything
(6) The select media button barely stands out of the same gray background, may not be apparent as a button for sight impaired(?) -- why is it not a primary button?
(7) the checkboxes on the images look oddly placed but that is just a nature of the proportion of the images probably and how they get cut

seanB’s picture

Thanks @Gábor Hojtsy! Here is a first response:

1. Nice catch!

2. There is a padding on the tab content which is the same all around the content. I think this is also true for "regular" vertical tabs. Using the same padding as the vertical tab for the top/bottom is probably too little. Not sure if this is a big issue and how to best improve this?

3. Noticed that too, very annoying! The grid items have a fixed width. We should probably make them responsive flexbox items. I've done this in the PoC we made in Barcelona, so I have some code for this already.

4. We chose to align the bottom with the exposed filters to keep them visually linked to the view content. Not sure if this is a big issue and how to best improve this?

5. I think this is the "default" font size and boldness, but we could change this if needed?

6. This is going to be fixed in #3023797: Let users choose what to do after selecting and/or adding items in the media library.

7. Since the aspect ratio of the images can differ we try to center the images on a gray background. Not sure how to best improve this? I'm now also curious what would happen if you upload a very long image like an infographic or something. There are probably UX implications if we change to a fixed width/height and cut off parts of the image, but it would be good to take a look at this.

Gábor Hojtsy’s picture

2. I think is noticable very much here because of the border on Add files (which is IMHO superfluous and gives too much weight to adding BTW and should be done away in favor of a line separating the add from the library, but I defer to UX experts on that). So this border cut into the upper middle of the "Image" word, IMHO its strange. Like we squeezed together the sidebar tabs for some reason and have the tab content a lot more breathing space.

4. Well it does not look like its aligned probably because the dropdown heigh is also not uniform. To my eyes the bottom of the grid/table switcher looked like it is below the bottom of the apply filters button, but it is apparently aligned. Maybe it looks like its below because of the darker color. What happens if you add a bunch more filters, what would jump where?

5. It may be the same font size as "Apply Filters" (which is oddly title cased, now that I look at it, unlike everything else on the dialog including its title, the tabs, the select media button or the dropdown options :D) but the Apply Filters have a container that elevates it.

seanB’s picture

Gábor Hojtsy’s picture

Changing the button from "Apply Filters" to "Apply filters" would also need to be done btw :) Not sure if that fits into a process somewhere in an existing issue or needs a new one.

seanB’s picture

Not sure if we have any issues touching the view at the moment, so I guess we need to file a new issue for #6.

To summarize, I think we still need issues for:

  • Improve design of grid/table switcher (alignment, font size/boldness)
  • Alignment of the checkbox on the gray background of the grid items
  • Changing the button from "Apply Filters" to "Apply filters".
Gábor Hojtsy’s picture

+1 to #7.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

starshaped’s picture

Assigned: Unassigned » starshaped
starshaped’s picture

Per issue #7:

1. Updated the font size to 15px for the grid/table switcher links. I also added a bold styling to the actively selected item. Finally, I adjusted the alignment of the grid/table links to align better with the 'Apply filters' button.
2. I made a slight adjustment to the checkbox alignment within media items. I also added the appropriate RTL style for this.
3. I updated the view configuration to change the button to 'Apply filters'.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

CSS looks great to me! Thanks, @starshaped. Also discussed with Gàbor and he gave this his blessing. Let's get it in.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 3037668-11.patch, failed testing. View results

starshaped’s picture

starshaped’s picture

Status: Needs work » Needs review
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me. RTBC once green!

lauriii’s picture

Assigned: starshaped » Unassigned
Issue tags: +Needs followup
+++ b/core/modules/media_library/css/media_library.module.css
@@ -52,10 +52,15 @@
+[dir="rtl"] .media-library-item .js-click-to-select-checkbox {

We shouldn't use js- prefixed classes for styling. Since this is just adding RTL styles for a pre-existing selector, we should probably just open a follow-up for this.

phenaproxima’s picture

Gábor Hojtsy’s picture

1. Does this patch complete the remaining items of the issue that were not resolved yet elsewhere?
2. Would be really useful to see before/after screenshots if possible.

Gábor Hojtsy’s picture

Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Fixed

We can keep improving this in further issues if we need to. Thanks all for breaking out issues and solving the remaining items.

Gábor Hojtsy’s picture

BTW I fixed this on commit:

core/modules/media_library/css/media_library.theme.css
 250:3  ✖  Expected "font-size" to come before "line-height"      order/properties-order

  • Gábor Hojtsy committed d0f0aba on 8.8.x
    Issue #3037668 by starshaped, Gábor Hojtsy, phenaproxima, seanB, lauriii...

  • Gábor Hojtsy committed 0c16a45 on 8.7.x
    Issue #3037668 by starshaped, Gábor Hojtsy, phenaproxima, seanB, lauriii...

Status: Fixed » Closed (fixed)

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