Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v70]
On Thu, 16 May 2024 18:17:31 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > address review feedback @jonathan-gibbons I'm extremely impressed with this work, especially given its size and the non-trivial problems you had to solve. Kudos also to @pavelrappo for relentless reviewing! - Marked as reviewed by hannesw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16388#pullrequestreview-2062926643
Re: RFR: 8298405: Implement JEP 467: Markdown Documentation Comments [v69]
On Wed, 15 May 2024 21:04:36 GMT, Jonathan Gibbons wrote:
>> Please review a patch to add support for Markdown syntax in documentation
>> comments, as described in the associated JEP.
>>
>> Notable features:
>>
>> * support for `///` documentation comments in `JavaTokenizer`
>> * new module `jdk.internal.md` -- a private copy of the `commonmark-java`
>> library
>> * updates to `DocCommentParser` to treat `///` comments as Markdown
>> * updates to the standard doclet to render Markdown comments in HTML
>
> Jonathan Gibbons has updated the pull request incrementally with one
> additional commit since the last revision:
>
> update tests for dangling doc comments, per review feedback
src/jdk.internal.md/share/classes/jdk/internal/markdown/MarkdownTransformer.java
line 276:
> 274: // The following {@code transform} methods invoke {@code
> transform} on
> 275: // any children that may contain Markdown. If the
> transformations on
> 276: // the children are all identify transformations (that is the
> result
Typo: identify -> identity
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 1465:
> 1463: private final HtmlRenderer renderer = HtmlRenderer.builder()
> 1464: .nodeRendererFactory(headingRendererFactory)
> 1465: .extensions(List.of(tablesExtn/*, headingIdsExtn*/))
Is there a reason to keep the commented argument?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1603439954
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1603716675
RFR: JDK-8329617: Update stylesheet for specs and tool documentation
Please review an update to the `jdk-default.css` stylesheet used for specifications and tool guides. The original purpose was to make use of the Dejavu web fonts provided by the API docs and to update the navigation bar to match the one in the API docs. However, the updates include some other fixes and improvements also described below. - The change to use the DejaVu web fonts consists only of the `@import` statement in line 16 as the stylesheet already used DejaVu web fonts as first choice in its `font-family` rules. - The changes to make the navigation bar match the one in the API docs are mostly located at the end of the file (beyond line 160). However, this also includes setting the `margin` property to '0' in the `body` element and adding a `margin` in the `main` and `footer` elements instead. - To set the horizontal margin for page content elements outside the `main` element which occur in some pages, a margin is set explicitly on those elements in lines 48-50. While this is a bit awkward, I think it's still better than working with negative margins in the header to offset the margin in the `body` element. - Most of the remaining changes (lines 53-110) are changes are to redefine the styles in simpler terms, such as leaving out declarations that are equal to browser defaults, and removing the units from `0`-length values. The changes are intended to preserve the layout of the pages, including the body font size which is slightly different from the one used in API docs (`10pt` vs `14px`). I can provide before/after snapshots of the rendered documentation if desired. - Commit messages: - JDK-8329617: Update stylesheet for specs and tool documentation Changes: https://git.openjdk.org/jdk/pull/18694/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18694=00 Issue: https://bugs.openjdk.org/browse/JDK-8329617 Stats: 73 lines in 1 file changed: 27 ins; 19 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/18694.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18694/head:pull/18694 PR: https://git.openjdk.org/jdk/pull/18694
Integrated: JDK-8324774: Add DejaVu web fonts
On Tue, 30 Jan 2024 16:13:42 GMT, Hannes Wallnöfer wrote: > This change adds the DejaVu web fonts that were previously maintained > externally to the open repository so they are available both in JDK API > documentation and any API documentation generated with the `javadoc` tool. > All files added in this PR are the same as the ones previously maintained > externally, with the exception of added license and name/version comments in > `dejavu.css`. > > Copying of font files to the generated documentation is done by looking for > font file names in `dejavu.css`, so font file names can be changed without > changing the code. However, the font file list is hard-coded in > `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name > and version in the legal file matches the one in the stylesheet. Of course I > also performed manual tests to make sure the font and legal files are copied > to the output tree and used correctly in browsers. > > Once #17411 is integrated, `dejavu.css` should also be added to the list of > files checked by the new "pass-through" test. This pull request has now been integrated. Changeset: d0a26503 Author:Hannes Wallnöfer URL: https://git.openjdk.org/jdk/commit/d0a265039a36292d87b249af0e8977982e5acc7b Stats: 546 lines in 38 files changed: 533 ins; 0 del; 13 mod 8324774: Add DejaVu web fonts 8327385: Add JavaDoc option to exclude web fonts from generated documentation Reviewed-by: ihse, jjg - PR: https://git.openjdk.org/jdk/pull/17633
Re: RFR: JDK-8324774: Add DejaVu web fonts [v2]
> This change adds the DejaVu web fonts that were previously maintained > externally to the open repository so they are available both in JDK API > documentation and any API documentation generated with the `javadoc` tool. > All files added in this PR are the same as the ones previously maintained > externally, with the exception of added license and name/version comments in > `dejavu.css`. > > Copying of font files to the generated documentation is done by looking for > font file names in `dejavu.css`, so font file names can be changed without > changing the code. However, the font file list is hard-coded in > `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name > and version in the legal file matches the one in the stylesheet. Of course I > also performed manual tests to make sure the font and legal files are copied > to the output tree and used correctly in browsers. > > Once #17411 is integrated, `dejavu.css` should also be added to the list of > files checked by the new "pass-through" test. Hannes Wallnöfer has updated the pull request incrementally with two additional commits since the last revision: - JDK-8327385: Add JavaDoc option to exclude web fonts from generated documentation - Merge try-with-resource statements - Changes: - all: https://git.openjdk.org/jdk/pull/17633/files - new: https://git.openjdk.org/jdk/pull/17633/files/2c6e2163..9f77de97 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17633=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17633=00-01 Stats: 188 lines in 7 files changed: 174 ins; 2 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17633.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17633/head:pull/17633 PR: https://git.openjdk.org/jdk/pull/17633
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Wed, 28 Feb 2024 17:12:04 GMT, Jonathan Gibbons wrote: >> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >> line 1601: >> >>> 1599: : eKind != ElementKind.OTHER ? 1 // module, >>> package, class, interface >>> 1600: : 0; // doc file >>> 1601: return "h" + Math.min(heading.getLevel() + offset, 6); >> >> I really like that we adapt the heading level to the current context, but I >> notice that the code kind of expects `h1` headings to be used everywhere, >> and "punishes" use of contextually correct headings by generating smaller >> headings. Maybe it would be more educational to only adjust the level if it >> needs adjusting? > > Setext headings only come in "level 1" and "level 2" flavors. > And, at the time the renderer sees the AST, the difference between ATX and > setext headings is erased. They're just "headings". > > I also think it is better to have a simple rule than an "adaptive" rule. If > you start doing a more complex rule, you have to consider the effect on > subheadings as well. I suspected it was about the limited range of Setext headings. Yesterday my proposal was intentionally vague, but thinking about this again I think we should actually do the simplest and least intrusive thing possible: // minLevel is 4 for members, 2 for page-level elements, 1 for doc files "h" + Math.max(heading.getLevel(), minLevel); This arguably generates the correct heading for most simple use cases. What it doesn't do is to translate whole hierarchies of headings, but I would argue that few people people need this and those who do should figure out the rules and use the correct ATX headings. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1507439797
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1601: > 1599: : eKind != ElementKind.OTHER ? 1 // module, > package, class, interface > 1600: : 0; // doc file > 1601: return "h" + Math.min(heading.getLevel() + offset, 6); I really like that we adapt the heading level to the current context, but I notice that the code kind of expects `h1` headings to be used everywhere, and "punishes" use of contextually correct headings by generating smaller headings. Maybe it would be more educational to only adjust the level if it needs adjusting? - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506190847
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1733: > 1731: return false; > 1732: } > 1733: The two methods below and some other methods in this class have too much indentation. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506145051
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1380: > 1378: > 1379: var useMarkdown = trees.stream().anyMatch(t -> t.getKind() == > Kind.MARKDOWN); > 1380: var markdownHandler = useMarkdown ? new > MarkdownHandler(element) : null; The `MarkdownHandler` and `HeadingNodeRenderer` classes must become aware of the current `TagletWriter.Context`. That's because headings and other block tags should only be generated if `TagletWriter.Context.isFirstSentence` is `false`. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506084275
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v38]
On Thu, 22 Feb 2024 00:17:29 GMT, Jonathan Gibbons wrote:
>> Please review a patch to add support for Markdown syntax in documentation
>> comments, as described in the associated JEP.
>>
>> Notable features:
>>
>> * support for `///` documentation comments in `JavaTokenizer`
>> * new module `jdk.internal.md` -- a private copy of the `commonmark-java`
>> library
>> * updates to `DocCommentParser` to treat `///` comments as Markdown
>> * updates to the standard doclet to render Markdown comments in HTML
>
> Jonathan Gibbons has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - update DocCommentParser and tests to improve handling of code blocks and
> code spans in Markdown documentation comments
> - fix indentation, for consistency
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 1373:
> 1371: while (indent < currIndent) {
> 1372: currIndent = indentStack.pop();
> 1373: }
This new Markdown class looks very good!
I think in this place we also need to check for indented code blocks, since we
reduced `currIndent` and may have 4 or more character indentation compared to
the new `currIndent` value.
My tentative fix is to add the following code here, but maybe there's a more
elegant solution by restructuring the method to first check for `indent <
currIndent` and then for new indented code blocks.
if (indent >= currIndent + 4 && !isParagraph(prevLineKind)) {
blockId++;
lineKind = LineKind.INDENTED_CODE_BLOCK;
return;
}
The following could be added to `TestMarkdownCodeBlocks.java` to test this case:
POST_LIST_INDENT(
"""
1. list item
second paragraph
{@code CODE}
@Anno
end""",
"""
list item
second paragraph
{@code CODE}
@Anno
end"""),
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1499375866
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v36]
On Fri, 16 Feb 2024 00:46:34 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with two > additional commits since the last revision: > > - Support for Table Of Contents in Markdown headings > - fix typo src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java line 286: > 284: lineKind = (ch == '\n' || ch == '\r') ? > LineKind.BLANK > 285: : (indent <= 3) ? peekLineKind() > 286: : lineKind != LineKind.OTHER ? > LineKind.INDENTED_CODE_BLOCK Doesn't this cause false positives for indented code blocks? In my understanding, indented lines in a list context and directly following a blockquote are not interpreted as code blocks, but as part of the list or blockquote. So my guess would be that `not OTHER` should be something like `BLANK or INDENTED_CODE_BLOCK or HEADER`, and that still leaves the problem of a [blank line in a list context](https://spec.commonmark.org/0.30/#example-108). - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1492065799
RFR: JDK-8324774: Add DejaVu web fonts
This change adds the DejaVu web fonts that were previously maintained externally to the open repository so they are available both in JDK API documentation and any API documentation generated with the `javadoc` tool. All files added in this PR are the same as the ones previously maintained externally, with the exception of added license and name/version comments in `dejavu.css`. Copying of font files to the generated documentation is done by looking for font file names in `dejavu.css`, so font file names can be changed without changing the code. However, the font file list is hard-coded in `APITest.java`. `CheckLibraryVersions.java` is updated to make sure the name and version in the legal file matches the one in the stylesheet. Of course I also performed manual tests to make sure the font and legal files are copied to the output tree and used correctly in browsers. Once #17411 is integrated, `dejavu.css` should also be added to the list of files checked by the new "pass-through" test. - Commit messages: - Add license comment - JDK-8324774: Add DejaVu web fonts Changes: https://git.openjdk.org/jdk/pull/17633/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17633=00 Issue: https://bugs.openjdk.org/browse/JDK-8324774 Stats: 373 lines in 32 files changed: 361 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17633.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17633/head:pull/17633 PR: https://git.openjdk.org/jdk/pull/17633
Integrated: JDK-8320458: Improve structural navigation in API documentation
On Mon, 11 Dec 2023 16:36:09 GMT, Hannes Wallnöfer wrote: > This is a rather big change to update the structural navigation in API > documentation generated by JavaDoc. It adds a table of contents for the > current page to module, package, and class documentation, and replaces the > old sub-navigation bar with a breadcrumb-style links in those pages. The > table of contents is displayed as sidebar on wide/desktop displays and > integrated into the collapsible menu for narrow/mobile displays. The > generated docs can be browsed here: > > https://cr.openjdk.org/~hannesw/8320458/api.00/ > > This change includes improvements in `stylesheet.css` and `script.js` that > are not strictly related to the navigation changes, but happened as a > consequence of the necessary restructuring. All these are described together > with the more on-topic changes in the list below. > > - The table of contents is composed as the respective writers generate the > pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of > new type `ListBuilder`. If the field is not `null` it is used to build the > table of contents as the page is built using either one of the > new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` > directly. > - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate > the markup for the sidebar and it is added to the page via the > `BodyContents.setSideContent` method. > - Both existing navigation bars (top and sub-navigation) get an additional > `` container with CSS class `nav-content` that uses a flex layout for > its content. This also handles vertical positioning, so the old workaround > for vertical of the language version label in `Docs.gmk` is not necessary > anymore. > - Apart from modules, packages, and classes, other pages that were converted > to obtain a table of contents are the "Constant Field Values" page and the > Help page. > - Originally, I used the `` element for the sidebar, but I learned > that this was the wrong element as it is meant for content that is not > strictly related to the main content of the page. The prevailing notion seems > to be that a table of contents is a navigation element and therefore should > use the `` element, so I used that for the TOC sidebar. The same applies > for the breadcrumbs sub-navigation, so I left the header `` element > wrapped around both top and sub-navigation. > - For the new lists in TOC and breadcrumbs I used ordered list elements > `` instead of unordered `` we use everywhere else, as that is what > should be used when list order is important... This pull request has now been integrated. Changeset: 81df265e Author:Hannes Wallnöfer URL: https://git.openjdk.org/jdk/commit/81df265e41d393cdde87729e091dd465934071fd Stats: 3104 lines in 71 files changed: 1670 ins; 873 del; 561 mod 8320458: Improve structural navigation in API documentation Reviewed-by: erikj, jjg - PR: https://git.openjdk.org/jdk/pull/17062
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v6]
> This is a rather big change to update the structural navigation in API > documentation generated by JavaDoc. It adds a table of contents for the > current page to module, package, and class documentation, and replaces the > old sub-navigation bar with a breadcrumb-style links in those pages. The > table of contents is displayed as sidebar on wide/desktop displays and > integrated into the collapsible menu for narrow/mobile displays. The > generated docs can be browsed here: > > https://cr.openjdk.org/~hannesw/8320458/api.00/ > > This change includes improvements in `stylesheet.css` and `script.js` that > are not strictly related to the navigation changes, but happened as a > consequence of the necessary restructuring. All these are described together > with the more on-topic changes in the list below. > > - The table of contents is composed as the respective writers generate the > pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of > new type `ListBuilder`. If the field is not `null` it is used to build the > table of contents as the page is built using either one of the > new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` > directly. > - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate > the markup for the sidebar and it is added to the page via the > `BodyContents.setSideContent` method. > - Both existing navigation bars (top and sub-navigation) get an additional > `` container with CSS class `nav-content` that uses a flex layout for > its content. This also handles vertical positioning, so the old workaround > for vertical of the language version label in `Docs.gmk` is not necessary > anymore. > - Apart from modules, packages, and classes, other pages that were converted > to obtain a table of contents are the "Constant Field Values" page and the > Help page. > - Originally, I used the `` element for the sidebar, but I learned > that this was the wrong element as it is meant for content that is not > strictly related to the main content of the page. The prevailing notion seems > to be that a table of contents is a navigation element and therefore should > use the `` element, so I used that for the TOC sidebar. The same applies > for the breadcrumbs sub-navigation, so I left the header `` element > wrapped around both top and sub-navigation. > - For the new lists in TOC and breadcrumbs I used ordered list elements > `` instead of unordered `` we use everywhere else, as that is what > should be used when list order is important... Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision: Initialize tableOfContents field in HtmlDoclet constructor - Changes: - all: https://git.openjdk.org/jdk/pull/17062/files - new: https://git.openjdk.org/jdk/pull/17062/files/bf8cff91..e777ca9d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17062=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=04-05 Stats: 21 lines in 7 files changed: 7 ins; 5 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/17062.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062 PR: https://git.openjdk.org/jdk/pull/17062
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v5]
> This is a rather big change to update the structural navigation in API > documentation generated by JavaDoc. It adds a table of contents for the > current page to module, package, and class documentation, and replaces the > old sub-navigation bar with a breadcrumb-style links in those pages. The > table of contents is displayed as sidebar on wide/desktop displays and > integrated into the collapsible menu for narrow/mobile displays. The > generated docs can be browsed here: > > https://cr.openjdk.org/~hannesw/8320458/api.00/ > > This change includes improvements in `stylesheet.css` and `script.js` that > are not strictly related to the navigation changes, but happened as a > consequence of the necessary restructuring. All these are described together > with the more on-topic changes in the list below. > > - The table of contents is composed as the respective writers generate the > pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of > new type `ListBuilder`. If the field is not `null` it is used to build the > table of contents as the page is built using either one of the > new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` > directly. > - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate > the markup for the sidebar and it is added to the page via the > `BodyContents.setSideContent` method. > - Both existing navigation bars (top and sub-navigation) get an additional > `` container with CSS class `nav-content` that uses a flex layout for > its content. This also handles vertical positioning, so the old workaround > for vertical of the language version label in `Docs.gmk` is not necessary > anymore. > - Apart from modules, packages, and classes, other pages that were converted > to obtain a table of contents are the "Constant Field Values" page and the > Help page. > - Originally, I used the `` element for the sidebar, but I learned > that this was the wrong element as it is meant for content that is not > strictly related to the main content of the page. The prevailing notion seems > to be that a table of contents is a navigation element and therefore should > use the `` element, so I used that for the TOC sidebar. The same applies > for the breadcrumbs sub-navigation, so I left the header `` element > wrapped around both top and sub-navigation. > - For the new lists in TOC and breadcrumbs I used ordered list elements > `` instead of unordered `` we use everywhere else, as that is what > should be used when list order is important... Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision: Address review feedback - Use record instead of array in ListBuilder - Rename TableOfContents.getSideBar to toContent - Changes: - all: https://git.openjdk.org/jdk/pull/17062/files - new: https://git.openjdk.org/jdk/pull/17062/files/70d06e59..bf8cff91 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17062=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=03-04 Stats: 14 lines in 7 files changed: 1 ins; 1 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17062.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062 PR: https://git.openjdk.org/jdk/pull/17062
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]
On Tue, 16 Jan 2024 19:47:47 GMT, Jonathan Gibbons wrote: > * Medium-size suggestion/question: > consider init-ing `HtmlDocletWriter.tableOfContents` directly in the > constructor for `HtmlDocletWriter` and not in the constructors for the > subtypes. I agree it would be nice, but not sure how to do it without proliferation of `HtmlDocletWriter` constructors and/or constructor arguments (we have 15 direct `HtmlDocletWriter` subclasses of which 5 have a table of contents, and they are using both available super-constructors). - PR Comment: https://git.openjdk.org/jdk/pull/17062#issuecomment-1898279784
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]
On Tue, 16 Jan 2024 19:46:30 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Consolidate TOC functionality into new TableOfContents class
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TableOfContents.java
> line 83:
>
>> 81: * @return a content object
>> 82: */
>> 83: protected Content getSideBar(boolean hasFilterInput) {
>
> (Just asking)
>
> In other builders for "complex" content, like `Table` and `Navigation` , we
> have methods like `toContent` or `getContent`. Should we follow that naming
> convention? While the name `getSidebar` is certainly more descriptive, is
> the returned object inherently a sidebar -- or is the fact that it is a
> sidebar just an artifact of how we currently choose to present the content?
It is actually up to the CSS how the table of contents is rendered. I'm
renaming the method to `toContent()`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1457174804
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]
On Tue, 16 Jan 2024 19:26:21 GMT, Jonathan Gibbons wrote: >> Hannes Wallnöfer has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Consolidate TOC functionality into new TableOfContents class > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriter.java > line 99: > >> 97: >> 98: pHelper = new PropertyUtils.PropertyHelper(configuration, >> typeElement); >> 99: tableOfContents = new TableOfContents(this); > > Is there a reason to do this here (and also in `ModuleWriter` and > `PackageWriter` etc) rather than in the (shared) super-constructor for > `HtmlDocletWriter` ? The reason for doing it in the concrete writer classes is that only some `HtmlDocletWriter` subclasses have a table of contents, and I don't think there's an easy way to figure out in the super-constructor which instances need a table of contents and which don't. We could have the subclasses pass it as an argument to the super-constructor I guess. - PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1457162662
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]
On Fri, 12 Jan 2024 18:52:46 GMT, Hannes Wallnöfer wrote: >> This is a rather big change to update the structural navigation in API >> documentation generated by JavaDoc. It adds a table of contents for the >> current page to module, package, and class documentation, and replaces the >> old sub-navigation bar with a breadcrumb-style links in those pages. The >> table of contents is displayed as sidebar on wide/desktop displays and >> integrated into the collapsible menu for narrow/mobile displays. The >> generated docs can be browsed here: >> >> https://cr.openjdk.org/~hannesw/8320458/api.00/ >> >> This change includes improvements in `stylesheet.css` and `script.js` that >> are not strictly related to the navigation changes, but happened as a >> consequence of the necessary restructuring. All these are described together >> with the more on-topic changes in the list below. >> >> - The table of contents is composed as the respective writers generate the >> pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of >> new type `ListBuilder`. If the field is not `null` it is used to build the >> table of contents as the page is built using either one of the >> new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` >> directly. >> - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate >> the markup for the sidebar and it is added to the page via the >> `BodyContents.setSideContent` method. >> - Both existing navigation bars (top and sub-navigation) get an additional >> `` container with CSS class `nav-content` that uses a flex layout for >> its content. This also handles vertical positioning, so the old workaround >> for vertical of the language version label in `Docs.gmk` is not necessary >> anymore. >> - Apart from modules, packages, and classes, other pages that were >> converted to obtain a table of contents are the "Constant Field Values" page >> and the Help page. >> - Originally, I used the `` element for the sidebar, but I learned >> that this was the wrong element as it is meant for content that is not >> strictly related to the main content of the page. The prevailing notion >> seems to be that a table of contents is a navigation element and therefore >> should use the `` element, so I used that for the TOC sidebar. The same >> applies for the breadcrumbs sub-navigation, so I left the header `` >> element wrapped around both top and sub-navigation. >> - For the new lists in TOC and breadcrumbs I used ordered list elements >> `` instead of unordered `` we use everywhere else, as that is what >> should be used when... > > Hannes Wallnöfer has updated the pull request incrementally with one > additional commit since the last revision: > > Consolidate TOC functionality into new TableOfContents class The last commit ([70d06e5](https://github.com/openjdk/jdk/pull/17062/commits/70d06e59711f61f732bc64529e26df5282671bfd)) moves the TOC-related methods from `HtmlDocletWriter` to a new `TableOfContents` class. This includes some not totally obvious changes: - `` headings in the main description are now added directly to the TOC/`ListBuilder` instead of collecting them separately and then adding them as a complete sublist. This has a few other consequences: - Method `ListBuilder.addNested(HtmlTree)` is no longer needed and has been removed, all sublists are added via a sequence of `push`-`add`-`pop`. - The stack in `ListBuilder` now stacks both the current list and the current list item as an `HtmlTree[]` array of length 2. This is because a nested list is only added to the parent list item after it is popped to make sure it is not empty. - The constant values page did previously not add `tabindex="0"` attributes. This has been fixed which required the respective test to be updated. - PR Comment: https://git.openjdk.org/jdk/pull/17062#issuecomment-1889842593
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v4]
> This is a rather big change to update the structural navigation in API > documentation generated by JavaDoc. It adds a table of contents for the > current page to module, package, and class documentation, and replaces the > old sub-navigation bar with a breadcrumb-style links in those pages. The > table of contents is displayed as sidebar on wide/desktop displays and > integrated into the collapsible menu for narrow/mobile displays. The > generated docs can be browsed here: > > https://cr.openjdk.org/~hannesw/8320458/api.00/ > > This change includes improvements in `stylesheet.css` and `script.js` that > are not strictly related to the navigation changes, but happened as a > consequence of the necessary restructuring. All these are described together > with the more on-topic changes in the list below. > > - The table of contents is composed as the respective writers generate the > pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of > new type `ListBuilder`. If the field is not `null` it is used to build the > table of contents as the page is built using either one of the > new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` > directly. > - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate > the markup for the sidebar and it is added to the page via the > `BodyContents.setSideContent` method. > - Both existing navigation bars (top and sub-navigation) get an additional > `` container with CSS class `nav-content` that uses a flex layout for > its content. This also handles vertical positioning, so the old workaround > for vertical of the language version label in `Docs.gmk` is not necessary > anymore. > - Apart from modules, packages, and classes, other pages that were converted > to obtain a table of contents are the "Constant Field Values" page and the > Help page. > - Originally, I used the `` element for the sidebar, but I learned > that this was the wrong element as it is meant for content that is not > strictly related to the main content of the page. The prevailing notion seems > to be that a table of contents is a navigation element and therefore should > use the `` element, so I used that for the TOC sidebar. The same applies > for the breadcrumbs sub-navigation, so I left the header `` element > wrapped around both top and sub-navigation. > - For the new lists in TOC and breadcrumbs I used ordered list elements > `` instead of unordered `` we use everywhere else, as that is what > should be used when list order is important... Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision: Consolidate TOC functionality into new TableOfContents class - Changes: - all: https://git.openjdk.org/jdk/pull/17062/files - new: https://git.openjdk.org/jdk/pull/17062/files/17fa1c0e..70d06e59 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17062=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=02-03 Stats: 289 lines in 17 files changed: 119 ins; 84 del; 86 mod Patch: https://git.openjdk.org/jdk/pull/17062.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062 PR: https://git.openjdk.org/jdk/pull/17062
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v3]
> This is a rather big change to update the structural navigation in API > documentation generated by JavaDoc. It adds a table of contents for the > current page to module, package, and class documentation, and replaces the > old sub-navigation bar with a breadcrumb-style links in those pages. The > table of contents is displayed as sidebar on wide/desktop displays and > integrated into the collapsible menu for narrow/mobile displays. The > generated docs can be browsed here: > > https://cr.openjdk.org/~hannesw/8320458/api.00/ > > This change includes improvements in `stylesheet.css` and `script.js` that > are not strictly related to the navigation changes, but happened as a > consequence of the necessary restructuring. All these are described together > with the more on-topic changes in the list below. > > - The table of contents is composed as the respective writers generate the > pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of > new type `ListBuilder`. If the field is not `null` it is used to build the > table of contents as the page is built using either one of the > new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` > directly. > - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate > the markup for the sidebar and it is added to the page via the > `BodyContents.setSideContent` method. > - Both existing navigation bars (top and sub-navigation) get an additional > `` container with CSS class `nav-content` that uses a flex layout for > its content. This also handles vertical positioning, so the old workaround > for vertical of the language version label in `Docs.gmk` is not necessary > anymore. > - Apart from modules, packages, and classes, other pages that were converted > to obtain a table of contents are the "Constant Field Values" page and the > Help page. > - Originally, I used the `` element for the sidebar, but I learned > that this was the wrong element as it is meant for content that is not > strictly related to the main content of the page. The prevailing notion seems > to be that a table of contents is a navigation element and therefore should > use the `` element, so I used that for the TOC sidebar. The same applies > for the breadcrumbs sub-navigation, so I left the header `` element > wrapped around both top and sub-navigation. > - For the new lists in TOC and breadcrumbs I used ordered list elements > `` instead of unordered `` we use everywhere else, as that is what > should be used when list order is important... Hannes Wallnöfer has updated the pull request incrementally with two additional commits since the last revision: - Merge remote-tracking branch 'origin/JDK-8320458' into JDK-8320458 - Address review feedback, update copyright headers - Changes: - all: https://git.openjdk.org/jdk/pull/17062/files - new: https://git.openjdk.org/jdk/pull/17062/files/4c6f4223..17fa1c0e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17062=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=01-02 Stats: 96 lines in 65 files changed: 4 ins; 6 del; 86 mod Patch: https://git.openjdk.org/jdk/pull/17062.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062 PR: https://git.openjdk.org/jdk/pull/17062
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v2]
On Tue, 9 Jan 2024 23:44:51 GMT, Jonathan Gibbons wrote: >> Hannes Wallnöfer has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update >> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >> >> Co-authored-by: Andrey Turbanov > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/search.js.template > line 33: > >> 31: loading: "##REPLACE:doclet.search.loading##", >> 32: searching: "##REPLACE:doclet.search.searching##", >> 33: redirecting: "##REPLACE:doclet.search.redirecting##", > > Suggestion ... > Would it make sense to "minimize" the use of templates by moving all the > template-y stuff to a single template file, that can be included by the other > JavaScript files, which would then become "normal" JavaScript `.js` files. That's something we should consider. We could use `script.js` for this as it is the first loaded script file, therefore available in all other script files. - PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1449005491
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v2]
On Wed, 10 Jan 2024 00:01:35 GMT, Jonathan Gibbons wrote: >> Hannes Wallnöfer has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update >> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >> >> Co-authored-by: Andrey Turbanov > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/stylesheet.css > line 1373: > >> 1371: margin: 4px 0; >> 1372: border-radius: 2px; >> 1373: /* transition: all 0.1s; */ > > Should this line stay or be removed? Will be removed. - PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1448994874
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v2]
On Tue, 9 Jan 2024 23:07:17 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
>>
>> Co-authored-by: Andrey Turbanov
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js.template
> line 1:
>
>> 1: /*
>
> I note that at least in part this is a rename of `script.js` (and rightly so)
> that Git has failed to detect (grrr.) Others may want to use external tools
> to compare the old and new forms. The differences are primarily a significant
> expansion of these lines in the old code:
>
>
> // Dynamically set scroll margin to accomodate for draft header
> document.addEventListener("DOMContentLoaded", function(e) {
> document.querySelectorAll(':not(input)[id]').forEach(
> function(c) {
> c.style["scroll-margin-top"] =
> Math.ceil(document.querySelector("header").offsetHeight) + "px"
> });
> });
Right. I wondered if I had done anything wrong when renaming the file, but it
seems `git` always handles renames as remove-add, and the only way to influence
rename detection is in `git diff`. I guess I could have prevented this by
renaming and adding/updating in two different commits, which I will do in the
future.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1448971042
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v2]
On Tue, 9 Jan 2024 23:12:51 GMT, Jonathan Gibbons wrote:
>> Hannes Wallnöfer has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
>>
>> Co-authored-by: Andrey Turbanov
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js.template
> line 251:
>
>> 249: setTopMargin();
>> 250: // Make sure current element is visible in breadcrumb navigation on
>> small displays
>> 251: const subnav = document.querySelector("ol.sub-nav-list");
>
> This is but one instance of many similar `querySelector` calls that leverage
> CSS class names and/or HTML ids.
>
> In the Java code, we used named constants for such names, defined in
> `HtmlStyle` and `HtmlId`, which allows us to use an IDE to track their usage.
> But that does not work across the language boundary into code like this. I
> wonder (just asking) if it is worth manually marking the Java constants in
> some way to indicate the names are also used in JavaScript code. As an
> example, this could be done with an individual compile-time arg-free
> annotation on each constant that needs to be marked, or a single annotation
> on (say) the `HtmlStyle` class containing an array of enum values for the
> relevant constants.
Interesting suggestion! What would the advantages of using annotations be
compared to just using comments, which could also contain a description of
where/how the constants are used?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1448976721
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v2]
On Wed, 10 Jan 2024 23:46:57 GMT, Jonathan Gibbons wrote: >> Hannes Wallnöfer has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update >> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >> >> Co-authored-by: Andrey Turbanov > > test/langtools/jdk/javadoc/doclet/testCopyFiles/TestCopyFiles.java line 60: > >> 58: // check top navbar >> 59: """ >> 60: Tree""", > > What is the significance of this filename change? For context-specific links in the top navigation bar we always link to the target closest to the current page; for example, The "USE" link in class documentation links to the Class Use page of the current class, and the "TREE" link in files belonging to a package links to the Package Tree page. This was previously not handled correctly for package-level doc files. It is now fixed, so the test has to be updated. - PR Review Comment: https://git.openjdk.org/jdk/pull/17062#discussion_r1448877285
Re: RFR: JDK-8320458: Improve structural navigation in API documentation [v2]
> This is a rather big change to update the structural navigation in API > documentation generated by JavaDoc. It adds a table of contents for the > current page to module, package, and class documentation, and replaces the > old sub-navigation bar with a breadcrumb-style links in those pages. The > table of contents is displayed as sidebar on wide/desktop displays and > integrated into the collapsible menu for narrow/mobile displays. The > generated docs can be browsed here: > > https://cr.openjdk.org/~hannesw/8320458/api.00/ > > This change includes improvements in `stylesheet.css` and `script.js` that > are not strictly related to the navigation changes, but happened as a > consequence of the necessary restructuring. All these are described together > with the more on-topic changes in the list below. > > - The table of contents is composed as the respective writers generate the > pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of > new type `ListBuilder`. If the field is not `null` it is used to build the > table of contents as the page is built using either one of the > new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` > directly. > - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate > the markup for the sidebar and it is added to the page via the > `BodyContents.setSideContent` method. > - Both existing navigation bars (top and sub-navigation) get an additional > `` container with CSS class `nav-content` that uses a flex layout for > its content. This also handles vertical positioning, so the old workaround > for vertical of the language version label in `Docs.gmk` is not necessary > anymore. > - Apart from modules, packages, and classes, other pages that were converted > to obtain a table of contents are the "Constant Field Values" page and the > Help page. > - Originally, I used the `` element for the sidebar, but I learned > that this was the wrong element as it is meant for content that is not > strictly related to the main content of the page. The prevailing notion seems > to be that a table of contents is a navigation element and therefore should > use the `` element, so I used that for the TOC sidebar. The same applies > for the breadcrumbs sub-navigation, so I left the header `` element > wrapped around both top and sub-navigation. > - For the new lists in TOC and breadcrumbs I used ordered list elements > `` instead of unordered `` we use everywhere else, as that is what > should be used when list order is important... Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision: Update src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java Co-authored-by: Andrey Turbanov - Changes: - all: https://git.openjdk.org/jdk/pull/17062/files - new: https://git.openjdk.org/jdk/pull/17062/files/40f6486d..4c6f4223 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17062=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17062=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17062.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17062/head:pull/17062 PR: https://git.openjdk.org/jdk/pull/17062
RFR: JDK-8320458: Improve structural navigation in API documentation
This is a rather big change to update the structural navigation in API documentation generated by JavaDoc. It adds a table of contents for the current page to module, package, and class documentation, and replaces the old sub-navigation bar with a breadcrumb-style links in those pages. The table of contents is displayed as sidebar on wide/desktop displays and integrated into the collapsible menu for narrow/mobile displays. The generated docs can be browsed here: https://cr.openjdk.org/~hannesw/8320458/api.00/ This change includes improvements in `stylesheet.css` and `script.js` that are not strictly related to the navigation changes, but happened as a consequence of the necessary restructuring. All these are described together with the more on-topic changes in the list below. - The table of contents is composed as the respective writers generate the pages. For this purpose, `HtmlDocletWriter` has a new `tocBuilder` field of new type `ListBuilder`. If the field is not `null` it is used to build the table of contents as the page is built using either one of the new`HtmlDocletWriter.addToTableOfContents` methods or the `ListBuilder` directly. - Once the TOC is built, `HtmlDocletWriter.getSideBar` is used to generate the markup for the sidebar and it is added to the page via the `BodyContents.setSideContent` method. - Both existing navigation bars (top and sub-navigation) get an additional `` container with CSS class `nav-content` that uses a flex layout for its content. This also handles vertical positioning, so the old workaround for vertical of the language version label in `Docs.gmk` is not necessary anymore. - Apart from modules, packages, and classes, other pages that were converted to obtain a table of contents are the "Constant Field Values" page and the Help page. - Originally, I used the `` element for the sidebar, but I learned that this was the wrong element as it is meant for content that is not strictly related to the main content of the page. The prevailing notion seems to be that a table of contents is a navigation element and therefore should use the `` element, so I used that for the TOC sidebar. The same applies for the breadcrumbs sub-navigation, so I left the header `` element wrapped around both top and sub-navigation. - For the new lists in TOC and breadcrumbs I used ordered list elements `` instead of unordered `` we use everywhere else, as that is what should be used when list order is important. - The `SEARCH` link that also served as label for the search input field has moved to the top navigation bar. The search input field uses "Search" as placeholder and also got a "Search in documentation" aria-label attribute. - The code to locate target anchors in relation to the header navigation has been much improved. For this purpose, new CSS custom properties `--top-nav-height`, `--sub-nav-height` and `--nav-height` have been introduced which are also updated dynamically to avoid any flickering seen in the previous version. - Also introduced were a few new CSS custom properties and rules for font sizes that line height that were previously missing. One example is package pages which previously had different line spacing depending on whether a `` tag was present or not. - The `Navigation` has been simplified a bit and now uses a single `addOverviewLink` to generate a link to the overview page, the first module page, or the first package page, depending on configuration. a new `addPageElementLink` method creates a link to the current page element. - Several tests were added to `TestNavigation` and `TestModuleNavigation` to check the navigation links in previously untested configurations. - Some functionality in `search.js.template` such as implementation of the mobile menu and creating the heading anchor links was not search related and therefore belonged into `script.js`. This required `script.js` localized properties to be injected into `script.js` which is therefore renamed to `script.js.template` Unfortunately, git did not recognize this as renaming but rather as removal of `script.js` and addition of `script.js.template`, but the file is pretty much unchanged up to line 232 with the new code starting in line 233. - The reset `x` button in the search input field and the new TOC filter field behave a bit differently than previously in that they are only visible if there is some text in the input field. - An empty string `""` in a fragment was so far interpreted as no fragment. However, the empty fragment is now used to navigate to the top of the page (this is standard behaviour and implemented in all browsers and different from clicking on a link without `#` which results in reloading the page rather than scrolling to the top). This required the changes in `DocLink` and `HtmlLinkInfo` as well as the `LinkChecker` test. - Empty and single element lists are required or hard to avoid in breadcrumb and TOC
Re: RFR: JDK-8304689: Add hidden option to disable external spec page [v2]
On Thu, 23 Mar 2023 13:37:19 GMT, Jonathan Gibbons wrote: >> Please review a change to introduce a hidden option to temporarily disable >> the "External Specifications" page while we add `@spec` tags, and until we >> have a critical mass of such tags. > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Update > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlOptions.java > > Co-authored-by: Andrey Turbanov Looks good to me! - Marked as reviewed by hannesw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13127#pullrequestreview-1357102503
