Page MenuHomePhabricator

ToC: Layout spec
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

Layout changes from design review

Specification

we will now be supporting 3 layouts:

  1. article page with toc
  2. article page without toc
  3. special pages that need full width

specs of layouts at various widths:

demo: https://di-layout-spec.web.app

article page with toc (example)article page without toc (example)special pages that need full width (example)
Wider than outter container max
image.png (1×3 px, 193 KB)
image.png (1×3 px, 159 KB)
image.png (1×3 px, 141 KB)
At outter container max
image.png (1×3 px, 188 KB)
image.png (1×3 px, 159 KB)
image.png (1×3 px, 141 KB)
Below outter container max
image.png (1×3 px, 189 KB)
image.png (1×3 px, 155 KB)
image.png (1×3 px, 141 KB)
Below content max
image.png (1×3 px, 194 KB)
image.png (1×3 px, 142 KB)
image.png (1×3 px, 141 KB)

regarding the table of contents specifically:

image.png (1×2 px, 179 KB)

our current layout (e.g. https://en.wikipedia.org/wiki/Finland?useskin=vector-2022&tableofcontents=true) seems pretty close to this already. a few things (but not a comprehensive list) to look into:

  • reduce the max-width of the outer container
  • check/adjust the spacing between the header and the content area
  • check/adjust the width of the left sidebar
  • check/adjust layout of toc within left sidebar
  • check/adjust the spacing around the toc
  • create layout for article pages with no toc (page content gets centered in that case)
  • check/adjust the width of the logo container (should match sidebar width)

QA Results - Prod

Event Timeline

ovasileva created this task.
ovasileva added a subscriber: alexhollender_WMF.

Sidebar fixed width at 296px with the TOC width set to 244px:

Current sidebar for Vector is computed in https://github.com/wikimedia/Vector/blob/master/resources/common/variables.less#L115-L118:

// Sidebar
@margin-toc-start-content: 16em;
@space-end-sidebar: 2em;
@width-sidebar: @margin-toc-start-content - @space-end-sidebar;

With base font-size at 16px, @width-sidebar works out to be 256px - 32px = 224px which is confirmed upon inspection on the Vector main branch.

To increase TOC width to 244px and sidebar width to 296px, we can set the following:

// Sidebar
@margin-toc-start-content: 18.5em;
@space-end-sidebar: 3.25em;
@width-sidebar: @margin-toc-start-content - @space-end-sidebar;

The mocks also indicate an increase on the padding of the main page container which is set by https://github.com/wikimedia/Vector/blob/master/resources/common/variables.less#L175 - this can be increased to unit( 40px / @font-size-browser, em ).

To make the header max-width set to 1514px (set by https://github.com/wikimedia/Vector/blob/master/resources/common/variables.less#L174), @max-width-page-container can be increased to unit( 1514px / @font-size-browser, em ).

To make the logo area + hamburger menu occupy the same width as the sidebar, we can add a min-width to the .mw-logo selector. The hamburger menu + right padding take up 58px which leaves 238px for the logo area. In em's the width can be set to 14.375em:

  • if we apply a 12px (0.75em) left margin to the entire TOC container (.mw-table-of-contents-container) to get a right margin of 40px
  • if we take into account that the searchbox in the header applies a left margin to its div.

Question whether we need to set a max-width? >> answered below in T305069#7832390 -- set only min-width for the time being.

One piece I'm not sure about is getting the top of the TOC in horizontal alignment with the article title - can discuss during grooming.

Preliminary patch below covers most of these adjustments. There may need some more refining with the lining up of the arrows with the hamburger menu icon.

All in all the style updates should be straightforward and just need eagle eyes to confirm precision with mocks.

Change 777004 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/skins/Vector@master] Update TOC layout styles

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

cjming added a subscriber: cjming.

hi @alexhollender_WMF a few clarifying questions:

  • For the max-width of the outer container, setting the header max-width to 1514px with 40px padding on each side brings the total outer container with to 1594px -- is this ok? you specified 1596px in the mocks.
  • For the logo area, do you want to define a max-width for it? also is the idea that the search box should line up with the left edge of the main content area?

hi @alexhollender_WMF a few clarifying questions:

  • For the max-width of the outer container, setting the header max-width to 1514px with 40px padding on each side brings the total outer container with to 1594px -- is this ok? you specified 1596px in the mocks.

ah yes, that's fine. realizing the 2px difference here is because I have a 1px border on the right & left of the outer container

  • For the logo area, do you want to define a max-width for it? also is the idea that the search box should line up with the left edge of the main content area?

I think we decided that a max-width might be risky because it might result in clipping some logos, but that we can at least add a min-width for now. And yes, the idea is that the search box should line up with the left edge of the main content area, so the min-width of the logo area should aim to ensure that happens (aside of course for cases where the logo is so wide that it pushes everything over)

LGoto updated Other Assignee, added: Jdlrobson.

@cjming's POC patch looked pretty good to me and involves minimal additions to the code. I uploaded the visual diff between the HEAD of master (reference) and the POC on top of master (test) at: https://624c8c80318db418c250fe76--stirring-douhua-ac8a6e.netlify.app/html_report/

Namely, there were a couple things I noticed

Personally, I'd be okay with continuing with Clare's patch without the CSS grid ticket if it continues to mostly involve variable changes and minimal additional lines of code

nray removed nray as the assignee of this task.Apr 5 2022, 7:10 PM
nray added a subscriber: nray.
  • The tabs have been moved down which I'm not sure is the desired output. @alexhollender_WMF the spec shows 32px of space between the header and the content, but the POC has 32px between the header and the top of the tabs. Should it be the former or the latter?

this prototype has the tabs and is to spec, does that help clarify things? https://di-toc-with-menu-above.web.app/Tuna

Screen Shot 2022-04-06 at 10.22.56 AM.png (1×2 px, 838 KB)

I've also looked at @cjming patch. I'm happy to go ahead with the proposal there. I think it shouldn't impact the grid work which will need to revise a lot of this anyway. Nick's UI regression testing suite should flag less regressions if this work is done first. Happy with a small or medium estimate per our earlier discussion.

@alexhollender_WMF yes, thank you and sorry I completely forgot about the prototype! That is super helpful

Test wiki created on Patch demo by Jdlrobson using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/dd4dd325df/w/

Clare my review might take a little longer here as I wanted to use this as an excuse to try out Nick's UI regression suite (here I'm looking for a UI progression :-))

