Added margin top to lists. Fixes issue #5626

[sam09]

This offers an alternative to #5627

[jhass]

Before

After

As said in #5627 (comment), I would prefer to not have a top margin at the top there, only between two elements of the post.

[sam09]

I think this would fix it.

[jhass]

That doesn't seem to solve the original issue, right?

I also don't like the added indent tbh, it looks odd to me, especially if there's nothing else in the post.

[sam09]

I guess added indent is a bit odd.
As for original issue, isn't it supposed to add a margin to list if its not the first element ? Or, is it supposed to add no margin at all?

[jhass]

I think the desired scheme is

---------------------------------------------------------------------
<no margin>
<element>
<no margin>
---------------------------------------------------------------------
<no margin>
<element>
<margin>
<element>
<no margin>
---------------------------------------------------------------------
<no margin>
<element>
<margin>
<element>
<margin>
<element>
<no margin>
---------------------------------------------------------------------

[sam09]

@jhass please have a look.

[jhass]

Still has the top margin if the list is the first item in the post:

I start to wonder whether you actually have a local test install and try your changes out?

[svbergerem]

@sam09 I think all you have to do is setting the margin-bottom to 0 for ul and ol that are the 4th last child.

[sam09]

What about the case when the list is the only element in the post.

[sam09]

The case when list is the only element in post is a special case of the element being first child and the fourth last child.

[svbergerem]

Ah, ok. Just saw @jhass's comment where he asked for an extra margin top for lists. You set all four margin values (top, right, bottom, left) in this PR which is not necessary. Instead your code could look cleaner if you would

1. set top and bottom margin for ul and ol
2. remove the top margin for ul and ol if it is the first child
3. remove the bottom margin for ul and ol if it is the 4th last child

[svbergerem]

Alright @sam09, could you please squash the commits? Then I will merge this PR.

[sam09]

@svbergerem Is it fine?

[svbergerem]

You added left-margin: 25px. (at least I didn't see that before) Bootstrap already does that for us so there is no need to add it. While removing that line I think you could even refactor your code like this:

ul, ol {
  margin-top:0.8em;
  margin-bottom:0.8em;
  &:first-child { margin-top: 0; }
  &:nth-last-child(4) { margin-bottom: 0; }
}

After you changed that you can update your existing commit with

git commit --amend app/assets/stylesheets/stream_element.scss

[svbergerem]

Thank you!