Page MenuHomePhabricator

Vector: Use a consistent indentation for all sidebar links
Closed, ResolvedPublic

Description

On
https://en.wikipedia.org/wiki/Main_Page?uselang=en&useskin=vector
the links from sections "Interaction", "Tools", "Print/Export" and "Languages" are not aligned with the Links from the top ("Navigation") section.

Maybe this was caused by the removal of the collapsible feature?

See Also:

Details

Reference
bz65444
Related Gerrit Patches:

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 3:20 AM
bzimport added projects: Vector, good first bug.
bzimport set Reference to bz65444.
bzimport added a subscriber: Unknown Object (MLST).
He7d3r created this task.May 17 2014, 10:10 PM

The current formatting also makes many items to wrap to a new line.

(In reply to Helder from comment #1)

The current formatting also makes many items to wrap to a new line.

Ignore this comment, I was testing something on Firefox and set the minimum font size to 13px. This was the cause of the wrapping...

Why is this a code-update-regression, and what is the actual problem?
I guess they are not meant to be aligned, because these section were collapsible a while ago, but I don't see any change in indentation compared to the last weeks?

Compare the bugzilla sidebar to the sidebar on the wikis. On bugzilla, there is no indentation for the uncollapsed sections. This is how it should look like on the wikis as well.

Also, see the image on
https://www.mediawiki.org/wiki/Wikimedia_Foundation_Design/Whitespace#Alignment

Before the code update, there was a image in the left of the headers, and in that case the margin(padding?) makes sense, to make the items aligned with the header. Now, there is no icon, so there should be no white space either.

Change-Id: I220057c799bef7e8c4d964f47d56f61ec120d8c2

Created attachment 15437
en.wiki sidebar without JavaScript

The screenshots on that page are useless, because they show everything collapsed (even stuff which normally isn't). The misalignment is caused by the JavaScript which removes the header "navigation" in the first sidebar box, see for instance in 1.18: http://wiki.wikimedia.it/wiki/Pagina_principale?useskin=vector

Misaligning the sidebar was probably the intended effect, so this is not really a bug; no idea what enhancements would be accepted.

Attached:

/skins/vector/components/navigation.less assign different margin values to div#mw-panel div.portal div.body and div#mw-panel div.portal.first div.body

The former uses the default @menu-main-body-margin (0 0 0 1.25em; } while the latter uses { margin-left: .5em } hence the .75em difference in indentation.

Caused by https://gerrit.wikimedia.org/r/#/c/131402/

I tried to improve the bug summary.

Change 134311 had a related patch set uploaded by Odder:
Fix indentation in Vector sidebar links

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

(In reply to Helder from comment #2)

(In reply to Helder from comment #1)

The current formatting also makes many items to wrap to a new line.

Ignore this comment, I was testing something on Firefox and set the minimum
font size to 13px. This was the cause of the wrapping...

Indeed. They were already aligned like this. The position is exactly the same as before we removed the collapsibleNav module.

(In reply to Helder from comment #4)

Compare the bugzilla sidebar to the sidebar on the wikis. On bugzilla, there
is no indentation for the uncollapsed sections. This is how it should look
like on the wikis as well.
Also, see the image on
https://www.mediawiki.org/wiki/Wikimedia_Foundation_Design/
Whitespace#Alignment
Before the code update, there was a image in the left of the headers, and in
that case the margin(padding?) makes sense, to make the items aligned with
the header. Now, there is no icon, so there should be no white space either.

Note that the Bugzilla sidebar has another important difference. It has the separator lines between the header and the items. Not between the sections.

Bugzilla (and how Vector was in 2009):

Header


Item
Item

Header


Item
Item

MediaWiki Vector (2010-2013)

\/ Header

  Item
  Item

--------

\/ Header

Item
Item

MediaWiki Vector (now)

Header

  Item
  Item

--------

Header

Item
Item

(In reply to Krinkle from comment #11)

To be more explicit:
MediaWiki Vector (now)

Item
Item


Header

  Item
  Item

--------

Header

Item
Item

(In reply to Helder from comment #12)

(In reply to Krinkle from comment #11)
To be more explicit

Indeed, the first section has no heading and is not indented. However, that also has been the case since 2010.

I think we need design input (which I can provide, but I'd like a second pair of eyes on this) because this is not a technical matter.

My argument is:

  • Aligning header and items is fine (e.g. no extra indentation), but in that case we should move the separator line back to between the header and the items (like it was in 2009, and how it still is on Bugzilla). Otherwise it gets too crammy and too close to each other.
  • The current proposed patch set (I0c880c1a31dae6c patch set 2) would in my opinion be a regression as it doesn't align the items, nor does it keep separation. What it does is remove the indentation but keeps the margins as they are which means they now neither have spacial separation, nor a line separation, and don't have alignment either. They almost have any of them, but exactlt have nothing. That patch changes it to:

    ---- Heading Item Item

The space here in front of "Item" is exaggerated, it basically makes it almost match but not exactly (see http://i.imgur.com/AmmLMjc.png), and removes a lot of the visual separation, breathing room and decluttering.

(In reply to Krinkle from comment #13)

  • Aligning header and items is fine (e.g. no extra indentation), but in that

case we should move the separator line back to between the header and the
items (like it was in 2009, and how it still is on Bugzilla). Otherwise it
gets too crammy and too close to each other.

Makes sense.

The space here in front of "Item" is exaggerated, it basically makes it
almost match but not exactly (see http://i.imgur.com/AmmLMjc.png), and
removes a lot of the visual separation, breathing room and decluttering.

Agreed, that screenshot is no good.

TheDJ added a comment.May 26 2014, 3:31 PM

Created attachment 15476
What it used to be

Since I have access to an instance with a somewhat older version of vector, a screeshot of how it once was.

Attached:

That can be achieved by undoing parts of https://gerrit.wikimedia.org/r/#/c/131402/. There will likely still be parts of that change that should remain, and some parts that were changed/removed at other times that may need restoring.

Change 134311 abandoned by Odder:
Fix indentation in Vector sidebar links

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

He7d3r updated the task description. (Show Details)Dec 22 2015, 1:27 PM
He7d3r set Security to None.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 22 2015, 1:27 PM
Jorm removed a subscriber: Jorm.Dec 26 2015, 7:23 PM
Catrope removed a subscriber: Catrope.Jan 15 2016, 10:46 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 27 2016, 4:22 PM

I want to solve this issue, Can anyone give more information about it

Hi @Rammanojpotla and thanks for your interest! Please see previous comments (and linked patches) and if anything is unclear, please ask. Thanks a lot!

Krinkle removed a subscriber: Krinkle.Mar 25 2017, 1:05 AM

I have read the above messages does it conclude that the the solution to the present issue is modifying the present vector skin to screenshot2 provided above?

BeforeAfter

(There's a patch for this at https://gerrit.wikimedia.org/r/#/c/344976/. I have no opinion on this change myself.)

@matmarex Just to clarify this: Are the screenshots "This is how it looks now (before) and that is something I've quickly made up with browser devtools and what I think it should look like after a fitting patch has been applied (after)" or is this a "This is how it looks now (before) and that is how it looks with patch 344976 applied"?

Yes, that's before and after 344976.

Thanks for clarification!

BeforeAfter

Attached:

Both of these look good to me. Important details:

  • Space to the left of the menu items is removed.
  • Separator line between menus is replaced with equal-sized space without separator line.
  • Separator line is now between section title and section items.

These three together make it work. Otherwise the menu items (links) and menu title (heading) are too similar and too close to each other.

@Aklapper can you please review the patch for the issue T67444 in https://gerrit.wikimedia.org/r/#/c/344976/ ?

EddieGP moved this task from Backlog to Done on the good first bug board.

@Rammanojpotla Is there a reason that you ask specifically Aklapper to review this? Around here, usually people who are familar with this specific part of code or the overall product (here: Vector) are "responsible" for reviewing patches. I don't really think it's Andres job to review this: As this is a style change in the default mediawiki skin, it's most likely something the web reading team will review this. I'm being bold and add this to their sprint, this will very likely make them notice it and review this.

Jdlrobson added a subscriber: Jdlrobson.

Thanks for adding the tag @EddieGP. I've also added unplanned sprint work tag.
We usually document the why we add something to a sprint so let me do that now:
Reading team has been asked to okay the change illustrated in https://phabricator.wikimedia.org/T67444#3145102 (removal of ident).

@Jdlrobson Would you mind to explain why you added unplanned sprint work in addition? The description of that tag is "Allows teams to track any work that enters a sprint after it begins.", and Reading-Web-Sprint-95 says "Sprint Start Dat Wed, Apr 5, 07:00 ", which means I added this to the sprint a few days before it started. I guess Reading team uses this tag different, so it would be nice if there's any explanation (or hint to documentation) how you use it, because it isn't obvious to me from the description of Unplanned-Sprint-Work .

Jdlrobson added a subscriber: Nirzar.EditedApr 4 2017, 6:27 PM

@EddieGP sprint 95 hasn't started yet, so it's best to throw it in our current sprint (94) as unplanned sprint. We can then decide what to do with it. Feel free to email me off Phabricator/or email if you want to understand how our process works a little better.

I've checked with @Nirzar and he's fine with the change so I've merged the fix.

Change 344976 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Proper indentation applied to horizontal links in the vector skin

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

Jdlrobson closed this task as Resolved.Apr 4 2017, 6:47 PM
Jdlrobson claimed this task.

Verified on beta cluster.

Nikerabbit reopened this task as Open.Apr 6 2017, 7:48 AM
Nikerabbit added a subscriber: Nikerabbit.

This causes the ULS cog to be positioned incorrectly in the sidebar. I am looking how to fix this, but I might need help figuring out how to make the positioning more generic, or how to detect presence of this change reliably, or have this change reverted before next deployment train if no fix is ready at at that time.

Change 346752 had a related patch set uploaded (by Nikerabbit):
[mediawiki/extensions/UniversalLanguageSelector@master] Try to make cog positioning more robust

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