Change 778599 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/skins/Vector@master] Style fixes for sticky TOC

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

hi @alexhollender_WMF -- some more follow up Qs:

Regarding the last screenshot in the ticket when you specify that the border edge of the TOC should vertical align with the edge of the hamburger icon -- can you confirm this is what you meant?

Option 1

with gridlines:

Screen Shot 2022-04-08 at 3.53.52 PM.png (1×938 px, 211 KB)

without gridlines while hovering over hamburger menu:

Screen Shot 2022-04-08 at 3.59.14 PM.png (1×984 px, 229 KB)

without gridlines while NOT hovering over hamburger menu:

Screen Shot 2022-04-08 at 4.03.36 PM.png (1×966 px, 211 KB)

Versus having the hover border edge of the hamburger icon vertical align with the border edge of the TOC instead i.e.

Option 2

with gridlines:

Screen Shot 2022-04-08 at 3.52.34 PM.png (1×962 px, 219 KB)

without gridlines while hovering over hamburger menu:

Screen Shot 2022-04-08 at 3.52.18 PM.png (1×924 px, 199 KB)

without gridlines while NOT hovering over hamburger menu:

Screen Shot 2022-04-08 at 4.05.59 PM.png (1×942 px, 196 KB)

I'm erring on Option 1 in my latest patch that combines updates for this ticket and the visual changes task T304166

Also, with the horizontal alignment of the top of the TOC with the article title underline, should this also apply to the sidebar menu as well when it's toggled and rendered above the TOC? I err'd yes on the patch but it can easily be reverted if this horizontal alignment should only apply when the TOC is at the top of the sidebar.

Change 778599 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/skins/Vector@master] Style fixes for sticky TOC

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

Doesn't look like the changes are working on 1200px but that could be my misunderstanding. Looks good at normal desktop breakpoint though!

Screen Shot 2022-04-08 at 5.40.31 PM.png (702×2 px, 243 KB)

