On Thu, 20 Jan 2022 16:08:03 GMT, Hannes Wallnöfer <[email protected]> wrote:
> This is a simple cleanup to add and/or make use of `HtmlTree` factory methods
> for `UL` and `DIV` with a single `HtmlStyle` parameter.
>
> There are no test changes (noreg-cleanup) and no changes in the generated JDK
> documentation.
Looks good to me!
The final comment on (HtmlTree.UL) says it all ... a small addition has a big
impact. Nice.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlSerialMethodWriter.java
line 74:
> 72: @Override
> 73: public Content getMethodsContentHeader(boolean isLastContent) {
> 74: return new HtmlTree(TagName.LI).setStyle(HtmlStyle.blockList);
While I agree that this is a direct simplification of the existing code, I
thought our CSS style was to put/declare the style just on the `ul` element,
and then use `ul.STYLE > li` in the stylesheet.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PackageTreeWriter.java
line 152:
> 150: HtmlTree ul = HtmlTree.UL(HtmlStyle.horizontal);
> 151:
> ul.add(getNavLinkMainTree(resources.getText("doclet.All_Packages")));
> 152: div.add(ul);
This is a general comment that is not specific to this instance.
You could also simplify the code further by using method chaining, even to the
point of (in this case) eliminating the need for the local variable.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java
line 858:
> 856: .setStyle(style);
> 857: }
> 858:
Minor addition, with a big impact: very nice!
-------------
Marked as reviewed by jjg (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7162
