Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This is spun off from #2831936-176: Add "File" MediaSource plugin, specifically this point of review:
I noticed that the "Media" view is inconsistent with the "Content" and "Files" views:
- When no content is available, we don't see the
<thead>
that those other views do show.- The
Media namevalue is in the middle instead of the very first (leftmost) thing.- The button to apply the exposed filters says
Applyinstead ofFilter- Media:
Publishing status, Content:Published status Providershould beMedia sourceor probablySourceGenerally, we should make the "Media" view as consistent with the "Content" view as possible, since that's the canonical/most used one.
Let us address all of problems, here in this issue.
- Media BEFORE
- Media AFTER
- Files
- Comments
- Content
Comment | File | Size | Author |
---|---|---|---|
#5 | Screen Shot 2017-06-08 at 12.40.44.png | 121.72 KB | Wim Leers |
#5 | 2884379-5.patch | 5.52 KB | Wim Leers |
#3 | admin:content:media.png | 112.31 KB | Wim Leers |
#3 | admin:content:files.png | 113.7 KB | Wim Leers |
#3 | admin:content:comments.png | 123.06 KB | Wim Leers |
Comments
Comment #2
phenaproximaComment #3
Wim LeersI wrote that in #2831936-176: Add "File" MediaSource plugin. Adding the screenshots I forgot to add.
Comment #4
Wim LeersThis fixes points 2–5.
Comment #5
Wim LeersOh, and by diffing
views.view.files
andviews.view.media
, I was able to find the solution for point 1.Now the Media view looks like this:
Comment #6
Gábor HojtsyHm, I was surprised the submit button would be labeled Filter and not Apply. We had an issue to change this default in views, but reverted later. But I guess it applies to core views at least. It becomes odd once you have a sort-by form element exposed as well when the things you set are not only filters but also sorts. But core exposed views elements do not include exposed sorts.
Comment #7
Gábor HojtsyReviewed all the changes and apart from the unfounded concern I had above it looks good :)
Comment #9
catchCommitted/pushed to 8.4.x, thanks!
Do we need test coverage for the admin view - I'm assuming the views config-only patch here means we have zero.
Comment #10
Wim LeersIndeed.