@nray I tried to publish the UI regression results here (https://jdlrobson.github.io/T305069/) but the paths are relative and 404. Is there an easy way to fix that?

hi @alexhollender_WMF -- some more follow up Qs:
Regarding the last screenshot in the ticket when you specify that the border edge of the TOC should vertical align with the edge of the hamburger icon -- can you confirm this is what you meant?
with gridlines:

Screen Shot 2022-04-08 at 3.53.52 PM.png (1×938 px, 211 KB)

yes, this is correct

Also, with the horizontal alignment of the top of the TOC with the article title underline, should this also apply to the sidebar menu as well when it's toggled and rendered above the TOC?

the menu can be aligned higher up, screenshot below also demo if helpful here: https://di-toc-with-menu-above.web.app/Tuna

image.png (1×2 px, 689 KB)

Change 777004 abandoned by Clare Ming:

[mediawiki/skins/Vector@master] Update TOC layout styles

Reason:

abandoning in favor of combing changes in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/778599

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

Test wiki created on Patch demo by CMing (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/7bf0454258/w/

hi @alexhollender_WMF re: your updated reqs for special pages that need full width - the first column of screenshots under article page with toc indicate that the max-width of the header element should be 1514px -- should the header and main content stretch to the full container beyond 1514px?

This is what a special page looks like with header + main content currently constrained at max-width 1514px within a 2500+px viewport:

Screen Shot 2022-04-11 at 5.13.22 PM.png (1×5 px, 541 KB)

You're saying that the header + content containers should stretch to fill the entire viewport (minus 40px left/right margins)?

@cjming this is looking great. notes:

Sticky position of ToC
I apologize for not adding this to the spec to begin with. The ToC should stick 24px from the top of the screen, or 24px from the bottom of the sticky header when logged-in. This seems to already be correct if you've opened the main menu then scrolled down, but when the main menu is closed and you scroll down the ToC sticks a bit too far down:

logged-outlogged-in
menu openmenu closedmen openmenu closed
correctincorrectcorrectincorrect
image.png (918×1 px, 328 KB)
image.png (918×1 px, 330 KB)
image.png (918×1 px, 284 KB)
image.png (918×1 px, 329 KB)

Special pages that need full width
Some special pages (e.g. Recent changes, History, Contributions) do not have a max-width on the content area. In those cases the content area should match the width of the header. Currently it is constrained to 1440px via .mw-workspace-container, which I think we could remove at this point.

currentexpected
image.png (2×3 px, 1 MB)
image.png (2×3 px, 1 MB)

I think removing .mw-workspace-container would also help with the consistency of pages that don't have a ToC, because the main menu would be aligned the same as it is for pages that do have a ToC. For example:

with 1440px maxwithout 1440px max
image.png (1×3 px, 609 KB)
image.png (1×3 px, 563 KB)

Screen Shot 2022-04-11 at 5.13.22 PM.png (1×5 px, 541 KB)

You're saying that the header + content containers should stretch to fill the entire viewport (minus 40px left/right margins)?

yes, that's correct ✅

Test wiki on Patch demo by CMing (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/7bf0454258/w/

Test wiki created on Patch demo by CMing (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/1bfd991efc/w/

Test wiki on Patch demo by CMing (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/1bfd991efc/w/

Test wiki created on Patch demo by CMing (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/7eb47c98e2/w/

@cjming the patch looks good and the improvements are noticeable. I do think the horizontal alignment for both sidebar and ToC. LGTM!

Test wiki on Patch demo by CMing (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/7eb47c98e2/w/

Test wiki created on Patch demo by CMing (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/c0e2e484ef/w/

Test wiki created on Patch demo by CMing (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/995593dce7/w/

Change 778599 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Layout + visual style changes for sticky TOC

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

Change 782404 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/skins/Vector@master] Vertically align header searchbox with main content text

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

Change 782473 had a related patch set uploaded (by Clare Ming; author: Clare Ming):

[mediawiki/skins/Vector@master] Adjust alignment of searchbox in header

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

Change 782404 abandoned by Clare Ming:

[mediawiki/skins/Vector@master] Vertically align header searchbox with main content text

Reason:

in favor of https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/782473

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

Test wiki on Patch demo by CMing (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/995593dce7/w/

Test wiki on Patch demo by CMing (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/c0e2e484ef/w/

Test wiki created on Patch demo by CMing (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/33de85e58a/w/

Sticky position of ToC
I apologize for not adding this to the spec to begin with. The ToC should stick 24px from the top of the screen, or 24px from the bottom of the sticky header when logged-in. This seems to already be correct if you've opened the main menu then scrolled down, but when the main menu is closed and you scroll down the ToC sticks a bit too far down:

logged-outlogged-in
menu openmenu closedmen openmenu closed
correctincorrectcorrectincorrect
image.png (918×1 px, 328 KB)
image.png (918×1 px, 330 KB)
image.png (918×1 px, 284 KB)
image.png (918×1 px, 329 KB)

hi @alexhollender_WMF just a note about the logged-in use case at viewports below 1000px -- the top margin of the TOC should be fixed by this patch that's part of T300612. It can be pulled out into its own patch and hopefully merged right away (cc @bwang) if the sticky header js patch needs more revamping/code review.

Other than this known issue, the changes requested in this ticket + T304166 can be reviewed together at https://patchdemo.wmflabs.org/wikis/33de85e58a/wiki/TestTOCupdates?useskin=vector-2022&tableofcontents=1

Thanks for your patience and props to Bernard + Nick for shining bright lights on my blind spots to get these changes merged.

looks great, awesome work @cjming

hi @alexhollender_WMF just a note about the logged-in use case at viewports below 1000px -- the top margin of the TOC should be fixed by this patch that's part of T300612. It can be pulled out into its own patch and hopefully merged right away (cc @bwang) if the sticky header js patch needs more revamping/code review.

sounds good, thanks for the heads up

Change 782473 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Adjust alignment of searchbox in header

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

@alexhollender_WMF @ovasileva The following link shows the visual difference report between last week's release (1.39.0-wmf.7) and this week's upcoming release (1.39.0-wmf.8). It includes the work from this ticket:

https://visionary-halva-dfd216.netlify.app/

The "reference" column shows screenshots from 1.39.0-wmf.7, while the "test" column shows screenshots from 1.39.0-wmf.8. You can click on the red button to only include screenshots where there were visual differences detected. The button says "failed" because it assumes 1.39.0-wmf.7 is the "correct" version, but hopefully the "test" screenshots are "correct". If not, please let us know 😊

Not sure if its helpful as its more noisy, but for kicks I also added a report of the visual diff between 1.39.0-wmf.6 and 1.39.0-wmf.8 which additionally includes the earliest work from T304166: https://glittering-zuccutto-a35151.netlify.app/

Test Result - Prod

Status:
Environment: enwiki
OS: macOS Monterey
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps
✅ AC1: article page with toc (example)✅ AC2: article page without toc (example)✅ AC3: special pages that need full width (example)
Wider than outter container max
Screen Shot 2022-04-26 at 7.04.03 PM.png (516×309 px, 38 KB)
Screen Shot 2022-04-26 at 7.04.27 PM.png (257×307 px, 35 KB)
Screen Shot 2022-04-26 at 7.03.06 PM.png (293×278 px, 29 KB)
Screen Shot 2022-04-26 at 7.15.56 PM.png (268×409 px, 35 KB)
Screen Shot 2022-04-26 at 7.07.28 PM.png (1×1 px, 504 KB)
Screen Shot 2022-04-27 at 3.32.36 PM.png (472×694 px, 68 KB)
Screen Shot 2022-04-27 at 3.32.11 PM.png (584×554 px, 73 KB)
Screen Shot 2022-04-27 at 3.31.54 PM.png (810×580 px, 103 KB)
Screen Shot 2022-04-27 at 3.33.03 PM.png (1×2 px, 574 KB)
Screen Shot 2022-04-27 at 3.52.17 PM.png (694×766 px, 74 KB)
Screen Shot 2022-04-27 at 3.50.56 PM.png (712×610 px, 84 KB)
Screen Shot 2022-04-27 at 3.50.46 PM.png (574×718 px, 81 KB)
Screen Shot 2022-04-27 at 3.51.30 PM.png (940×832 px, 140 KB)
At outter container max
Screen Shot 2022-04-26 at 7.18.50 PM.png (226×342 px, 28 KB)
Screen Shot 2022-04-26 at 7.17.37 PM.png (240×324 px, 31 KB)
Screen Shot 2022-04-26 at 7.18.32 PM.png (302×275 px, 30 KB)
Screen Shot 2022-04-26 at 7.17.31 PM.png (567×325 px, 39 KB)
Screen Shot 2022-04-26 at 7.19.35 PM.png (1×1 px, 435 KB)
Screen Shot 2022-04-27 at 3.37.16 PM.png (574×484 px, 69 KB)
Screen Shot 2022-04-27 at 3.37.32 PM.png (698×564 px, 89 KB)
Screen Shot 2022-04-27 at 3.39.03 PM.png (916×2 px, 518 KB)
Screen Shot 2022-04-27 at 3.39.18 PM.png (1×3 px, 590 KB)
Screen Shot 2022-04-27 at 5.52.57 PM.png (295×258 px, 27 KB)
Screen Shot 2022-04-27 at 5.53.36 PM.png (272×352 px, 26 KB)
Screen Shot 2022-04-27 at 5.54.05 PM.png (1×1 px, 418 KB)
Screen Shot 2022-04-27 at 5.53.24 PM.png (786×574 px, 154 KB)
Screen Shot 2022-04-27 at 5.57.27 PM.png (239×320 px, 27 KB)
Below outter container max
Screen Shot 2022-04-26 at 7.26.58 PM.png (295×279 px, 29 KB)
Screen Shot 2022-04-26 at 7.26.14 PM.png (233×336 px, 29 KB)
Screen Shot 2022-04-26 at 7.26.38 PM.png (232×301 px, 30 KB)
Screen Shot 2022-04-26 at 7.27.14 PM.png (510×309 px, 38 KB)
Screen Shot 2022-04-26 at 7.27.36 PM.png (1×1 px, 425 KB)
Screen Shot 2022-04-27 at 3.43.42 PM.png (718×564 px, 91 KB)
Screen Shot 2022-04-27 at 3.43.28 PM.png (570×502 px, 70 KB)
Screen Shot 2022-04-27 at 3.43.06 PM.png (766×746 px, 103 KB)
Screen Shot 2022-04-27 at 3.44.06 PM.png (1×2 px, 549 KB)
Screen Shot 2022-04-27 at 5.55.13 PM.png (287×245 px, 27 KB)
Screen Shot 2022-04-27 at 5.55.33 PM.png (274×350 px, 28 KB)
Screen Shot 2022-04-27 at 5.56.04 PM.png (1×1 px, 351 KB)
Screen Shot 2022-04-27 at 5.55.19 PM.png (787×380 px, 99 KB)
Below content max
Screen Shot 2022-04-26 at 7.31.53 PM.png (238×332 px, 29 KB)
Screen Shot 2022-04-26 at 7.31.17 PM.png (302×282 px, 29 KB)
Screen Shot 2022-04-26 at 7.31.38 PM.png (235×282 px, 26 KB)
Screen Shot 2022-04-26 at 7.32.03 PM.png (521×317 px, 38 KB)
Screen Shot 2022-04-26 at 7.32.31 PM.png (1×1 px, 353 KB)
Screen Shot 2022-04-27 at 3.47.20 PM.png (592×542 px, 75 KB)
Screen Shot 2022-04-27 at 3.45.33 PM.png (466×626 px, 64 KB)
Screen Shot 2022-04-27 at 3.45.24 PM.png (578×506 px, 73 KB)
Screen Shot 2022-04-27 at 3.46.21 PM.png (1×1 px, 344 KB)
Screen Shot 2022-04-27 at 5.59.25 PM.png (68×313 px, 9 KB)
Screen Shot 2022-04-27 at 5.59.41 PM.png (254×316 px, 26 KB)
Screen Shot 2022-04-27 at 5.59.05 PM.png (295×262 px, 28 KB)
Screen Shot 2022-04-27 at 6.00.03 PM.png (1×909 px, 262 KB)
Screen Shot 2022-04-27 at 5.59.49 PM.png (790×401 px, 99 KB)

✅ AC4: regarding the table of contents specifically:

Screen Shot 2022-04-27 at 6.29.52 PM.png (721×631 px, 133 KB)

Screen Shot 2022-04-27 at 6.00.03 PM.png (1×909 px, 262 KB)

Screen Shot 2022-04-27 at 6.27.05 PM.png (1×1 px, 522 KB)

@Jdlrobson, I'm not sure if the gradient protruding from the bottom left of the TOC is a defect related to this task or not. I'm highlighting it here but also communicated it to the team. If the rest of the task looks satisfactory and you feel that the gradient issue below is a different task, let me know and I'll pass this one.

Screen Shot 2022-04-27 at 6.34.07 PM.png (775×809 px, 160 KB)

ovasileva claimed this task.

All done here, resolving!