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 statusGenerally, 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.
| 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.filesandviews.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.