Page MenuHomePhabricator

Two lines appear beneath H1 section headings when they are immediately followed by an H2
Closed, ResolvedPublic

Assigned To
Authored By
ppelberg
Sep 21 2022, 1:40 AM
Referenced Files
F35538143: image.png
Sep 28 2022, 3:48 PM
F35538137: image.png
Sep 28 2022, 3:48 PM
F35538149: image.png
Sep 28 2022, 3:48 PM
F35538139: image.png
Sep 28 2022, 3:48 PM
F35538152: image.png
Sep 28 2022, 3:48 PM
F35538135: image.png
Sep 28 2022, 3:48 PM
F35538121: image.png
Sep 28 2022, 3:48 PM
F35526472: Screen Shot 2022-09-20 at 6.36.21 PM.png
Sep 21 2022, 1:40 AM

Description

This task involves an issue with how the horizontal lines that differentiate talk page sections can appear in certain situations on desktop talk pages.

Behavior

  1. On desktop, visit a talk page where Topic Containers are enabled and said talk page contains a minimum of one = H1 = section heading that:
    • A) does not contain any content within it and
    • B) is followed by an == H2 ==. E.g. https://w.wiki/5izu .

Actual

  1. ❗️Notice two horizontal lines appear beneath the = H1 =:

Screen Shot 2022-09-20 at 6.36.21 PM.png (430×1 px, 89 KB)

Expected

  1. TBD

Open questions

  • 1. What ought to happen in this case? Should the horizontal line (read: section marker) appear above the = H1 = as is done for all other == H2 == section headings?

Issue reported by @Alsee on mediawiki.org: Topic:Wypgefpw1h19upfr.

Event Timeline

I think in the example given, having two lines is correct. If there was an h1 followed by an h2 with no content in-between I could see the argument for suppressing the line above the h2 - we already do this if the h2 is the first element on the page (i.e. follows the page title h1).

Change 833852 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/DiscussionTools@master] Suppress top border when section follows <h1>

https://gerrit.wikimedia.org/r/833852

Akk, please do not deploy that patch.

That changes a somewhat-predictable, bizarre, and confusing result into an utterly unpredictable, even more bizarre, and even more confusing result. If you asked one hundred editors, I expect exactly none could predict or explain such behavior.

This issue is new, it was caused by the recent change that moved the h2 line above the header on Talk pages. The requested fix is to put the h2 line back where it was, below the header, and the issue vanishes.

We already suppress the border if the <h2> is immediately after the page title <h1>, so suppressing it after other <h1>s makes it more consistent.

Change 833852 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Suppress top border when section follows <h1>

https://gerrit.wikimedia.org/r/833852

I approved the patch, but I'm not really happy with it and I'd like us to think about the design some more.

In my mind, our horizontal lines are not "above H2 headings", they are "between H2 sections". This is the most obvious in the mobile version of the design with collapsible sections:

image.png (2×3 px, 262 KB)

We already have ugly special cases in the code trying to achieve this (e.g. we hide the top border if the heading is the first element on the page or if it directly follows the table of contents). I don't think it makes the behavior more unpredictable, it was already pretty bad in that regard.

