RFR: JDK-8287597: List all preview features on the javadoc PREVIEW page

[Jonathan Gibbons]

On Thu, 30 Jun 2022 15:14:40 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

> Please review an enhancement to the preview page to add a list of preview features and allow users to explore the preview APIs by feature. 
> 
> While the changes for the enhancement itself are not overly complex, the work entailed a moderate cleanup of other summary API lists (Deprecated and New API pages) as well as a few unrelated summary pages that happened to share some of the CSS classes and HTML structure.
> 
> The change itself starts with a new internal `jdk.internal.javac.PreviewFeature.JEP` annotation interface to add JEP information to the nearby `jdk.internal.javac.PreviewFeature.Feature` enum. This JEP information is retrieved by new methods in the javadoc `Utils` class and then fetched by `PreviewAPIListBuilder` which passes it on to `PreviewListWriter`. 
> 
> The superclass of `PreviewListWriter` and other API summary writer classes, `SummaryListWriter`, gets a new `addContentSelectors(Content)` method to add the checkboxes to show/hide various parts of the page content. The class is also now abstract as this and other methods do not have useful implementations in the base class. 
> 
> Another change to `SummaryListWriter` is that `pageMode`, `description`, `headContent` and `titleKey` are no longer fields in the class but rather sent to the `generateSummaryListFile` method as parameters since they are mostly just used locally in that method. On the other hand, the associated Builder is used in many places and previously had to be retrieved from the configuration object or passed around as parameter, so that is now set as a final field in `SummaryListWriter`. 
> 
> Finally, a few words about changes in CSS and HTML: The "Contents" list containing various kinds of elements in the API list was previously contained in the `<div class="header">` element that also contains the page header, and the stylesheet used the `.header ul`  selector to style the list. Now that the checkboxes separate the page header from the contents list it felt wrong to put all this in the header div (most other pages only use the div for the header itself). I also didn't like the indirect CSS selector too much, `.header ul` is not very indicative of what it applies to.
> 
> I therefore decided to create a new dedicated CSS class for the contents list called `contents-list`, and move the `<ul>` element out of the header div. Apart from the summary API pages, this change also affects the "Constant Values"  page as well as the top and package level "Tree" (class hierarchy) pages. I made sure the contents list in these pages look the same as before.
> 
> The list items in the contents list now each have an `id` attribute to allow hiding the list item if no content for the element kind is currently selected/visible. The value of the `id` attribute uses the format `contents-<element-kind>`.
> 
> There is one additional CSS class called `preview-feature-list` used for the list of preview feature JEPs. 
> 
> Demo output for the new preview page as well as other pages is available here:
> http://cr.openjdk.java.net/~hannesw/8287597/api.03/preview-list.html

Mostly excellent.

There are some minor comments inline.

There are more "English phrases" (recognized as "strings with words separated by spaces") than I expected, at least some of which will end up in generated docs. But that being said, I guess this is a JDK-only feature, and we don't translate the source going into our API docs.

src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java line 91:

> 89:         /** JEP number */
> 90:         int number() default 0;
> 91:         /** JEP title */

suggest noting the format (i.e. plain text, not HTML, not a resource key)

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java line 164:

> 162:     protected void addIndexLink(HtmlId id, String headingKey, Content content) {
> 163:         var li = HtmlTree.LI(links.createLink(id,
> 164:                 contents.getContent(headingKey))).setId(HtmlId.of("contents-" + id.name()));

Suggest adding a comment that `"contents-" + id` appears in the JavaScript code as well. You presumably can't change one without the other (and can't use a common function across the language barrier.)

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties line 125:

> 123: doclet.Preview_API=Preview API
> 124: doclet.Preview_API_Checkbox_Label=Show preview API for:
> 125: doclet.Preview_JEP_URL=https://openjdk.java.net/jeps/{0}

Use `openjdk.org`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/stylesheet.css line 243:

> 241: }
> 242: ul.contents-list li {
> 243:     font-size: 13px;

Do these values have to be normalized into `.em` units, to align with new themes?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 1346:

> 1344:      * @return the map of PreviewFeature.JEP annotation element values, or an empty map
> 1345:      */
> 1346:     public Map<? extends ExecutableElement, ? extends AnnotationValue> getJepInfo(String feature) {

This method should borderline be in `Workarounds.java` because you are accessing javac internal features, even if only reflectively.

-------------

PR: https://git.openjdk.org/jdk/pull/9336