Restyle table of contents
Closed, ResolvedPublic3 Story Points

Description

Design

Typography

Spacing

Current section covers entire parent section.
Bullet point shows current subsection

Background standard white out blur

Between h2 items there will be a line. But no line between h2 items and any h3 or h4 items in that section.
Between h2 items and line 20px.
Between h3 items and other h3 items 20px.
No indentation on h2 items, only on h3 items and above.

KHammerstein updated the task description. (Show Details)
KHammerstein raised the priority of this task from to Needs Triage.
KHammerstein added a subscriber: KHammerstein.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 23 2015, 11:14 PM
KHammerstein set Security to None.
MBinder_WMF edited a custom field.Sep 25 2015, 7:20 PM
JMinor triaged this task as Normal priority.Oct 2 2015, 3:23 AM

Checked with 5.0.0 (411) on iPad mini iOS 8.2 and iPhone 5 iOS 9.02.

Questions for Design review:

  • use of full-bleed dividers is ok since there is no ancoring elements?
  • the left indent looks more prominent in the mock-up?

@Fjalapeno

  • Spacing too tight on h2 sections - will include in spec
  • h3 sections should not be indented, only h4 and higher sections
  • Contents label missing

@Fjalapeno

  • Spacing too tight on h2 sections - will include in spec

Cool having the specific padding you want on sections that would be great

  • h3 sections should not be indented, only h4 and higher sections

Got it

  • Contents label missing

@KHammerstein

I believe @Mhurd removed the "Contents" header the other week when "Introduction" was changed to the title of the article. As far as I know this was done intentionally because having the word "contents" above the article title seems odd? Also I think he said it matches up with the Android implementation…

Moved to blocked - waiting on updated spec and a definitive answer about what goes into the header of the ToC.

@JMinor - specs are out of date for the top of the ToC since we removed "Introduction" and replaced with the title of the article. Can you work with @KHammerstein to get this updated taking in account those changes?

Fjalapeno reassigned this task from Fjalapeno to JMinor.Oct 30 2015, 7:26 PM

As far as I know this was done intentionally because having the word "contents" above the article title seems odd?

The issue was that some articles' first section is called "Introduction," so it looks like a dupe, see T115145 for more info.

Also I think he said it matches up with the Android implementation…

Correct

@Fjalapeno
Its fine to call Introduction the name of the article, I think that makes sense. But I'd like to keep the CONTENTS label, we also had it in legacy iOS app.
I added spec for spacing between items. Let me know if that makes sense or if you wana chat about it!

Fjalapeno removed JMinor as the assignee of this task.

Checked with Dev 5.0.0 (475)on iPad mini 8.2 - seems that all specs are in place.



KHammerstein added a comment.EditedNov 10 2015, 7:26 PM

@Fjalapeno
Make sure height between h2 items and lines is same as height between h3 items.
That height should be 20px, it looks to be around 10-15px now.

The lines should be between h2 sections. There shouldn't be lines between h2 and its h3 sections.

See spacing mock in description.

KHammerstein updated the task description. (Show Details)

@KHammerstein spacing between all elements is 20 points (this is 40 pixels retina) I looked at the code for a difference between the spacing of the h2 / h3 headers, and there wasn't one.

To be sure I measured actual pixels using xScope to get measurements. The actual number isn't important (31 pixels)… its just my computer screen so it isn't the same, but you can see the spacing is the same between h2 and h3 elements:

I can bump up the spacing to make it bigger if you want.

@KHammerstein

The lines should be between h2 sections. There shouldn't be lines between h2 and its h3 sections.

So the lines between "Lightning", "General considerations', and "Frequency of lightning", "Establishing conditions necessary for lightning" are ok? But not between, e.g. "Establishing conditions necessary for lightning" and "Establishing the electric field in CG lightning"?

@Etonkovidova Thats correct. I talked with @Fjalapeno about this and he said to create another task to capture the line issue - T118528

@Fjalapeno
I noticed that when you first open TOC, it scrolls down to the section you're currently in. That's good, but even if you are at the top of the article in the first section, the TOC is scrolled down slightly, so the "Contents" label is obscured.
Can we change it so that if you open the TOC when in the first section, the TOC is not scrolled down?

Example:

@KHammerstein
T115931: [Regression 5.0.0.492]ToC displays extra space when device is rotated from a horizontal position to vertical. reports on two issues

  • ToC scrolling up when rotating to horizontal
  • a small overlapping of 'Content' label
  • an extra space between 'Content' and the first item in the content after rotating to a vertical position.

@JMinor
Moving this to your sign off, though there are bugs and some other tasks that came up.

T118300 Rotating bug
T119019 Scrolling bug
T119024 Changing bg color of top area on Contents - responding to feedback that title was hard to distinguish

Thanks @KHammerstein I'll go ahead and resolve this and we'll address these tickets individually.

JMinor closed this task as Resolved.Nov 20 2015, 6:43 PM