FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc,tools: use only one level 1 header per page by Trott · Pull Request #37839 · nodejs/node · GitHub

/ node Public

doc,tools: use only one level 1 header per page - #37839

Merged
Trott merged 2 commits into
nodejs:masterfrom
Trott:h1
Mar 25, 2021
ConversationCommits (2)ChecksFiles changed
Merged

doc,tools: use only one level 1 header per page#37839
Trott merged 2 commits into
nodejs:masterfrom
Trott:h1

Conversation

Trott commented Mar 20, 2021

Copy link
Copy Markdown
Member

First commit:

doc: reduce header nesting in async_hooks.md

Maximum header level reduced to 5.

Second commit:

doc,tools: use only one level 1 header per page

Increment the header levels from markdown files when producing HTML
documents. This is both better semantically (as the two h1 headers in
current docs are not actually equivalent level semantically--the second
belongs below/inside the first) and better for accessibility. (It is
valid HTML to have multiple h1 headers in a document, but it can be bad
for screen reader experience.)

nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Mar 20, 2021

Trott commented Mar 20, 2021

Copy link
Copy Markdown
Member Author

@aduh95 This ever-so-slightly changes the logic for the rendering optimization you added in #37301. (Sections now wrap <h3> instead of <h2> since I'm incrementing all the headers.) I don't think it has a negative affect, but PTAL.

aduh95 reviewed Mar 20, 2021
View reviewed changes
Comment thread tools/doc/html.js Outdated
Comment thread tools/doc/html.js Outdated

aduh95 commented Mar 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This currently fails with this error:

Error: Cannot parse header tag with attributes
    at processContent (…/tools/doc/html.js:77:11)
    at Object.toHTML (…/tools/doc/html.js:105:46)
    at main (…/tools/doc/generate.js:97:29)
make: *** [out/doc/api/deprecations.html] Error 1

I think this is because of

node/tools/doc/html.js

Lines 402 to 409 in eaadc4b

const isDeprecationHeading =
DEPRECATION_HEADING_PATTERN.test(headingText);
if (isDeprecationHeading) {
if (!node.data) node.data = {};
if (!node.data.hProperties) node.data.hProperties = {};
node.data.hProperties.id =
headingText.substring(0, headingText.indexOf(':'));
}

aduh95 reviewed Mar 20, 2021
View reviewed changes
Comment thread tools/doc/html.js Outdated
Trott added 2 commits March 25, 2021 11:23
doc: reduce header nesting in async_hooks.md
8262f7a 
Maximum header level reduced to 5.

PR-URL: nodejs#37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
doc,tools: use only one level 1 header per page
3700ba0 
Increment the header levels from markdown files when producing HTML
documents. This is both better semantically (as the two h1 headers in
current docs are not actually equivalent level semantically--the second
belongs below/inside the first) and better for accessibility. (It is
valid HTML to have multiple h1 headers in a document, but it can be bad
for screen reader experience.)

PR-URL: nodejs#37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Trott merged commit 3700ba0 into nodejs:master Mar 25, 2021

Trott commented Mar 25, 2021

Copy link
Copy Markdown
Member Author

d1e2184...3700ba0

Trott deleted the h1 branch March 25, 2021 18:29
ruyadorno pushed a commit that referenced this pull request Mar 29, 2021
doc: reduce header nesting in async_hooks.md
d30c263 
Maximum header level reduced to 5.

PR-URL: #37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno pushed a commit that referenced this pull request Mar 29, 2021
doc,tools: use only one level 1 header per page
e17b96b 
Increment the header levels from markdown files when producing HTML
documents. This is both better semantically (as the two h1 headers in
current docs are not actually equivalent level semantically--the second
belongs below/inside the first) and better for accessibility. (It is
valid HTML to have multiple h1 headers in a document, but it can be bad
for screen reader experience.)

PR-URL: #37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno pushed a commit that referenced this pull request Mar 30, 2021
doc: reduce header nesting in async_hooks.md
e84252b 
Maximum header level reduced to 5.

PR-URL: #37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno pushed a commit that referenced this pull request Mar 30, 2021
doc,tools: use only one level 1 header per page
3925458 
Increment the header levels from markdown files when producing HTML
documents. This is both better semantically (as the two h1 headers in
current docs are not actually equivalent level semantically--the second
belongs below/inside the first) and better for accessibility. (It is
valid HTML to have multiple h1 headers in a document, but it can be bad
for screen reader experience.)

PR-URL: #37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
ruyadorno mentioned this pull request Mar 30, 2021
Merged
targos pushed a commit that referenced this pull request May 1, 2021
doc: reduce header nesting in async_hooks.md
b51651c 
Maximum header level reduced to 5.

PR-URL: #37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request May 1, 2021
doc,tools: use only one level 1 header per page
eb6ea85 
Increment the header levels from markdown files when producing HTML
documents. This is both better semantically (as the two h1 headers in
current docs are not actually equivalent level semantically--the second
belongs below/inside the first) and better for accessibility. (It is
valid HTML to have multiple h1 headers in a document, but it can be bad
for screen reader experience.)

PR-URL: #37839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams mentioned this pull request May 3, 2021
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
benjamingr benjamingr approved these changes
aduh95 aduh95 approved these changes
marsonya marsonya approved these changes
+1 more reviewer
PoojaDurgad PoojaDurgad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL