On Fri, 5 May 2023 13:42:22 GMT, Hannes Wallnöfer <[email protected]> wrote:

> Please review a change to use plain HTML links instead of buttons to provide 
> the URL to page section/heading anchors. Special care was taken to retain the 
> accessibility features added in JDK-8299718, so section links can be reached 
> and activated by keyboard and have an `aria-label` attribute. The generated 
> documentation was tested on a range of desktop and mobile browsers (Firefox, 
> Chrome on Mac OS and Linux, Safari on Mac OS and iOS, Chrome on Android) and 
> can be viewed here:
> 
> https://cr.openjdk.org/~hannesw/8305958/api.00/index.html

* a typo
* suggest a CSS class, not `style` attribute for the link
* I like the new icon

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

> 409:     $("main").click(collapse);
> 410:     $("section[id] > :header, :header[id], 
> :header:has(a[id])").each(function(idx, el) {
> 411:         // Create anchor liniks for headers with an associated id 
> attribute

typo: `linik`

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

> 415:             hdr.append(" <a href='#" + id + "' class='anchor-link' 
> aria-label='" + messages.linkToSection
> 416:                 + "'><img src='" + pathtoroot + "link.svg' alt='" + 
> messages.linkIcon +"' tabindex='0'"
> 417:                 + " style='width: 0.9em; height: 0.9em; vertical-align: 
> baseline;'></a>");

Should this be in a CSS class, not an inline `style` ?

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

PR Review: https://git.openjdk.org/jdk/pull/13831#pullrequestreview-1415207608
PR Review Comment: https://git.openjdk.org/jdk/pull/13831#discussion_r1186355923
PR Review Comment: https://git.openjdk.org/jdk/pull/13831#discussion_r1186357616

Reply via email to