RFR: JDK-8273034: Make javadoc navigation collapsible on small displays [v2]

[Jonathan Gibbons]

On Mon, 6 Sep 2021 14:43:06 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> Please review a change to add a collapsible "hamburger" navigation bar to javadoc pages for small screens. The collapsible navbar is activated when the browser width is below 920px, which is the point where the normal navigation bar starts to wrap for some javadoc pages. 
>> 
>> The feature is implemented using CSS and JavaScript techniques employed by popular frameworks and has been successfully tested on several browsers and platforms, including various Android and iOS devices as well as IE and Edge browsers on Windows 10.
>> 
>> Sample documentation generated with this change is provided at the URL below. For testing, please review it on a mobile phone or tablet, or using the device emulation features provided with the developer tools of many desktop browsers.
>> 
>> http://cr.openjdk.java.net/~hannesw/8273034/api.00/index.html
>> 
>> One peculiarity of this change is that the sub-navigation links are rendered separately for the normal and the mobile navbar. The reason for this is that the links are structured differently (flat list vs nested lists) and reside in different containers. 
>> 
>> On the CSS side, apart from the collapsible navbar I overhauled the styles for the search input, especially the small reset "X" button. The positioning of the reset button is now simpler and more robust by using `absolute` instead of `relative` position, and doesn't require moving the search input and label to the right anymore.
>> 
>> The collapsible menu uses the `aria-controls`, `aria-expanded` attributes as described in the following wiki page:
>> 
>> https://www.w3.org/WAI/GL/wiki/Using_the_WAI-ARIA_aria-expanded_state_to_mark_expandable_and_collapsible_regions
>> 
>> Furthermore, the `aria-label` attribute is used to provide a human-readable lable for the button.
>> 
>> https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-label_attribute
>
> Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8273034: tweak button height

Only minor comments/questions

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java line 369:

> 367:                     };
> 368:                     if (nested) {
> 369:                         tree.add(HtmlTree.LI(HtmlTree.P(label))

Is the `HtmlTree.P` necessary/useful?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java line 406:

> 404:             if (!listContents.isEmpty()) {
> 405:                 if (nested) {
> 406:                     Content li = HtmlTree.LI(HtmlTree.P(contents.detailLabel));

Ditto: Is the HtmlTree.P necessary/useful?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java line 653:

> 651:         tree.add(navDiv);
> 652: 
> 653:         HtmlTree subDiv = new HtmlTree(TagName.DIV).setStyle(HtmlStyle.subNav);

Maybe we should have some factory methods for `HtmlTree.DIV`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/search.js.template line 294:

> 292: }
> 293: $(function() {
> 294:     var expanded = false;

#include standard/recurring discussion on testing JavaScript

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

> 808: }
> 809: /**
> 810:  * Tweak style for small screens.

General comment, for discussion ...
There's a growing set of absolute values in the stylesheet. At what point should we generate the .css file from a template and a separate list of definitions?

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

> 866:         margin: 5px 0;
> 867:     }
> 868:     div#navbar-sub-list {

Note to us: 

The use of standard id's in the stylesheet should be documented in the upcoming new spec document

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

Marked as reviewed by jjg (Reviewer).

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