Page MenuHomePhabricator

Proposal: Stop collapsing Vector menu items under more menu and removal associated code
Closed, DeclinedPublic

Assigned To
None
Authored By
Jdlrobson
Jun 2 2020, 7:44 PM
Referenced Files
F31851897: vector-collapsibe-1140px.png
Jun 3 2020, 7:21 AM
F31850806: vector-collapsibe-removed-950px.png
Jun 3 2020, 5:44 AM
F31851822: vector-collapsibe-760px.png
Jun 3 2020, 5:44 AM
F31851480: Screen Shot 2020-06-02 at 12.32.09 PM.png
Jun 2 2020, 7:44 PM
F31851488: Screen Shot 2020-06-02 at 12.40.42 PM.png
Jun 2 2020, 7:44 PM
F31851485: Screen Shot 2020-06-02 at 12.35.15 PM.png
Jun 2 2020, 7:44 PM
F31851486: Screen Shot 2020-06-02 at 12.37.31 PM.png
Jun 2 2020, 7:44 PM
Tokens
"Dislike" token, awarded by Demian.

Description

NOTE: Completely removing tab collapsing in legacy would affect many users negatively. If that's even considered, multiple communities should be consulted first, to avoid another drama like T250393: Use monospace font (or editfont preference) for diffs.

Motivation: T71729: [collapsibleTabs] If a tab's width changes after initial page load, endless animation loop can happen

The legacy Vector skin contains several lines of JS that collapses menu items under the more menu at low resolutions. The Vector skin however is not a responsive skin and it's questionable to how much this improves the experience:

Screen Shot 2020-06-02 at 12.40.42 PM.png (800×1 px, 355 KB)

If anything it gives the illusion of responsive support and makes the product look half baked and unfinished.

The code has one longstanding bug (T71729), creates complexity in the code (Vector is tied to a collapsible and emptyPortlet class and must keep track of gadgets using addPortletLink) and enforces constraints to modernizing the new Vector on the classes that it needs to support.

It's proposed that this code is removed to lighten the maintenance cost of Vector and under the assumption that the new Vector that is currently in development and will soon be the default everywhere will solve this problem more elegantly.

Three proposals are suggested for removing this code to lightweight alternatives:

Proposals

Option 1 - just remove code

Remove code and do nothing. Optimizing Vector at 500px doesn't seem a valuable use of time.
This would cause wrapping of the tabs up to 760px, with gadgets, edit links 1100px+:

vector-collapsibe-760px.png (80×759 px, 22 KB)

vector-collapsibe-1140px.png (58×1 px, 37 KB)

On user pages included from meta above 900px (as noted in T71729#6183822):
vector-collapsibe-removed-950px.png (67×920 px, 22 KB)

At 500px:
Screen Shot 2020-06-02 at 12.32.09 PM.png (728×1 px, 257 KB)

Option 2 - hide #right-navigation at smaller widths

Screen Shot 2020-06-02 at 12.35.15 PM.png (470×1 px, 167 KB)

The expectation would be that if users want to access these links they have to resize.
Note an additional media query rule would be needed for device-width to ensure that mobile phone users using Vector would still see the links.

Option 3 - increase the header height up to at 500px

This would keep the links accessible but without the complexity of the JS.
Tabs still wrap 500px - ca. 750px depending on language, font-size, page (actual tabs shows for the page).

Screen Shot 2020-06-02 at 12.37.31 PM.png (604×1 px, 223 KB)

@media screen and ( max-width: 500px ) {
	#mw-page-base {
		/* personal-menu (2.5em) + .vector-menu-tabs (2.5em) * 2: */
		height: 7.5em;
	}
}

Related Objects

Event Timeline

Demian subscribed.

Fixed min-width: 500px in Option 3.

CC @alexhollender @ovasileva @Niedzielski for team input. This is a significant decision in the context of legacy Vector.
Completely removing tab collapsing in legacy would affect many users negatively. This issue is present up to 900px+ as documented in T253819, T253819#6174864, T253819#6182981, T71729#6183820, 1100px+ with gadgets, edit links, even more with increased browser font-size.
If that's even considered, multiple communities should be consulted first, to avoid another drama like T250393: Use monospace font (or editfont preference) for diffs.

