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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Mar 25, 2021
Merged

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

merged 2 commits into from
Mar 25, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Mar 20, 2021

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 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
Copy link
Member Author

Trott commented Mar 20, 2021

@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
aduh95 reviewed Mar 20, 2021
View reviewed changes
tools/doc/html.js Outdated Show resolved Hide resolved
tools/doc/html.js Show resolved Hide resolved
@aduh95
Copy link
Contributor

aduh95 commented Mar 20, 2021 •
edited
Loading

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
aduh95 reviewed Mar 20, 2021
View reviewed changes
tools/doc/html.js Outdated Show resolved Hide resolved
@Trott
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>
@Trott
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 Trott merged commit 3700ba0 into nodejs:master Mar 25, 2021
@Trott
Copy link
Member Author

Trott commented Mar 25, 2021

d1e2184...3700ba0

@Trott Trott deleted the h1 branch March 25, 2021 18:29
ruyadorno pushed a commit that referenced this pull request Mar 29, 2021
@Trott @ruyadorno
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
@Trott @ruyadorno
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
@Trott @ruyadorno
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
@Trott @ruyadorno
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 ruyadorno mentioned this pull request Mar 30, 2021
Merged
targos pushed a commit that referenced this pull request May 1, 2021
@Trott @targos
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
@Trott @targos
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 danielleadams mentioned this pull request May 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PoojaDurgad PoojaDurgad PoojaDurgad approved these changes

@marsonya marsonya marsonya approved these changes

@aduh95 aduh95 aduh95 approved these changes

@benjamingr benjamingr benjamingr approved these changes

@jasnell jasnell jasnell approved these changes

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
@Trott @aduh95 @jasnell @benjamingr @marsonya @PoojaDurgad @nodejs-github-bot