RFR: JDK-8241895 use new "details-list" CSS class instead of general "block-list" for list of details sections
Hannes Wallnoefer
HANNES.WALLNOEFER at ORACLE.COM
Fri Apr 3 19:39:54 UTC 2020
Hi Jon,
The changes look good to me. As I said in my previous review, the separation of method and CSS class use is a big improvement.
The html.*Writer classes still make be a bit uneasy. Browsing through the classes I found a number of unused methods, but that is not related to your changeset. I’ll file a new JBS issue for this.
Hannes
> Am 01.04.2020 um 02:27 schrieb Jonathan Gibbons <jonathan.gibbons at oracle.com>:
>
> Please review the next in the series to clean up the use of "block-list" CSS class.
>
> The previous review (JDK-8241625) handled the nested lists for the details for members of a given kind. This review is for the enclosing list ... the list of nested lists for members of a given kind. The CSS class is changed from "block-list" to "details-list"
>
> The mechanism and code flow is much the same as in the previous review. Starting with the builder class for each kind of member, a new method is used to create a new "details list". That is passed down into the writer and writer-impl, where eventually a call of `getMemberTree` is replaced by a call to another new method to create a new "details list item". I've taken the opportunity to restructure the code a bit when creating the details list item, but the content of the details list item should be exactly the same as before.
>
> Notes:
>
> I don't like the fact that the call to create and add a "details list item" is so far removed from the call to create a "details list". This makes me want to have some of these builder methods return what they build instead of add it into a container. Maybe soon...
>
> In WriterFactoryImpl, a cast has been changed from (SubWriterHolderWriter) to an appropriate WriterImpl class. The old cast was a "sideways cast", which was somewhat weird. The new cast is a cast down to the impl subtype, but is then widened to SubWriterHolderWriter inside the constructor that is called. The use of SubWriterHolderWriter is still a bit weird, but is now a bit less weird. The IDE seems to be happier as well, and better able to locate impls of some of the interface methods.
>
> In the Builder classes, I replaced a "skip if null" check from various methods with a defensive Objects.requireNonNull call in the constructor.
>
> The names of the new CSS classes can be thought of as placeholders. The point is that they are new unique names, that can be easily changed if and when we decide to open up the naming bikeshed.
>
> I don't know if the benefits outweigh the "complexity", but one possibility is for these lists (member list, detail list, etc) to use an abstract subtype of Content such that adding items to the list would automatically wrap them in <li> elements. That would cut down on some of the new methods being added, since they would become instance methods on this new abstract type of content.
>
> Coming soon:
>
> Summary lists?
> Tables?
>
> -- Jon
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8241895
> Webrev: http://cr.openjdk.java.net/~jjg/8241895/webrev.00/
> API: http://cr.openjdk.java.net/~jjg/8241895/api.00/
>
More information about the javadoc-dev
mailing list