Page MenuHomePhabricator

Refactor Table of Contents styles
Closed, ResolvedPublic

Description

The table of contents in Vector 2022 can appear in 4 different places:

Screen Shot 2022-09-16 at 11.59.44 AM.png (635×1 px, 347 KB)
Screen Shot 2022-09-16 at 11.59.52 AM.png (608×1 px, 334 KB)
Screen Shot 2022-09-16 at 12.00.06 PM.png (451×1 px, 189 KB)
Screen Shot 2022-09-16 at 12.00.18 PM.png (457×768 px, 173 KB)
In the sidebarBeside the page titleIn the sticky headerPinned to the edge of the screen

The code for these positions currently resides in tableOfContents.less and tableOfContentsCollapsed.less.

We should draw clearer distinctions between these 4 different ToC states by separating out the code responsible for them. The logic can be split into 5 different files:

  • tableOfContentsBase.less - containing the styles for the actual ToC list items, as well as any hide/show behaviour shared across the different placements.
  • tableOfContentsSidebar.less - styles for the ToC in the sidebar.
  • tableOfContentsHeader.less - styles for the ToC near the page title.
  • tableOfContentsStickyHeader.less - styles for the ToC when it's in the sticky header.
  • tableOfContentsPinned.less - styles for the ToC when it's pinned to the edge of the screen, at lower resolutions.

For the most part, the logic determining these placements is based on breakpoints and whether the ToC has been "hidden".

IF viewport greater than 1000px:
hidden false: in sidebar
hidden true: in header AND in sticky header

IF viewport less than 1000px:
hidden false: in header
hidden true: in header
scrolled below title: pinned to edge.

This refactor should structure the code in a way that clarifies this logic.

Pixel tests should also be added to account for the different placements and expanded/collapsed states.

Event Timeline

@Jdrewniak can this be resolved or updated? I know we dont have the CSS structure you described in the task description, but we do have 3 files for the TOC ATM.
TableofContents.less is equivalent to tableOfContentsBase.less as you described above
layouts/toc/pinned.less contains code for tableOfContentsSidebar.less
layouts/toc/unpinned.less contains code for tableOfContentsHeader.less, tableOfContentsStickyHeader.less & tableOfContentsPinned.less

Personally I don't think we need separate files for each case, especially after Zebra, the TOC styles have been simplified to the point where most cases is handled by applying a single mixin. The cases you laid out also arent as distinct as the file names suggest. For example, the styles needed to achieve the case where the TOC is on the edge of the screen depends on whether or not the sticky header is visible, which overlaps with the logic for tableOfContentsHeader.less and tableOfContentsStickyHeader.less. The no js TOC case is similar, code relating to that case would need to be in multiple files, or in its own file based off the original structure you described in the task description. Instead, I would prefer a tableOfContents.less file as the "base" file, and a single layouts/tableOfContents.less file for all the states with good comments

Jdrewniak claimed this task.

@bwang yeah I think this can be closed given how the CSS has evolved since this ticket was written, and as you mention, moving towards an implementation that unifies and shares styles across the different states is also more preferable than the approach outlined in the task description.

@Jdrewniak I also think the more important task is to clarify the naming of all the TOC states, I think thats a far bigger source of confusion. Here's my understanding of the terms we have in our codebase already, and why I think we should use them this way.

Pinned TOC: IMO 'pinned' should always refer to 'pinned to the sidebar'. I know that we often use it in other ways, like 'pinned' referring to 'pinned to the page title' or 'pinned to the side of the page'. Considering how 'pinnable elements' is a generic term now that includes page tools and main menu, and how in those 2 cases 'pinned' always refers to the sidebar, i think we should also be consistent with the TOC which we treat as a pinnable element as well.

Unpinned TOC: This one is unfortunately complicated because when the TOC is unpinned, it can either be in the header, sticky header, or 'floating' in the top left. If there wasnt such thing as the 'floating' case, we could generalize this to just mean when the TOC is in the header (whether its the main or sticky header).

"Floating" TOC: In my mind this is a subcase of unpinned TOCs, when you are scroll down the page and there's no sticky header. I called it "floating" similar to the floating action buttons in Material design, but I dont know if there's a better name for this case. We cant call it sticky because the TOC is also sticky when its pinned

The terminology around pinning is a bit confusing.

I think the problem lays with with the word unpinned. This is description by negation -- it describes what the element isn't, not what it is. As you mention, it could be in multiple places when it's not pinned. Maybe a cleaner description of this state would be a dropdown TOC, since in all the cases where it's not pinned, it's behind a dropdown of some kind (floating, sticky header, or header).