Given this, it would make sense to hide the top border in the original example too (https://w.wiki/5izu), because it doesn't serve to separate two H2 sections. However, the patch doesn't do it.

(I think this is impossible to implement exactly in just CSS, we'd need the PHP code to add some extra classes to headings which are/aren't supposed to have separators.)


Regarding "put the h2 line back where it was, below the header", I made a few quick mockups to see how this would look.

I am not really a fan of this, I think the line looks a bit weird with all the extra stuff that we added to the headings. Putting the line in the middle of the heading (below heading text, above added metadata) actually looks kind of okay to me, but I think I still like the current version more. Putting it below everything makes it look too much like a separator. I also tried shortening the lines to only underline the heading text, thinking that this would avoid looking like a separator, but I don't actually like it at all.

Current design (line above heading)Before DiscussionTools
image.png (2×3 px, 577 KB)
image.png (2×3 px, 567 KB)
Proposed v1 (line below heading text)Proposed v1b (short line below heading text)
image.png (2×3 px, 517 KB)
image.png (2×3 px, 522 KB)
Proposed v2 (line below entire heading)Proposed v2b (short line below entire heading)
image.png (2×3 px, 517 KB)
image.png (2×3 px, 517 KB)

I think the key is to get back to the fundamental bug I was attempting to report. The current patch mangles the rendering when you copy-paste content from an article to Talk for discussion and work. The expected behavior is consistent and accurate rendering of wikitext when content is copy-pasted. Moving the horizontal line had weird unexpected side effects, like the double-line effect when you have an H2 after an H1, but that should not distract from the underlying issue.

I agree with @matmarex that the metadata looks better below the line, Proposed v1. I suspected that would be the case, but I didn't want to get into that detail without seeing it. The picture was helpful.

To be precise: Sections with zero signatures detected should render exactly the same as they do on article pages. Discussion tools are built on top of talk pages because there was a global-community/WMF agreement that it was important to preserve existing Talk page functionality as a complete-and-accurate workplace for article content. We shouldn't even consider what "looks better" in that case, there is exactly one canonical correct answer.

Sections where signatures are detected should be derived from the baseline header with minimal change. Preexisting elements should be styled exactly the same, only adding the new elements.

ppelberg renamed this task from Two lines appear above H1 section headings when they are immediately followed by an H2 to Two lines appear beneath H1 section headings when they are immediately followed by an H2.Sep 29 2022, 8:48 PM

Feel free to file another task about changing the appearance of <h2>s, although considering we have received generally positive feedback about the new appearance and no similar complaints to your's, it seems unlikely we will want to change this.

(I think this is impossible to implement exactly in just CSS, we'd need the PHP code to add some extra classes to headings which are/aren't supposed to have separators.)

The clean solution would be wrapping topics in <article> HTML elements (in PHP). However, this would probably not play nicely with wrapper templates like {{Archive top}}, which are technically part of the previous section. A maybe close-enough, and actually CSS-only, solution is the CSS general sibling connector, along these lines:

h2 ~ h2 {
	border-top: /* whatever */;
}

This would also break on {{Archive top}}, since the wrapping causes the headings to no longer be siblings, but is it maybe a good thing? If on-wiki templates provide extra styles, maybe our separator is not that important after all.

Regarding "put the h2 line back where it was, below the header", I made a few quick mockups to see how this would look.

I am not really a fan of this, I think the line looks a bit weird with all the extra stuff that we added to the headings. Putting the line in the middle of the heading (below heading text, above added metadata) actually looks kind of okay to me, but I think I still like the current version more. Putting it below everything makes it look too much like a separator. I also tried shortening the lines to only underline the heading text, thinking that this would avoid looking like a separator, but I don't actually like it at all.

IMO the short lines look horrible, especially when they’re shorter than one of the elements (e.g. Localizing Topic Containers in v1b or the hr tag with… in v2b). Among the long versions, I think I actually like the bottom version (v2) the most – definitely more than the middle version (v1), but even a bit more than the top (current) version.

Meta

Change 833852 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/DiscussionTools@master] Suppress top border when section follows <h1>

https://gerrit.wikimedia.org/r/833852

Considering:

  1. This task is about the case where a Topic Container follows an <H1> that contains content
  2. 833852 addresses the case where a Topic Container follows an <H1> that does NOT contain content

...I've filed T319147 and we'll leave this ticket open.

...back to the fundamental bug I was attempting to report.

@Alsee I'll follow up about this soon (although, not during this next week).

...back to the fundamental bug I was attempting to report.

@Alsee I'll follow up about this soon (although, not during this next week).

hi @Alsee – thank you for being patient with me here.

I've filed T322255 where we will revisit the approach we defined in T302450#7862086 and ultimately, decide how to present == H2 == headings that accompany sections that the comment parser does NOT detect any signatures within.

I've posed a follow up question to what you shared in T318198#8270941 there.


Meta: in moving the discussion around the larger question of how H2 w/o signatures should be styled to T322255, I think this ticket can be QA'd seeing as how the patches to resolve the specific issue this task is scoped to address have been merged.

ppelberg claimed this task.