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,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.

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

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(':'));
}

tools/doc/html.js Outdated Show resolved Hide resolved
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>
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
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
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
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
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
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
targos pushed a commit that referenced this pull request May 1, 2021
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
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
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. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants