[openstreetmap/openstreetmap-website] add vertical margin to changeset_more div (#2717)

Andy Allan notifications at github.com
Sun Jul 19 13:30:51 UTC 2020


@gravitystorm requested changes on this pull request.



> @@ -938,6 +938,10 @@ tr.turn:hover {
   .comments-0 {
     opacity: 0.5;
   }
+  
+  .changeset_more {

We're moving away from having custom CSS rules, especially 'one-time' rules like this which are very hard to correlate between the view and the CSS files (and vice versa). When the CSS refactoring is finished, the only CSS we should have (if any) will apply to entire types of elements (e.g. all submit buttons), not individual elements on individual pages.

Where you need to make specific spacing changes to just one situation, then better approach is to use bootstrap margin and padding utilities, directly on the affected element in the specific view template. 

https://getbootstrap.com/docs/4.4/utilities/spacing/

Here's an example to show what it might look like: https://github.com/openstreetmap/openstreetmap-website/blob/65b984f90b51eae53b6c969cabf617eeab08b8ae/app/views/site/help.html.erb#L11

This means if the view template is refactored, no unused CSS lingers in the stylesheet, and it's obvious that a specific override applies without having to check every class in the view to see if there's a matching class in the stylesheet.

But actually, the best situation is where we have no need for any margin or padding overrides at all. This can normally be achieved by reworking the HTML, since Bootstrap almost always gives good spacing out-of-the-box. So where elements are too close together, or too far apart, it's often because the HTML isn't structured very well, for example having a mixture of text in paragraphs and other text directly in container divs. So I would suggest reviewing the HTML first, to see if there is better structure, and only using the spacing utilities as a last resort.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/2717#pullrequestreview-451131870
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20200719/5a2e2e9a/attachment.htm>


More information about the rails-dev mailing list