[Regression]: Remove extra spacing between header and page title (from empty site notice)
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	bwang
	Aug 18 2022, 6:10 PM

Description

Background

The initial fix from T312749 introduced extra 24px spacing between the header and page title.

current	previous/correct

Before
After

More context: https://phabricator.wikimedia.org/T312749#8163262

AC

There is a 24px spacing between the header and page title when the site notice is empty
When the site notice is present there is a 24 px spacing between the header, site notice, and page title

Developer notes

The simplest solution is to removing row gap from the grid container. This impacts the spacing of the TOC in all the non collapsed cases (i.e. main menu closed, main menu open, scrolled down with sticky header, scrolled down without sticky header), so the TOC CSS should be carefully updated and QAed.

Details

Subject	Repo	Branch	Lines +/-
Followup: clean up cached HTML code from T315595	mediawiki/skins/Vector	master	+0 -2
Fix site notice spacing	mediawiki/skins/Vector	wmf/1.39.0-wmf.26	+41 -46
Fix site notice spacing	mediawiki/skins/Vector	master	+41 -46
Reapply "Remove grid row gap in favor of margins"	mediawiki/skins/Vector	master	+35 -43
Remove grid row gap in favor of margins	mediawiki/skins/Vector	master	+35 -43
Remove grid row gap in favor of margins	mediawiki/skins/Vector	wmf/1.39.0-wmf.26	+36 -43
Remove grid row gap in favor of margins	mediawiki/skins/Vector	wmf/1.39.0-wmf.25	+36 -43

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Resolved	Jdlrobson	T309972 [Goal] prepare desktop improvements project for further deployment
Resolved	ovasileva	T301780 [Epic] Visual design & layout refinements
Resolved	ovasileva	T315595 [Regression]: Remove extra spacing between header and page title (from empty site notice)

Event Timeline

bwang created this task.Aug 18 2022, 6:10 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 18 2022, 6:10 PM

bwang mentioned this in T312749: Show central notice at full width after grid work.Aug 18 2022, 6:10 PM

bwang updated the task description. (Show Details)Aug 18 2022, 6:15 PM

• alexhollender_WMF updated the task description. (Show Details)Aug 18 2022, 8:01 PM

when looking at this I also noticed that the height of the site header is defined as 66px, rather than using min-content as all of the other grid rows do (aside from the main content row). from the standpoint of evaluating layout issues (like this one), and just generally being able to easily understand the front-end, it seems like it would be better if we used min-content and then added padding: 8px 0 to .mw-header.

current	updated

• alexhollender_WMF added a parent task: T301780: [Epic] Visual design & layout refinements.Aug 18 2022, 8:11 PM

• alexhollender_WMF added a project: Web-Team-Backlog.

• alexhollender_WMF renamed this task from Remove extra spacing between header and page title from empty site notice to [Visual refinements] Remove extra spacing between header and page title from empty site notice.Aug 19 2022, 2:12 PM

• alexhollender_WMF edited projects, added Web-Team-Backlog (Kanbanana-2022-23-Q1); removed Web-Team-Backlog.

This particular bug is due to our use of row-gap and the site notice row being empty. A quick Google didn't yield any answers for me on how to avoid this, so I suspect solving this will involve removing the row gap and using margins instead.

ovasileva triaged this task as High priority.Aug 22 2022, 9:27 AM

ovasileva renamed this task from [Visual refinements] Remove extra spacing between header and page title from empty site notice to Regression: Remove extra spacing between header and page title from empty site notice.Aug 22 2022, 3:20 PM

ovasileva renamed this task from Regression: Remove extra spacing between header and page title from empty site notice to [Regression]: Remove extra spacing between header and page title from empty site notice.

We should backport this one once it's fixed

ovasileva set the point value for this task to 3.Aug 22 2022, 5:46 PM

ovasileva moved this task from Upcoming to Ready for Development on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.

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

[mediawiki/skins/Vector@master] Remove grid row gap in favor of margins

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

gerritbot added a project: Patch-For-Review.Aug 22 2022, 8:25 PM

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

[mediawiki/skins/Vector@master] Remove grid row gap in favor of margins

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

bwang moved this task from Ready for Development to Code Review on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.Aug 22 2022, 8:39 PM

bwang mentioned this in T313060: [Visual refinements] Table of contents overlaps the footer or page boundary depending on article length.Aug 22 2022, 9:35 PM

Change 825401 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Remove grid row gap in favor of margins

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.27; 2022-08-29).Aug 23 2022, 8:00 AM

Change 825752 had a related patch set uploaded (by Jdlrobson; author: Bernard Wang):

[mediawiki/skins/Vector@wmf/1.39.0-wmf.25] Remove grid row gap in favor of margins

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

Next step: backporting and QA. Note backporting will also clean out the noise in https://pixel.wmcloud.org/reports/desktop/index.html.

It looks like this can't easily be backported without backporting another change. I'm getting variable @selector-main-menu-closed is undefined - do you know which patch introduces that element?

Change 825752 abandoned by Jdlrobson:

[mediawiki/skins/Vector@wmf/1.39.0-wmf.25] Remove grid row gap in favor of margins

Reason:

Can't be backported - requires another Vector patch to be backported first but I am not sure which.

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

Jdlrobson reassigned this task from bwang to Edtadros.Aug 23 2022, 3:55 PM

Jdlrobson moved this task from Code Review to QA on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.

• alexhollender_WMF renamed this task from [Regression]: Remove extra spacing between header and page title from empty site notice to [Regression]: Remove extra spacing between header and page title (from empty site notice).Aug 23 2022, 4:45 PM

@Jdlrobson Looks like it was https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/824267, was a small CSS variable clean up patch so i believe it's not as risky relatively speaking. Hoping that's the only dependency

Change 825752 restored by Jdlrobson:

[mediawiki/skins/Vector@wmf/1.39.0-wmf.25] Remove grid row gap in favor of margins

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

Change 825889 had a related patch set uploaded (by Urbanecm; author: Bernard Wang):

[mediawiki/skins/Vector@wmf/1.39.0-wmf.26] Remove grid row gap in favor of margins

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

Change 825752 merged by jenkins-bot:

[mediawiki/skins/Vector@wmf/1.39.0-wmf.25] Remove grid row gap in favor of margins

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

Change 825889 merged by jenkins-bot:

[mediawiki/skins/Vector@wmf/1.39.0-wmf.26] Remove grid row gap in favor of margins

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

ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.25; 2022-08-15); removed MW-1.39-notes (1.39.0-wmf.27; 2022-08-29).Aug 23 2022, 9:00 PM

Jdlrobson moved this task from QA to Needs More Work on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.Aug 24 2022, 1:35 PM

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

[mediawiki/skins/Vector@master] Site notice spacing fix

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

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

[mediawiki/skins/Vector@master] Remove grid row gap in favor of margins

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

bwang moved this task from Needs More Work to Code Review on the Web-Team-Backlog (Kanbanana-2022-23-Q1) board.Aug 24 2022, 5:45 PM

bwang reassigned this task from Edtadros to Jdlrobson.Aug 24 2022, 5:47 PM

bwang added a subscriber: Edtadros.

Change 826355 abandoned by Bernard Wang:

[mediawiki/skins/Vector@master] Reapply "Remove grid row gap in favor of margins"

Reason:

squashed into https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/826307

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

bwang reassigned this task from Jdlrobson to Jdrewniak.Aug 25 2022, 4:51 PM

Change 826307 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Fix site notice spacing

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

ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.27; 2022-08-29); removed MW-1.39-notes (1.39.0-wmf.25; 2022-08-15).Aug 26 2022, 9:00 PM

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

[mediawiki/skins/Vector@wmf/1.39.0-wmf.26] Fix site notice spacing

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

Change 827534 merged by Clare Ming:

[mediawiki/skins/Vector@wmf/1.39.0-wmf.26] Fix site notice spacing

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

Mentioned in SAL (#wikimedia-operations) [2022-08-29T20:42:29Z] <cjming@deploy1002> Synchronized php-1.39.0-wmf.26/skins/Vector: Backport: [[gerrit:827534|Fix site notice spacing (T315595)]] (duration: 03m 46s)

ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.26; 2022-08-22); removed MW-1.39-notes (1.39.0-wmf.27; 2022-08-29).Aug 29 2022, 9:00 PM

Skipping QA in beta cluster because this was backported.

Test Result - Prod

Status: ✅ PASS
Environment: testwiki
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: There is a 24px spacing between the header and page title when the site notice is empty

Screen Shot 2022-08-31 at 6.29.09 PM.png (367×1 px, 158 KB)

✅ AC2: When the site notice is present there is a 24 px spacing between the header, site notice, and page title

Screen Shot 2022-08-31 at 6.28.00 PM.png (789×1 px, 318 KB)

ovasileva closed this task as Resolved.Sep 1 2022, 8:40 AM

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

[mediawiki/skins/Vector@master] Followup: clean up cached HTML code from T315595

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

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

[mediawiki/skins/Vector@master] Followup: clean up cached HTML code from T315595

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

Change 829046 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Followup: clean up cached HTML code from T315595

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.1; 2022-09-12).Sep 9 2022, 11:00 PM

Jdlrobson added a project: Regression.Oct 12 2022, 6:51 PM

	F35503139: Screen Shot 2022-08-31 at 6.28.00 PM.png
	Sep 1 2022, 1:31 AM

	F35503137: Screen Shot 2022-08-31 at 6.29.09 PM.png
	Sep 1 2022, 1:31 AM

	F35476694: image.png
	Aug 18 2022, 8:10 PM

	F35476696: image.png
	Aug 18 2022, 8:10 PM

[Regression]: Remove extra spacing between header and page title (from empty site notice)Closed, ResolvedPublic3 Estimated Story PointsActions

Description

Background

AC

Developer notes

Details

Related ObjectsSearch...

Event Timeline

Test Result - Prod

QA Steps

[Regression]: Remove extra spacing between header and page title (from empty site notice)
Closed, ResolvedPublic3 Estimated Story Points
Actions

Related Objects
Search...