On Mon, 6 Sep 2021 14:43:06 GMT, Hannes Wallnöfer <hann...@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
