Page MenuHomePhabricator

Restyle table of contents
Closed, ResolvedPublic3 Estimated Story Points

Assigned To
Authored By
KHammerstein
Sep 23 2015, 11:14 PM
Referenced Files
F2958247: IMG_7032.PNG
Nov 13 2015, 12:10 AM
F2958160: IMG_0370.PNG
Nov 12 2015, 11:22 PM
F2951981: Screenshot 2015-11-10 15.17.43.png
Nov 10 2015, 8:31 PM
F2951984: Screenshot 2015-11-10 15.15.42.png
Nov 10 2015, 8:31 PM
F2951877: contents-spacing-13.png
Nov 10 2015, 7:33 PM
F2924287: IMG_0350.PNG
Nov 7 2015, 12:17 AM
F2924292: IMG_0351.PNG
Nov 7 2015, 12:17 AM
F2924290: IMG_0349.PNG
Nov 7 2015, 12:17 AM

Description

Design

contents-03.png (1×643 px, 93 KB)

Typography

contents-05.png (1×1 px, 157 KB)

Spacing

contents-spacing-13.png (1×1 px, 155 KB)

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.

Event Timeline

KHammerstein raised the priority of this task from to Needs Triage.
KHammerstein updated the task description. (Show Details)
KHammerstein subscribed.
JMinor triaged this task as Medium 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?

IMG_0322.PNG (2×1 px, 215 KB)

IMG_0321.PNG (2×1 px, 233 KB)

IMG_0320.PNG (2×1 px, 196 KB)

IMG_0319.PNG (2×1 px, 169 KB)

@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?

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!

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

IMG_0349.PNG (2×1 px, 272 KB)

IMG_0350.PNG (2×1 px, 162 KB)

IMG_0351.PNG (2×1 px, 153 KB)

@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 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:

Screenshot 2015-11-10 15.17.43.png (362×764 px, 54 KB)

Screenshot 2015-11-10 15.15.42.png (252×733 px, 40 KB)

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"?

IMG_0370.PNG (2×1 px, 182 KB)

@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:

IMG_7032.PNG (1×750 px, 127 KB)

@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.