GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: improve header styling for API docs by Fishrock123 · Pull Request #8811 · nodejs/node

Conversation

@Fishrock123

@Fishrock123 Fishrock123 commented Sep 27, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Improved the header sizing.
Previously I found it consistently hard to find which API I was looking at.

Sizes were mostly decided by using http://type-scale.com/
with the 1.250 "Major Third" scaling.

Live preview at https://fishrock123.github.io/api/

Screenshots available at: https://gist.github.com/Fishrock123/e2a4676535abed7c8ecb72b1ae081f60

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bnb reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@Fishrock123 Fishrock123 added the doc Issues and PRs related to Node.js documentation. label Sep 27, 2016
@lpinca

lpinca commented Sep 28, 2016

Copy link
Copy Markdown
Member

I actually prefer the old version, but I'm fine with this as well.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Sep 28, 2016

Copy link
Copy Markdown
Contributor Author

Note: currently some of the header sizes are smaller than the surrounding text. I could reduce the scaling if necessary but I'm not sure how this could possibly be a step backwards.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lpinca

lpinca commented Sep 28, 2016

Copy link
Copy Markdown
Member

currently some of the header sizes are smaller than the surrounding text.

h3 headers are of the same size of the paragraphs which I agree is bad but everything looks so HUGE on the screenshots especially on mobile. Maybe the screenshots are misleading or is just me, don't worry.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Sep 28, 2016

Copy link
Copy Markdown
Contributor Author

The mobile screenshots scale up because of how GitHub displays them. My phone is an iPhone 6s with a screen size of something like 4.5 inches.

Your best bet would be to try it locally.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Sep 28, 2016

Copy link
Copy Markdown
Contributor Author

(Actually, currently h4 is smaller than h5: https://nodejs.org/dist/latest-v6.x/docs/api/stream.html#stream_class_stream_writable)
lpinca reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lpinca

lpinca commented Sep 28, 2016

Copy link
Copy Markdown
Member

Tested on iPhone 5, looks good.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Sep 29, 2016

Copy link
Copy Markdown
Contributor Author

maybe @nodejs/website would prefer to review?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ghost

ghost commented Sep 29, 2016

Copy link
Copy Markdown

tested on oneplus 3 and mbp retina 2014, looks good
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Oct 3, 2016

Copy link
Copy Markdown
Contributor Author

@nodejs/collaborators anyone with some CSS knowledge, could you please review?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
silverwind
silverwind suggested changes Oct 3, 2016
View reviewed changes

@silverwind silverwind left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Few CSS comments, didn't test the changes yet.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api_assets/style.css Outdated

@silverwind silverwind Oct 3, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Unnecessary leading zeroes. Can you fix them here and everywhere in this file?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api_assets/style.css Outdated

@silverwind silverwind Oct 3, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

this rule seems superfluous (see rule above).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api_assets/style.css Outdated

@silverwind silverwind Oct 3, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Care to investigate if we can reduce this selector to tt, code? It's a bit hackish having to specify all the different ancestors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Fishrock123
Fishrock123 force-pushed the better-docs-headers branch from 9d98a62 to 9779210 Compare October 4, 2016 19:32
@Fishrock123

Fishrock123 commented Oct 4, 2016

Copy link
Copy Markdown
Contributor Author

Updated, ptal @silverwind

The one ruleset is now for all tt, code { -- didn't seem to make an difference but perhaps there's some obscure spot where it makes a difference.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Oct 6, 2016

Copy link
Copy Markdown
Contributor Author

ping again @silverwind
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
silverwind
silverwind approved these changes Oct 6, 2016
View reviewed changes
@silverwind

silverwind commented Oct 6, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

LGTM, but one more suggestion: Can you increase the font size of the TOC by a notch? The difference between TOC entries and body/headings seems a bit too much. Or reduce the sizes of those slightly. Your call.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123

Fishrock123 commented Oct 6, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@silverwind which TOC do you mean? The sidebar? The "index" page, or what comes at the top of every page?

If you mean the one on every page, did you just mean the "table of Contents" Header, or the actual entries?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@silverwind

silverwind commented Oct 6, 2016

Copy link
Copy Markdown
Contributor

I mean the links on top here:
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fishrock123 added a commit to Fishrock123/fishrock123.github.io that referenced this pull request Oct 14, 2016
@Fishrock123
check in node.js docs from pr 8811
ed0a461 
nodejs/node#8811
@Fishrock123

Fishrock123 commented Oct 14, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I've put this up at https://fishrock123.github.io/api/ so others can view what it's like to scroll etc with it
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rvagg

rvagg commented Oct 17, 2016

Copy link
Copy Markdown
Member

Now I can cruise through examples I like the increased font sizes and the clarity of the difference between header sizes makes a lot of difference too.

lgtm

@nodejs/website last chance to pop in and give an opinion here
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lpinca
lpinca approved these changes Oct 17, 2016
View reviewed changes

@lpinca lpinca left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@phillipj

phillipj commented Oct 17, 2016

Copy link
Copy Markdown
Member

LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
princejwesley
princejwesley approved these changes Oct 17, 2016
View reviewed changes

@princejwesley princejwesley left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@rvagg
rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@phillipj phillipj mentioned this pull request Oct 18, 2016
Improve doc readability nodejs/nodejs.org#919
Closed
@Fishrock123
doc: improve header styling for API docs
f1a3755 
Sizes were mostly decided by using http://type-scale.com/
with the 1.250 "Major Third" scaling.

PR-URL: nodejs#8811
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Phillip Johnsen <[email protected]>
@Fishrock123
Fishrock123 force-pushed the better-docs-headers branch from 9779210 to f1a3755 Compare October 21, 2016 21:03
@Fishrock123

Fishrock123 commented Oct 21, 2016

Copy link
Copy Markdown
Contributor Author

Thanks, landed in f1a3755
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fishrock123
Fishrock123 merged commit f1a3755 into nodejs:master Oct 21, 2016
jasnell pushed a commit that referenced this pull request Oct 24, 2016
@Fishrock123 @jasnell
doc: improve header styling for API docs
3e2bafe 
Sizes were mostly decided by using http://type-scale.com/
with the 1.250 "Major Third" scaling.

PR-URL: #8811
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Phillip Johnsen <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Oct 31, 2016
code highlight slightly misaligned #9381
Closed
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
@Fishrock123 Myles Borins
doc: improve header styling for API docs
53de0c2 
Sizes were mostly decided by using http://type-scale.com/
with the 1.250 "Major Third" scaling.

PR-URL: #8811
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Phillip Johnsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
@Fishrock123 Myles Borins
doc: improve header styling for API docs
c1e5e66 
Sizes were mostly decided by using http://type-scale.com/
with the 1.250 "Major Third" scaling.

PR-URL: #8811
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Phillip Johnsen <[email protected]>
@MylesBorins MylesBorins added land-on-v4.x labels Nov 15, 2016
This was referenced Nov 22, 2016
v6.9.2 proposal #9735
Merged
v4.7.0 proposal #9736
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes
+2 more reviewers
@silverwind silverwind silverwind approved these changes
@princejwesley princejwesley princejwesley approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@Fishrock123 @lpinca @silverwind @rvagg @phillipj @princejwesley @MylesBorins
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.