RFR: 8253117: Replace HTML tables in javadoc summaries with CSS grid elements [v3]

[Jonathan Gibbons]

On Mon, 5 Oct 2020 14:27:52 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> This changes the output of the `html.markup.Table` class to plain `div` elements, using CSS Grid Layout to display them
>> in a tabular format.
>> I decided against renaming the Table class and related identifiers even though it does no longer emit an HTML <table>
>> element. I admit this results in a somewhat odd mismatch in a few places, but on the other hand the generated HTML
>> still represents tabular data. Also, the changes are much easier to understand and review this way. I'm open to
>> renaming things if we can find a better terminology.  I simplified the existing code in quite a few places:
>>  - Removed the setters for table tab ids and the browser tab script. The ids are now derived from the main table id which
>>    makes them unique even with multiple tables per page (provided the tables have different ids), and the browser script
>>    will always work for the used ids.
>>  
>>  - Removed the complex tab selection scheme based on bitwise operations and replaced it with one CSS class per tab. The
>>    elements making up a table row will have a CSS class for each tab they belong to. The CSS class names are derived from
>>    the table id as well.
>> 
>>  - Reduced per usage style classes for summary tables, thereby simplifying the style sheet. Instead of having a CSS class
>>    for each useage of a table (e.g. `member-summary`, `type-summary` etc) there is only one common CSS class for summary
>>    tables as well as one specifying the number of columns to use, e.g. `two-column-summary`, `three-column-summary` etc.
>> 
>> The rendering and spacing of the tables should be the same as previously. There are a few exceptions:
>> 
>>  - The style sheet has additional media queries to switch the layout of tables when the width of the browser window
>>    becomes very narrow. This happens at different thresholds for tables with two, three, or four columns. Note that these
>>    theresholds are based on heuristics, it is what I have found to work well under most circumstances.
>> 
>>  - The new grid never grow larger than the width available in the browser. When a table cell becomes too narrow to contain
>>    its content, the cell becomes scrollable. This happens very rarely and is not too disturbing IMO.
>> 
>>  - Spacing of columns is usually a bit different than previously. Grids offers very complex layout options, and the
>>    setting I came to use partitions space depending on the width of cell contents.
>> 
>> Here are the API docs for java.base rendered with these changes:
>> http://cr.openjdk.java.net/~hannesw/8253117/api.00/
>> 
>> Here are the API docs with these changes and additionally the patch for JDK-8248566 (mobile browser optimizations)
>> applied: http://cr.openjdk.java.net/~hannesw/8253117/api.00.mobile/
>
> Hannes Wallnöfer has updated the pull request with a new target base due to a merge or a rebase. The pull request now
> contains seven commits:
>  - Merge master
>  - Address issues raised in code review:
>     - remove unused styles and method
>     - rename alternating table row styles
>     - move id attribute for tabs
>     - add line breaks in test text blocks to make them easier to read and maintain
>  - Fix trailing whitespace
>  - Clean up comments and styles
>  - Restore table spacing
>  - Adapt tests to grid summaries
>  - Use CSS Grid Layout for javadoc summaries

Source (src/) looks OK;  I spot-checked some test files.
I like the improved formatting of the `<button>` elements in the test output.
I guess that was enough to help fix the test files; I was expecting to see changes in the HTMLTree code for adding
newlines, but I'm OK to see that done separately if it is ever necessary. There was one question in my review comments
about wondering why the number of columns contributes to the row color.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java line 460:

> 458:      * to filter the set of rows to be displayed
> 459:      */
> 460:     evenRowColor,

Yay!

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java line 167:

> 165:                 existingStyle == null ? style : existingStyle + " " + style);
> 166:         return this;
> 167:     }

OK for now, but I note there is no check for duplicates. If that becomes a problem, we might want to ensure that
`style` is not already present in `existingStyle`.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/script.js line 58:

> 56:         .forEach(function(elem, index) {
> 57:             elem.style.display = '';
> 58:             var isEvenRow = index % (columns * 2) < columns;

OK, this line is weird.  Why is the polarity of the row dependent on the number of columns?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/script.js line 89:

> 87:             selected.previousSibling.focus();
> 88:             e.preventDefault();
> 89:         } else if ((e.keyCode === 39 || e.keyCode === 40) && selected.nextSibling) {

Can we have constants, or at least a comment for the values 37, 38, 39, 40

test/langtools/jdk/javadoc/doclet/testAbstractMethod/TestAbstractMethod.java line 118:

> 116:         checkOutput("pkg/A.html", false,
> 117:                 """
> 118:                     <code>abstract void</code>""");

It would seem the purpose of this sub-test was to ensure the the modifier and type appears in the first column; is that
not still possible/easy?

test/langtools/jdk/javadoc/doclet/testAbstractMethod/TestAbstractMethod.java line 124:

> 122:                     Default Methods""",
> 123:                 """
> 124:                     <code>default void</code></td>""");

ditto to previous comment

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

PR: https://git.openjdk.java.net/jdk/pull/253