The cause of this issue, the complications and the possible solutions are detailed in T71729#6183821.
The dynamic nature of the layout unfortunately undermines pure-CSS solutions with a fixed width that can't adapt to actual width of the tabs.
Modern layout will be less affected once the SearchBox (ca. 320px) is moved to the header, therefore a different solution might be used for modern. That's in the future, though.

Possible solutions in order of personal preference:

Option 4 (remove the animation and fix the collapsibletabs code) is my suggestion as the most usable solution with the least code and complexity. Ca. half of the JS can be cut.

Note the following comment from @Jdlrobson (ref):

Legacy code is frozen now.

Option 5 (fix the collapsibletabs code) detailed at T71729#6183821 (Easy fix) is closest to that intent.

Option 6 (make the tabs responsive, use JS to position content) - tabs in 2 rows look bad with the current styling.

Option 2 (hide #right-navigation at small width) is feasible, if nobody (including the community) minds that the "Edit", "View history" and Twinkle buttons are no longer there at less than ca. 750px width on a typical page, up to 1100px+.
In that case the collapsibletabs JS can be reduced to a few lines. collapseCondition and the resize event handler is still necessary, there is no pure-CSS solution.

ovasileva triaged this task as Medium priority.EditedJun 3 2020, 11:06 AM

@Jdlrobson - is this task for vector overall, or just for the new version?Moving to needs analysis to discuss the options presented

I am disengaged from these discussions as of late, but I want to chime in to comment. In your own planning you are going to be limiting content width to 960px, making the space for tabs smaller than ever for everyone. That will inevitably make this code more relevant, not less, to the users reading non-English Wikipedias. If you want to modernise this code to make it more resilient, please look into what BBC GEL team has done with their masthead and IntersectionObserver in similar circumstances instead of removing perfectly fine and needed functionality because Vector is not ‘supposed’ to be responsive.

Jdlrobson renamed this task from Stop collapsing Vector menu items under more menu to Proposal: Stop collapsing Vector menu items under more menu and removal associated code.Jun 4 2020, 4:59 PM

I've removed the additional "solutions" from the task description. The question I was raising is whether to remove it. It's okay to end up declining it and say no.
@stjn thanks for your input.

The maintainers discussed this today and will revisit this further on in the project. We're not quite sure what this will look in the new version but we're hoping whatever solution we could up with there could be applied to old Vector.

The problem of the wrapping tabs up to 1100+px is not solved by this ticket.
There are 3 ways to fix the collapsing menus in T71729.

This functionality is still necessary up to 1100+px width (depending on page, gadgets, user preferences), as shown in the screenshots.
Option 1 and 3 don't solve the problem that motivated this task: the tabs would wrap in over the page title.
Option 2 removes the "Edit", "View history" and gadget tabs on narrow screens (no alternative to get to those), while still wrapping at mid-sized screens.

I've removed the additional "solutions" from the task description. The question I was raising is whether to remove it.

Fine by me. Even the leanest additional "solution" kept 5% of the code.

Jdlrobson lowered the priority of this task from Medium to Low.Jun 4 2020, 11:36 PM

The problem of the wrapping tabs up to 1100+px is not solved by this ticket.

Neither is it meant to be solved.
The question is do we want to maintain this code indefinitely and what it takes to remove it. However we are not in a place to make a decision on this either way right now.

The problem of the wrapping tabs up to 1100+px is not solved by this ticket.

Neither is it meant to be solved.
The question is do we want to maintain this code indefinitely and what it takes to remove it.

It seems to me what it takes to remove it is to solve the wrapping of tabs.
Perhaps there are alternatives, like moving the "tabs" to different, meaningful positions, but that's not on the DI plans.

Would Web-Team-Backlog please take another look at this and come to some conclusion, as there are more and more and more duplicates reported for T71729? Thanks.

ovasileva raised the priority of this task from Low to Medium.Oct 23 2020, 11:47 AM

Can reopen this if necessary, but this seems less important given we're moving away from touching legacy Vector and this works much better for making new Vector responsive on mobile resolutions.