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

Issue fork drupal-2272533

Command icon 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

sqndr’s picture

How 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?

#views-live-preview div.views-row + div.views-row {
/* margin-top: 36px; */
}

See: http://grab.by/x78u

pancho’s picture

Status: Active » Needs review
StatusFileSize
new9.7 KB
new860 bytes
new10.72 KB

The 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:
before
after

Status: Needs review » Needs work

The last submitted patch, 2: view_ui_preview_margin.2272533-1.patch, failed testing.

pancho’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

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

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

lendude’s picture

Version: 8.9.x-dev » 9.2.x-dev
Category: Bug report » Task
Status: Needs work » Needs review
Issue tags: +Bug Smash Initiative
StatusFileSize
new1.97 KB

Cleaning 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.

Madhu kumar’s picture

StatusFileSize
new119.1 KB

Applied 2272533-13.patch now the margin-top in Views live preview reduced
margin

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

rinku jacob 13’s picture

StatusFileSize
new69.49 KB
new69.61 KB

Verified 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

quietone’s picture

Issue summary: View changes

The 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

quietone’s picture

Status: Needs review » Reviewed & tested by the community

I have changed my mind, and despite my comment above, I will set this to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: 2272533-13.patch, failed testing. View results

Gauravmahlawat made their first commit to this issue’s fork.

lendude’s picture

Status: Needs work » Reviewed & tested by the community

Random fail I think? Back to RTBC

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

I'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.

chetanbharambe’s picture

Status: Needs review » Postponed (maintainer needs more info)
StatusFileSize
new430.89 KB
new439.88 KB

Verified 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.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

I'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.

bnjmnm’s picture

Issue summary: View changes
lendude’s picture

Status: Postponed (maintainer needs more info) » Closed (works as designed)

Let'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.