Skip to content

Widen site grid - #3284

Merged
j-low merged 55 commits into
masterfrom
widen_site_grid
Jul 12, 2018
Merged

Widen site grid#3284
j-low merged 55 commits into
masterfrom
widen_site_grid

Conversation

@j-low

@j-low j-low commented Jun 19, 2018

Copy link
Copy Markdown
Contributor

[description]:

  • widen overall site grid
  • move TOC to righthand gutter
  • implement fixed and independently-scrollable sidenav

@cockroach-teamcity

Copy link
Copy Markdown
Member

This change is Reviewable

@j-low

j-low commented Jun 19, 2018

Copy link
Copy Markdown
Contributor Author

cc: @jseldess @lhirata @rmloveland @Amruta-Ranade

This first build implements the independently-scrollable sidenav which is QA-able at this point. I have not merged or duplicated the other sidenav changes from the work performed last week.

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@jseldess

Copy link
Copy Markdown
Contributor

Scrollable sidenav looks to be working great, @j-low. I don't see the page toc moved to the right gutter, though. Is that still coming?

@lnhsingh

lnhsingh commented Jun 20, 2018

Copy link
Copy Markdown
Contributor

For the independently scrolling side nav, is there a way to lock the minimum size of the sidebar container? What I'm noticing is that it gets squished when you scroll down to the bottom of the main content / footer.

For example:

screen shot 2018-06-20 at 8 34 10 am

Let me know if you need clarification!

@cockroach-teamcity

Copy link
Copy Markdown
Member

@rmloveland

Copy link
Copy Markdown
Contributor

Hey @j-low thanks for working on this! Really looking forward to a wider layout on the site.

Unfortunately I have one issue: The new layout causes the content text not to wrap to fit inside the browser window when the browser is using half the screen in a "2-up" configuration with another app (screenshot attached).

screenshot 2018-06-20 10 59 40

In other words, it seems like this layout really wants the browser to be full-screened or at least prefer to be a horizontal rectangle rather than a vertical rectangle. I suspect this might break for some developers who read docs on a vertical monitor (something I've seen around), or for people like me who work in side-by-side windows.

@j-low

j-low commented Jun 20, 2018

Copy link
Copy Markdown
Contributor Author

@rmloveland thanks for the QA! That's actually a known issue that's being worked on. It just didn't make it into the last commit.

@cockroach-teamcity

Copy link
Copy Markdown
Member

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/440bc9dc167f666cadf4a58a47f1ada6c33a55f3/

@j-low

j-low commented Jun 20, 2018

Copy link
Copy Markdown
Contributor Author

Ignore that last commit message; that was meant for a Hugo commit. There is another update coming with the actual QA feedback implemented.

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@j-low

j-low commented Jun 20, 2018

Copy link
Copy Markdown
Contributor Author

The most recent build represents the first version ready for QA. There are a few judgment calls I made we should discuss, e.g.:

  • I added a check for a no_toc variable in the front matter to hide/show the righthand TOC column (there might be a more ideal way of doing this)
  • Below 1072px the right hand TOC goes away and is instead rendered in its old position; this is because the screen real estate begins to get very narrow and those extra gutter pixels are needed to maintain a functional layout.

There are likely other things that will probably come up in the QA process but perhaps we can have a brief stakeholder check-in early next week to run through it all.

@rmloveland

Copy link
Copy Markdown
Contributor

@j-low I love the new layout! The highest compliment I can pay is this: once this is out I can stop using my custom page-widening CSS for the site (which was sorta broken anyway as I'm not a developer). Thank you!

@jseldess

jseldess commented Jun 22, 2018

Copy link
Copy Markdown
Contributor

This is looking awesome, @j-low!

A few observations and requests:

@j-low

j-low commented Jun 25, 2018

Copy link
Copy Markdown
Contributor Author

@jseldess great stuff, thanks for the feedback. I like all the suggestions. The one about removing the green background @kuanluo had also suggested too but I had forgotten to remove it in this PR. Bullets can definitely go. I will experiment with making the TOC itself scrollable as well as removing all the extra whitespace. I will notify the docs channel when the PR is updated.

One question re: the green background: do we want it to be removed when the screen is resized and the TOC is rendered in the original area?

@rmloveland glad this all seems to be an improvement!

@kuanluo

kuanluo commented Jun 25, 2018

Copy link
Copy Markdown

@j-low @jseldess I echo all of Jesse's feedback.

Re: the green background: do we want it to be removed when the screen is resized and the TOC is rendered in the original area?

When the screen is resized and TOC is rendered in the original area, we can also get rid of the green background, depend on the way it's redesigned. In Firebase's case, for example, the right blue sidebar works well in smaller screens.

Reduce the whitespace between the sidenav and the main content, and between the topnav and the rest of the page. Especially now that we're using so much horizontal space, the padding feels excessive.

Agreed. @j-low and I discussed further tweaks to reduce the whitespace, as well as the line-height.

What do you think about using a left border like we currently do for nested selected items in the sidenav. Also an example: https://firebase.google.com/docs/android/setup

Yes.

@j-low, can you tweak the look and feel of the TOC and see how it goes?

  • Remove the green background & bullet points.
  • Add the vertical line on the secondary bullets, like the side nav. The first level items don't need the line.
  • Reduce line-height to 14px.
  • Move the TOC down to top-align with Overview, or whatever the first H1 is.

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@j-low

j-low commented Jul 2, 2018

Copy link
Copy Markdown
Contributor Author

@jseldess PR updated with the changes we discussed earlier (e.g. TOC sub-item highlighting, merge conflicts resolved; first tier nav items default to open on all versions).

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@lnhsingh

lnhsingh commented Jul 3, 2018

Copy link
Copy Markdown
Contributor

This looks great!

One minor thing I noticed is when you use search, the version toggle disappears. Since the search is limited to the selected version, it would be helpful to have an indicator of what version you're looking at.

@j-low

j-low commented Jul 10, 2018

Copy link
Copy Markdown
Contributor Author

@lhirata I believe that feature bug has been in place for some time. I think this is something we could tackle in the near-ish future. @jseldess and I spoke about it a few months ago in the context of a broader discussion about better visibility around versioning-- search results page included.

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

Previously, there were multiple ways for a page to have a toc.
Now, the only way is if `toc: true` is in the front-matter.

This PR makes this change and adds the script that was used
to do so.
@jseldess

Copy link
Copy Markdown
Contributor

@j-low, all pages now either have toc: true or toc: false. You can now put in log to show the toc in the right column only if toc: true.

@cockroach-teamcity

Copy link
Copy Markdown
Member

@jseldess jseldess changed the title [WIP] widen site grid Widen site grid Jul 11, 2018
…atter in default template; refactor CSS to align left edges of header logo and sidenav
@cockroach-teamcity

Copy link
Copy Markdown
Member

@cockroach-teamcity

Copy link
Copy Markdown
Member

@j-low
j-low merged commit 23eb00f into master Jul 12, 2018
@madelynnblue
madelynnblue deleted the widen_site_grid branch August 20, 2018 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants