Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: rewrite font sizes, add viewport meta #15660

Closed
wants to merge 7 commits into from

Conversation

silverwind
Copy link
Contributor

@silverwind silverwind commented Sep 28, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Rewrote pretty much all font size definitions in the docs CSS. The viewport meta lets mobile browsers know the site is mobile-ready, which made most of the font size hacks obsolete.

I converted most em units to rem because cascaded font sizes can lead to unwanted surprises. I retained a 90% size on monospace fonts because they appear to be more in-line with the variable width font. I also resolved a few issues that I've noticed surrounding the version selection dropdown, and one issue of a heading layout overflow.

Screenshots at various widths:

screen shot 2017-09-28 at 19 09 58

screen shot 2017-09-28 at 19 09 33

screen shot 2017-09-28 at 19 09 13

Obsoletes: #15436

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Sep 28, 2017
@silverwind silverwind force-pushed the doc-font-sizes branch 2 times, most recently from cac3c18 to 994dee1 Compare September 28, 2017 17:36
@larsch
Copy link

larsch commented Sep 29, 2017

Looks great for me on desktop, also when resizing the window.

Monospace font is strangely small on Chrome on Android (same problem i had with my attempts).

Everything is small on Firefox on Android.

Screenshots below.

Android, Chrome, silverwind:doc-font-sizes:

screenshot_20170929-094225_halfsize

Android, Chome, current nodejs.org:

screenshot_20170929-095140

Android, FireFox, silverwind:doc-font-sizes:

screenshot_20170929-094857

Android, FireFox, current nodejs.org:

screenshot_20170929-095102

@silverwind
Copy link
Contributor Author

Try clearing your browser cache on Android, you might not be getting the updated CSS file.

@larsch
Copy link

larsch commented Sep 29, 2017

I believe I have done that now, but with same result. Server log claims to have sent full .css to mobile.

@silverwind
Copy link
Contributor Author

silverwind commented Sep 29, 2017

@larsch: it works flawlessly on my Android 7. Try deleting your build files, IIRC there's some kind of bug where they are not refreshed sometimes:

rm -rf out/doc/api && make doc-only && http-server out/doc/api

And if you're on mobile Chrome, make sure to clean your cache in the privacy settings.

@silverwind
Copy link
Contributor Author

silverwind commented Sep 29, 2017

I opted to not increase mobile font size because the default 1rem (which is usually 16px) looks fine to me. Also, I included overflow-wrap to fix all cases where long lines could break the layout horizontally.

Here's a current rendering:

screen shot 2017-09-29 at 23 52 45

cc: @nodejs/website

pre, tt, code, .pre, span.type, a.type {
font-family: Monaco, Consolas, "Lucida Console", monospace;
font-size: .9em;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No rem here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the one place where em is needed so it does not dramatically downsize code tags inside headings (which can be up to 2rem, and a .9rem code tag would look very bad in them).

@larsch
Copy link

larsch commented Sep 30, 2017

@silverwind: Deleting the build files did the trick. Now it looks good on my Android too!

@lpinca
Copy link
Member

lpinca commented Oct 1, 2017

h5 looks smaller than p.

screen shot 2017-10-01 at 17 12 21

@silverwind
Copy link
Contributor Author

@lpinca Ah, a case of bad UA defaults. I fixed it by copying the sizes that bootstrap uses:

screen shot 2017-10-02 at 23 35 07

@refack
Copy link
Contributor

refack commented Oct 2, 2017

Preview available at http://node.refack.com.s3.amazonaws.com/index.html

@Fishrock123 Fishrock123 self-assigned this Oct 3, 2017
@Fishrock123
Copy link
Contributor

will try to review soon, assigning to try and remember

@lpinca
Copy link
Member

lpinca commented Oct 3, 2017

Headings look good now. I noticed a minor issue on the sidebar that triggers my OCD, see screenshots below.

Before

screen shot 2017-10-03 at 08 02 09

Now

screen shot 2017-10-03 at 08 02 42

The Node.js text is not aligned with the list items and is a lot smaller than before. It has the same size of the items.

@silverwind
Copy link
Contributor Author

@lpinca fixed, thanks:

before

screen shot 2017-10-03 at 19 01 48

after

screen shot 2017-10-03 at 19 01 34

@lpinca
Copy link
Member

lpinca commented Oct 4, 2017

Took another look. Another thing I noticed is that the changelog history text now has the same size of the normal text. It was smaller before.

Before

screen shot 2017-10-04 at 18 47 17

Now

screen shot 2017-10-04 at 18 47 44

@refack
Copy link
Contributor

refack commented Oct 5, 2017

Updated http://node.refack.com/index.html

@silverwind
Copy link
Contributor Author

@lpinca fixed, I restored the old font size. It looked okay with 1rem to me, but I guess it makes more sense to have it smaller.

before

screen shot 2017-10-06 at 15 07 20

after

screen shot 2017-10-06 at 15 10 52

@silverwind
Copy link
Contributor Author

Did some more minor tweaks, latest version here: https://silverwind.io/api/

I'll likely land this tomorrow unless there are objections.

silverwind added a commit that referenced this pull request Oct 10, 2017
This makes the docs much more mobile-friendly by adding a viewport meta
tag which makes mobile browers properly scale fonts. Additionally the
font sizes have been cleaned up to use `rem` units where possible. Also
included are some fixes for the version dropdown.

PR-URL: #15660
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@silverwind
Copy link
Contributor Author

Thanks. Landed in 6d9654b.

@silverwind silverwind closed this Oct 10, 2017
@silverwind silverwind deleted the doc-font-sizes branch October 10, 2017 19:29
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
This makes the docs much more mobile-friendly by adding a viewport meta
tag which makes mobile browers properly scale fonts. Additionally the
font sizes have been cleaned up to use `rem` units where possible. Also
included are some fixes for the version dropdown.

PR-URL: nodejs/node#15660
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
silverwind added a commit to silverwind/node that referenced this pull request Oct 14, 2017
Fix a line-height issue introduced in nodejs#15660 where paragraphs
containing <code> blocks would have unequal line heights.

Fixes: nodejs/nodejs.org#1399
silverwind added a commit that referenced this pull request Oct 15, 2017
Fix a line-height issue introduced in #15660 where paragraphs
containing <code> blocks would have unequal line heights.

Fixes: nodejs/nodejs.org#1399
PR-URL: #16200
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this pull request Oct 18, 2017
This makes the docs much more mobile-friendly by adding a viewport meta
tag which makes mobile browers properly scale fonts. Additionally the
font sizes have been cleaned up to use `rem` units where possible. Also
included are some fixes for the version dropdown.

PR-URL: #15660
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
Fix a line-height issue introduced in #15660 where paragraphs
containing <code> blocks would have unequal line heights.

Fixes: nodejs/nodejs.org#1399
PR-URL: #16200
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@MylesBorins
Copy link
Contributor

This does not land cleanly in v6.x LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants