Page MenuHomePhabricator

Table collapsing button is too close to the caption - add some CSS spacing
Open, Needs TriagePublic

Description

Hello. Open en.wikipedia.org (in safemode), ru.wikipedia.org or he.wikipedia.org, edit the source of a wiki page (for example your user page), and preview the following wikicode:

{| class="wikitable mw-collapsible"
|+ name
|-
|aaaaaaaaaa||bbbbbbbbbb||cccccccccc
|}

You'll see "name[hide]". There should be at least some space between the words. The same when collapsed.

This is likely in the file resources/src/jquery/jquery.makeCollapsible.css in MediaWiki core.

Test use cases:

Event Timeline

IKhitron created this task.Oct 25 2017, 2:02 PM
Restricted Application added subscribers: TerraCodes, Aklapper. · View Herald TranscriptOct 25 2017, 2:02 PM

For .mw-collapsible-toggle, padding-left: 0.2em; is defined in https://en.wikipedia.org/wiki/MediaWiki:Common.css
So that specific example would be on-wiki CSS and hence invalid for Phabricator.

However, if you wanted to change the default in MediaWiki core, that would be in resources/src/jquery/jquery.makeCollapsible.css (I think).

As I already said, enwiki in safemode only.

If this is still a valid task, maybe we can push it to Google-Code-in-2017?

@D3r1ck01 This sounds like a good task for Google-Code-in-2017 to me (just verified on hewiki one more time).

D3r1ck01 added a comment.EditedOct 28 2017, 7:42 AM

@D3r1ck01 This sounds like a good task for Google-Code-in-2017 to me (just verified on hewiki one more time).

Okay! If you like, I can mentor this along side with you :), will import to GCI site at once: https://codein.withgoogle.com/dashboard/tasks/5158707136561152/

@D3r1ck01 Sounds good. The task should not only be about adding the rules to core, but also remove the MediaWiki:Common.css rules afterwards from current understanding. Latter should probably be done by an experienced Wikipedia contributor though as it involves social interaction there.

Kipod removed a subscriber: Kipod.Nov 12 2017, 7:21 AM
Aklapper updated the task description. (Show Details)Nov 20 2017, 1:36 PM
Aklapper updated the task description. (Show Details)
Aklapper renamed this task from Table collapsing button is too close to the capture to Table collapsing button is too close to the capture - add some CSS spacing.Nov 20 2017, 1:38 PM
eflyjason added a subscriber: eflyjason.EditedDec 2 2017, 1:23 PM

With wiki code

{| class="wikitable mw-collapsible"
|+name
|-
|aaaaaaaaaa||bbbbbbbbbb||cccccccccc
|}

it seems that the HTML produced is:

<table class="wikitable mw-collapsible">
<caption>name
</caption>
<tr>
<td>aaaaaaaaaa</td>
<td>bbbbbbbbbb</td>
<td>cccccccccc
</td></tr></table>

There is an extra return line inside caption tag, meaning that if we simply add padding-left: 0.2em; to the mw-collapsible-toggle tag, an extra space will be produced (see image).

However, using the same wiki code on English Wikipedia, the extra return line is not produced. Any idea on what's the difference inside?

I tried something like this:

It should be working fine now, but I'm not sure if it's the best solution. Any suggestions?

@eflyjason, it would be good to use Gerrit Code Review(recommended) system to submit your patch. Here is a link to the Gerrit Tutorials: https://www.mediawiki.org/wiki/Gerrit/Tutorial#Prepare_to_work_with_Gerrit. Looking forward to it.

With wiki code

{| class="wikitable mw-collapsible"
|+name
|-
|aaaaaaaaaa||bbbbbbbbbb||cccccccccc
|}

it seems that the HTML produced is:

<table class="wikitable mw-collapsible">
<caption>name
</caption>
<tr>
<td>aaaaaaaaaa</td>
<td>bbbbbbbbbb</td>
<td>cccccccccc
</td></tr></table>

There is an extra return line inside caption tag, meaning that if we simply add padding-left: 0.2em; to the mw-collapsible-toggle tag, an extra space will be produced (see image).

However, using the same wiki code on English Wikipedia, the extra return line is not produced. Any idea on what's the difference inside?

Not yet sure of why this behavior but this calls for some investigation. Thanks.

Change 394847 had a related patch set uploaded (by Eflyjason; owner: Eflyjason):
[mediawiki/core@master] Component: Add some CSS spacing before table collapsing button

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

This comment was removed by eflyjason.
Fomafix added a subscriber: Fomafix.Dec 3 2017, 6:09 PM

The whitespace before </caption> and </td> should removed from the parser. This whitespace gets also copied into clipboard when selecting the caption or the table element with a triple click.

The whitespace before </caption> and </td> should removed from the parser. This whitespace gets also copied into clipboard when selecting the caption or the table element with a triple click.

