Page MenuHomePhabricator

Wider tables overlap sticky page tools (Upstream Minerva's responsive table styles to core SkinModule)
Closed, ResolvedPublic5 Estimated Story PointsBUG REPORT

Description

Background

Even before, wider tables were breaking out of the content area, but after the page tools were made sticky, this problem became significantly more problematic, as these tables will now overlap the menu. This is possible, because the content is later in the dom than the menu, because tables size to content and because grid allows elements to overflow outside of the allocated grid space.

Generally wide tables are not at the top of the page, so if the tools are not sticky, you won't encounter this problem.

User story

As a user I want tables to always be constrained to the content area.

Screenshot 2023-02-24 at 21.16.46.png (1×2 px, 790 KB)

Acceptance criteria

  • Large tables should be constrained to content area. Where they are larger than the content area a horizontal scroll bar will appear.

Implementation details

Minerva handles this using styles defined here:
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/skins/MinervaNeue/+/refs/heads/master/resources/skins.minerva.base.styles/content/tables.less#12-47

These should be moved to https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki.skinning/content.tables.less

  • Upstream styles from Minerva to mediawiki.skinning in core
  • Replace .content selector with .mw-parser-output selector.
  • Wrap tables with a div element with the noresize class on to limit them to the current content are in Vector.
  • The wrapping should not occur for infoboxes (e.g. https://www.mediawiki.org/wiki/Reading/Web/Desktop_Improvements) [T366119]
  • The wrapping should not occur for tables which are floated either via style attribute or floatright, floatleft classes.

QA steps

QA will be done as part of T366314

Sign off steps

  • Create a task for cleaning up Minerva's CSS following the upstreaming of this code to core.

Captured in T113101

Original ticket

I added the following for myself to get this somewhat workable, but... it's a bit of a mess.

/* Fix wide tables on narrow layout */
@media all and (max-width:999px) {
	/* table overflow protection */
	.wikitable {
		display: block;
		overflow-x: auto;
		/* table border collapse no longer functions, so remove border */
		border: 0;
		/* The background of the table can now be outside the table, so restore original */
		background-color: unset;
	}
	
	.wikitable > thead,
	.wikitable > tbody,
	.wikitable > tfoot {
		/* Cancel out floating table headers, as this is now within another scroll context */
		top: 0 !important;
		/* Apply the background color here, as no longer on table */
		background-color: #f8f9fa;
	}
}

/* Fix wide tables on pages with tools menu */
@media all and (min-width:1000px) and (max-width:1399px) {
	/* table overflow protection */
	html.client-js.vector-feature-page-tools-pinned-enabled .wikitable {
		display: block;
		overflow-x: auto;
		/* table border collapse no longer functions, so remove border */
		border: 0;
		/* The background of the table can now be outside the table, so restore original */
		background-color: unset;
	}

	html.client-js.vector-feature-page-tools-pinned-enabled .wikitable > thead,
	html.client-js.vector-feature-page-tools-pinned-enabled .wikitable > tbody,
	html.client-js.vector-feature-page-tools-pinned-enabled .wikitable > tfoot {
		/* Cancel out floating table headers, as this is now within another scroll context */
		top: 0 !important;
		/* Apply the background color here, as no longer on table */
		background-color: #f8f9fa;
	}

}

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson renamed this task from Wider tables overlap sticky page tools to Wider tables overlap sticky page tools (Upstream Minerva's responsive table styles to core SkinModule).May 16 2024, 5:59 PM
Jdlrobson updated the task description. (Show Details)

Change #1035034 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/core@master] Update content.table styles to ensure tables don't overlap other page elements and horizontally scroll if needed

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

bwang removed bwang as the assignee of this task.May 23 2024, 4:33 PM
bwang subscribed.

I will provide some examples.

Change #1035574 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Skin: The noresize class is upstreamed to core from Minerva

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

Change #1035034 merged by jenkins-bot:

[mediawiki/core@master] Skin: Responsive tables

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

Change #1035796 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Wrap tables with JS

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

Change #1035574 merged by jenkins-bot:

[mediawiki/core@master] Skin: The noresize class is upstreamed to core from Minerva

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

Change #1035796 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Wrap tables with JS

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

This breaks sticky table headers, at least in Chrome. See e.g. https://www.mediawiki.org/wiki/Developers/Maintainers#Misc.
overflow-x: auto makes the table a scroll container, and that makes it impossible to position the sticky header in such a way that it accounts for Vector's top bar.

Screenshot Capture - 2024-05-28 - 22-10-17.png (902×1 px, 215 KB)
Screenshot Capture - 2024-05-28 - 22-10-29.png (634×1 px, 173 KB)
Fully visible: header is off and obscures first non-header row.Scrolled: header doesn't actually stick.

Change #1036756 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Limit responsive tables to .wikitables

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

Jdlrobson raised the priority of this task from High to Unbreak Now!.May 28 2024, 9:48 PM

Change #1036665 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@wmf/1.43.0-wmf.7] Limit responsive tables to .wikitables

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

Change #1036756 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Limit responsive tables to .wikitables

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

On the test page for a patch for a different task, T362939, I see this for a floating table (link):

image.png (358×1 px, 45 KB)

class="noresize" is to blame.

And even if I remove float: right, look at these margins!

image.png (360×1 px, 45 KB)

Here we have table's 1em margins + paragraph's margins. These should collapse, but they don't due to the use of overflow-x: auto.

Change #1037018 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@wmf/1.43.0-wmf.7] Revert "Wrap tables with JS"

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

Change #1036665 abandoned by Jdlrobson:

[mediawiki/skins/Vector@wmf/1.43.0-wmf.7] Limit responsive tables to .wikitables

Reason:

We have decided to go ahead with this patch: https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/1037018?usp=search

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

Change #1037018 merged by jenkins-bot:

[mediawiki/skins/Vector@wmf/1.43.0-wmf.7] Revert "Wrap tables with JS"

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

Mentioned in SAL (#wikimedia-operations) [2024-05-29T16:50:25Z] <dancy@deploy1002> Started scap: Backport for [[gerrit:1037018|Revert "Wrap tables with JS" (T330527)]]

Jdlrobson lowered the priority of this task from Unbreak Now! to High.May 29 2024, 4:57 PM

the engineers chatted through next steps for this task before the sprint ends:

  • We are going to add a temporary feature flag
  • We will limit the table wrapping to tables that are not floated.

As part of sign off for next sprint:

  • Create a follow up ticket to deploy it in a more controlled limited environment (projects where legacy Vector is the default) to allow further testing before a wider roll out.

Mentioned in SAL (#wikimedia-operations) [2024-05-29T18:08:26Z] <dancy@deploy1002> Started scap: Backport for [[gerrit:1037018|Revert "Wrap tables with JS" (T330527)]]

Mentioned in SAL (#wikimedia-operations) [2024-05-29T18:10:59Z] <dancy@deploy1002> dancy and jdlrobson: Backport for [[gerrit:1037018|Revert "Wrap tables with JS" (T330527)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-05-29T18:33:36Z] <dancy@deploy1002> Finished scap: Backport for [[gerrit:1037018|Revert "Wrap tables with JS" (T330527)]] (duration: 25m 10s)

Change #1037187 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Limit table wrapping to configuration flag

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

Change #1037187 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Limit table wrapping to configuration flag

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

Change #1037583 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Restrict table logic to nonfloated wikitables

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

bwang removed bwang as the assignee of this task.May 30 2024, 5:09 PM

Change #1037583 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Restrict table logic to nonfloated wikitables

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

QA will be done as part of T366314
I just need to handle the sign off step on this one and it's done.

Change #1037583 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Restrict table logic to nonfloated wikitables

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

You might want to use getComputedStyle instead for avoiding floating tables? There will be tables with the floatright class (as an example) that should have this enhancement at narrow resolutions (where no table is safe, even the narrow ones).

Thanks @Izno I've made a note to investigate this as part of T366314.

This breaks sticky table headers, at least in Chrome. See e.g. https://www.mediawiki.org/wiki/Developers/Maintainers#Misc.
overflow-x: auto makes the table a scroll container, and that makes it impossible to position the sticky header in such a way that it accounts for Vector's top bar.

Screenshot Capture - 2024-05-28 - 22-10-17.png (902×1 px, 215 KB)
Screenshot Capture - 2024-05-28 - 22-10-29.png (634×1 px, 173 KB)
Fully visible: header is off and obscures first non-header row.Scrolled: header doesn't actually stick.

Has this been followed up on? It has been reported on mediawiki.org. I’d at least disable this feature when all right sidebars are collapsed, as it does more harm than good in that case.