Problem/Motivation
Excessive margin-top in Views live preview
Steps to reproduce
Proposed resolution
Remove the margin from the preview
Remove the media library specific margin removal in core/themes/claro/css/theme/media-library(.pcss).css as
removing the margins from previews overall would take care of this without the need for this targeted styling.
.views-live-preview .media-library-view div.views-row + div.views-row {
margin-top: 0;
}Remaining tasks
Review
Commit
User interface changes
Only for the views preview
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | After Patch 2272533.png | 439.88 KB | chetanbharambe |
| #23 | Before Patch 2272533.png | 430.89 KB | chetanbharambe |
| #16 | afterpatch.png | 69.61 KB | rinku jacob 13 |
| #16 | beforepatch.png | 69.49 KB | rinku jacob 13 |
| #14 | view-margin-top.png | 119.1 KB | Madhu kumar |
Issue fork drupal-2272533
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
sqndr commentedHow to recreate the issue?
- Create a few articles.
- Create a new, default (page) view. Only show content type article.
- When the view is created, check the Auto Preview. The titels of the articles are displayed with a lot of margin in between each item.
- On the page actual page, there's not that much margin between the elements.
Problem?
See: http://grab.by/x78u
Comment #2
panchoThe margins are necessary when rendering whole articles or teasers, but not in field-based views.
So I'm targeting view-mode-teaser and view-mode-default instead. We can't target article because depending on theme settings, user pictures might be nested as articles, too.
See before and after:


Comment #4
pancho2: view_ui_preview_margin.2272533-1.patch queued for re-testing.
Comment #13
lendudeCleaning up old bugs.
It's changed to 2.25em since this was opened, but still translates to about the same margin.
I'm not bothered by this, since the preview is rendered in the admin theme and the resulting View might not be, there are bound to be (massive) differences between the two anyway.
So in that light, I would argue that we shouldn't reduce the margin here, but we should just take it out completely. I don't see a reason to purposefully make it render different in the Preview.
This patch just removes the margin from the base CSS and Claro, haven't touched Stable.
Comment #14
Madhu kumar commentedApplied 2272533-13.patch now the margin-top in Views live preview reduced

Comment #16
rinku jacob 13 commentedVerified and tested patch#20 on the drupal 9.3.x-dev version. Patch applied successfully and looks good to me.Adding screenshot for the reference.thanks @Lendude
Comment #17
quietone commentedThe patch still applies to 9.3.x. I manually tested and it works as expected. Like is said in #13 there is no problem with the margin being different in the preview.
Added an issue summary.
I would RTBC this but the change is all css. RTBC+1
Comment #18
quietone commentedI have changed my mind, and despite my comment above, I will set this to RTBC.
Comment #21
lendudeRandom fail I think? Back to RTBC
Comment #22
alexpottI'm not sure this is a good change. I played around with this by changing a view to use a grid and when there were enough items to make it use multiple rows the fact there was no margin made it look very very odd. OTOH we don't have any margin for the columns so meh. I dunno. It really depends what's in your views. I do think that in quite a few cases this will render the live preview harder to look at and so I think we should re-consider removing the margin completely. Tricky.
Gonna set to needs review to elicit a few more thoughts.
Comment #23
chetanbharambe commentedVerified and tested patch #13.
Patch applied successfully and looks good to me.
Testing Steps:
# Goto: Appearance -> Apply Seven theme
# Create a few articles.
# Create a new, default (page) view. Only show content type article.
# When the view is created, check the Auto Preview.
# The titles of the articles are displayed with a lot of margin in between each item.
# On the page actual page, there's not that much margin between the elements.
Expected Results:
# After applying patch #13, the User is not able to see lots of margin in between each sitem.
Actual Results:
# Currently, The user is able to see titles of the articles are displayed with a lot of margins in between each item
Note: I agree with alexpott,
The margin is not appearing after applying the patch but UI is not looking good so I think this issue should be moved into Maintainer needs more info.
Please refer attached screenshots for the same.
Looks good to me.
Can be a move to Maintainer needs more info.
Comment #27
bnjmnmI'm also uncertain about the benefits of removing the margin, but I do see a benefit of keeping it there. Any view built for the public facing part of a site will likely look quite different in preview already, as it's using the admin theme instead of the default theme where it will be displayed. The additional margin provides a clear means of distinguishing between rows without having to introduce view-specific styling in the admin theme for a public facing view.
I also think that while there are instances where the additional margin can look a little rough, removing it will potentially introduce other less than ideal results such as the one mentioned in #22. I'm leaning towards not doing this, but if there were multiple before/after screenshots of different use cases and view types demonstrating this results in a clear improvement without unwanted tradeoffs, I'd be more open to the addition.
Comment #28
bnjmnmComment #29
lendudeLet's close this then. I'm certainly not bothered by the margin and if people see a benefit of it, great, let's keep it in. There are way too many permutations of possible views to really ever say there will be no unwanted trade offs, I think, so might as well stick with the trade off we have now.
We could go back to the solution in #2 but trying to target view modes sounds very bothersome, since these can just be created and then you'd have a difference in margin when flipping between different view modes if some are supported by the selector and some aren't, which also sounds less than ideal.
Since the preview is more about previewing your result set and not previewing how it's going to look, I'd opt not to spend too much time on this and just leave this as it is, until something like #2684509: [meta] Implement a preview-first Views UI with current functionality lands.