Done in my patch set 5. :D

I'd consider using a little different margin-left value as well, 0.5em seems preferable looking at various other elements in Vector (and MinervaNeue, which is using a different UI logic AFAIK).
0.2em seems like a magic number invented with no other element carrying it. .mw-content-ltr .mw-editsection uses 1em but that applies to a heading.

eflyjason added a comment.EditedDec 5 2017, 9:38 AM

I'd consider using a little different margin-left value as well, 0.5em seems preferable looking at various other elements in Vector (and MinervaNeue, which is using a different UI logic AFAIK).
0.2em seems like a magic number invented with no other element carrying it. .mw-content-ltr .mw-editsection uses 1em but that applies to a heading.

I guess 0.2em is to simulate a regular space's width. Setting to 0.5em seems somehow weird, especially after the table is collapsed.

0.2em:

0.5em:

eflyjason added a comment.EditedDec 5 2017, 9:41 AM

A follow-up change regarding parser is added here: https://gerrit.wikimedia.org/r/395477 (T182074)

Od1n added a subscriber: Od1n.Dec 5 2017, 1:42 PM

0.3em is a pretty frequent value on mediawiki, and it fits great here.

A distance can also generated by

.mw-collapsible-toggle:before {
	content: ' ';
}

This has a different behavior on wrapping.

eflyjason added a comment.EditedDec 5 2017, 2:52 PM

A distance can also generated by

.mw-collapsible-toggle:before {
	content: ' ';
}

This has a different behavior on wrapping.

That sounds like a good solution too as we won't have to worry about the em number. Would there be any compatibility issue with old browsers? I think since IE 8 they all support :before?

IE 8 supports the :before (not the ::before). But this does not matter because IE 8 is only supported without JavaScript and the collapsing needs JavaScript.

Od1n added a comment.EditedDec 5 2017, 3:30 PM

Please have a look at T155347 (and Gerrit patches 9 and 10), also please try your code on https://test.wikipedia.org/wiki/Test_suite_for_mw-collapsible, and especially on https://test.wikipedia.org/wiki/CollapsingTestpageMw (custom toggle).

An em based margin actually gives better layout control than CSS content in case we're unifying distance of such interactive elements together with text treatment, compare [ edit ] vs [Expand] on enwiki

  • internal whitespace and
  • uppercase action verb start

Also @Od1n, 0.3em is foremost used for top/bottom distance and only in one case .mw-echo-notifications-badge for horizontal distance. Compare

Please have a look at T155347 (and Gerrit patches 9 and 10), also please try your code on https://test.wikipedia.org/wiki/Test_suite_for_mw-collapsible, and especially on https://test.wikipedia.org/wiki/CollapsingTestpageMw (custom toggle).

I implemented your changes in your patch 10. https://test.wikipedia.org/wiki/Test_suite_for_mw-collapsible should be working correctly. But I think for https://test.wikipedia.org/wiki/CollapsingTestpageMw the only way is to add margin-left: 0; and margin-right: 0; manually to the custom toggle.

Od1n added a comment.EditedDec 6 2017, 9:42 AM

Yes, it would require manually adding margin-left: 0; and margin-right: 0; to the custom toggles, in addition to the already needed float: none;.
That's why I'd still bend to favor my patch 9, which has a "whitelist" approach.

The root cause of this, is because regular (automatically created) and custom (manually specified) toggles use the same class, thus aren't distinguished at the CSS level.
I don't know if there is room for improvement about this, but if so, let's keep it for a later, separate ticket.

Volker_E updated the task description. (Show Details)Dec 21 2017, 4:05 PM

Just for completion: 0.2em seems reasonable in context for the time being.

D3r1ck01 moved this task from Backlog to Doing on the good first task board.Jan 26 2018, 4:23 PM
Kipod renamed this task from Table collapsing button is too close to the capture - add some CSS spacing to Table collapsing button is too close to the caption - add some CSS spacing.Jan 31 2018, 10:17 PM
Izno moved this task from Backlog to Change CSS on the CSS board.Sep 7 2018, 4:55 PM
stjn added a subscriber: stjn.Oct 3 2018, 10:17 AM

Just for completion: 0.2em seems reasonable in context for the time being.

I’m quite late to this: maybe 0.25em for parity with edit section links?

What is the current status of this ticket? What kind of patch would be accepted to solve this issue?

I see there's an open patch which should probably be abandoned if it's not being worked on so somebody else can work on this task - https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/394847/

Od1n added a comment.Feb 4 2020, 3:45 PM

T155347 is ready and awaiting for more than two years 😭

Krinkle removed a subscriber: Krinkle.Feb 4 2020, 5:47 PM
eflyjason removed eflyjason as the assignee of this task.Mar 18 2020, 4:20 PM