zulip / zulip-flutter

Future Zulip client using Flutter
Apache License 2.0
159 stars 134 forks source link

Sibling layout interactions in content, like `p+ul` #162

Open gnprice opened 1 year ago

gnprice commented 1 year ago

Zulip web has a couple of CSS rules that adjust spacing between sibling block elements. From web/styles/rendered_markdown.css:

    /* The spacing between two paragraphs is significantly larger.  We
       arrange things so that this spacing matches the spacing between
       paragraphs in two consecutive 1-line messages. */
    & p + p {
        margin-top: 10px;
    }
// …

    /* Reduce top-margin when a paragraph is followed by an ordered or bulleted list */
    & p + ul,
    p + ol {
        margin-top: 0;
    }

(The details have changed occasionally over the years, as in zulip/zulip@d7aa186dabb8587d48c1ce076c92b049e4fbba02 and zulip/zulip@62f2396ee27f132a25f906107b60fa19b35c97ab.)

We should either match that behavior, or determine there's a good reason for mobile to differ from web here.

gnprice commented 7 months ago

@karlstolley writes last week that he'll be working soon on cleaning up that CSS, simplifying things. (For example, that rule about p + ul has no effect, because the p has a bottom margin of 3px anyway and the top margin of a ul without that rule would be only 2px.)

So this task should become simpler after that work is complete.

gnprice commented 7 months ago

Some details here:

In particular one planned simplification is:

Shift inter-element Markdown element spacing to a single dimension (margin-top)

(whereas the status quo involves margin-bottom too — which in turn means that its behavior involves CSS margin-collapsing). So that will provide a significantly more straightforward basis for us to translate from.

gnprice commented 7 months ago

Another rule with a similar character, about an absence-of-sibling rather than siblings, is the one with :first-child here:

    /* Headings */
    & h1,
    h2,
    h3,
    h4,
    h5,
    h6 {
        // …
        margin-top: 15px;
        // …
    }

    /* Headings: Ensure that messages that start with a heading don't have
       a weirdly blank area at the very start of the message. */
    & h1:first-child,
    h2:first-child,
    h3:first-child,
    h4:first-child,
    h5:first-child,
    h6:first-child {
        margin-top: 0;
    }

As @sirpengi points out at #492, we don't yet implement that rule either. Probably we should consider it as part of this issue.

karlstolley commented 7 months ago

My hope with headings is that presenting them with a generous line-height will obviate the need for additional margin-top, but I'll keep this case in mind as that work proceeds.