RFR: JDK-8241625 use new "member-list" CSS class instead of general "block-list" for list of members

[Hannes Wallnoefer]

+1

I just realised how much of an improvement this is by understanding how entailed the existing code is. The use of the getMemberTree method with hard-coded CSS class for very different purposes and the resulting HTML were pretty confusing. The new MemberWriter interface and reduction of per-writer methods is a nice improvement as well.

Regarding naming of the new methods in MemberWriter: I think newMemberList and newMemberListItem might be slightly more fitting. But I think we have plenty of other get* methods that create new Content, so maybe we should address that in a separate changes? I’m fine with leaving it as is in the current webrev.

Hannes

> Am 28.03.2020 um 01:44 schrieb Jonathan Gibbons <jonathan.gibbons at oracle.com>:
> 
> Please review the first in a series of steps to simplify the (re)use of the CSS class "block-list".
> 
> There are two parts:
> 
> 1. The first part is simple but moderately pervasive, but conceptually easy to review. The use of `class="block-list"` is removed from all <li> elements. There is a corresponding small change in the stylesheet so that
>     ul.block-list li.block-list
> is replaced by
>     ul.block-list > li
> The use of '>' ensure that the corresponding styles only apply to immediate children of `ul.block-list`.
> 
> 
> 2. The second part is more complicated but less pervasive. Within each list of details sections (Field Details, Method Details, etc) there is a list of members. Currently, both the enclosing list and the member lists use `block-list`.  With the proposed change, the inner, nested lists are changed to use a new CSS class called `member-list`.  Part of the complication is that the same Java methods were being used to construct the <ul> and <li> elements for the two different types of list. (Obviously, different code was used to create the contents of the <li> elements.) So, the problem was to track down where the list and list items for the member lists were being created and to create and use new method calls instead of the shared method calls.
> 
> The code sequences starts off in the builder for each of the different kinds of member elements: fields, constructor, method, etc), each of which calls code on the corresponding writer class. Somewhat surprising, there was no common supertype for these writers, so I've created and used a new interface MemberWriter that is a supertype of the different types of member writer. The code in the member WriterImpl leverages a shared impl in SubWriterHolderWriter that creates the new element nodes.  There's more than a little code-smell with SubWriterHolderWriter, including a side-ways cast, that I now understand, and will look to fix soon, but not here in this changeset.
> 
> The new MemberWriter interface only has the two new methods in it for now, but I foresee the possibility of renaming and pulling up method definitions from the immediate subtypes.
> 
> The new coding pattern should be the same for each of the kinds of member.
> 
> I removed an unnecessary call in each writer that caused the builder to go through two steps instead of one.  These are methods with names like `get...Doc`
> 
> The names of new methods deliberately emphasis their conceptual role:  getMemberList and getMemberListItem. Writing this description, I'd be open to starting to use a different (but otherwise standard) convention of newMemberList, newMemberListItem.
> 
> The new member-list class uses the exact same definitions as block-list in the stylesheet.  Thus, the intent is that there should be no change in the direct visual appearance of each type declaration page.  We're just changing the new of the CSS class that is used.
> 
> There are trivial changes to HelpWriter that are necessary until the parallel review to improve HelpWriter is pushed.
> 
> There are some minor cleanups, most notably fixing @link references to BaseOptions.noComment() which were not updated when we automatically encapsulated the fields in BaseOptions.
> 
> I've added a new test that for now tests the new member-list code.  As we improve other uses of block-list, I envisage this test being updated for additional new kinds of lists.
> 
> --Jon
> 
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8241625
> Webrev: http://cr.openjdk.java.net/~jjg/8241625/webrev.00/
> API: http://cr.openjdk.java.net/~jjg/8241625/api.00/
>