RFR: JDK-8261499: Simplify HTML for javadoc links

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8261499: Simplify HTML for javadoc links

Hannes Wallnöfer
This change gets rid of `<span>` elements used in conjunction with HTML links whose only purpose is to apply a CSS class to the link. Instead, the CSS class attribute is applied directly to the link's `<a href="...">` element.

There  are three CSS styles for links previously used with embedded or embedding `<span>` elements:

 - `type-name-link`, which was applied to a `<span>` element inside the link. This was generated by passing `true` as argument for the `strong` parameter in various get*Link methods.
 - `member-name-link` and `search-tag-link`, which were applied to a `<span>` element wrapping the link.

I could find no visual changes in the generated documentation with one exception:

[Class and interface links in the second column of "Uses" page tables][1] were previously wrapped in a `<span class="member-name-link">` element causing the whole class name (including non-linked parts such as type parameters) to be displayed with a bold font. With this change, the `type-name-link` style is applied only to the active link, leaving non-linked parts such as type parameters with normal font-weight.

[1]: https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/util/class-use/Map.html#java.util

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

Commit messages:
 - JDK-8261499: Include search-tag-link
 - JDK-8261499: Missing space
 - JDK-8261499: Simplify HTML for javadoc links

Changes: https://git.openjdk.java.net/jdk/pull/2516/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2516&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261499
  Stats: 714 lines in 40 files changed: 30 ins; 44 del; 640 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2516.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2516/head:pull/2516

PR: https://git.openjdk.java.net/jdk/pull/2516
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261499: Simplify HTML for javadoc links

Jonathan Gibbons-2
On Wed, 10 Feb 2021 18:20:02 GMT, Hannes Wallnöfer <[hidden email]> wrote:

> This change gets rid of `<span>` elements used in conjunction with HTML links whose only purpose is to apply a CSS class to the link. Instead, the CSS class attribute is applied directly to the link's `<a href="...">` element.
>
> There  are three CSS styles for links previously used with embedded or embedding `<span>` elements:
>
>  - `type-name-link`, which was applied to a `<span>` element inside the link. This was generated by passing `true` as argument for the `strong` parameter in various get*Link methods.
>  - `member-name-link` and `search-tag-link`, which were applied to a `<span>` element wrapping the link.
>
> I could find no visual changes in the generated documentation with one exception:
>
> [Class and interface links in the second column of "Uses" page tables][1] were previously wrapped in a `<span class="member-name-link">` element causing the whole class name (including non-linked parts such as type parameters) to be displayed with a bold font. With this change, the `type-name-link` style is applied only to the active link, leaving non-linked parts such as type parameters with normal font-weight.
>
> [1]: https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/util/class-use/Map.html#java.util

Generally, very nice indeed. Another good cleanup/simplification/modernization.

There was one tiny suggestion for a whitespace cleanup.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 99:

> 97:
> 98:         return writer.getDocLink(MEMBER_DEPRECATED_PREVIEW, utils.getEnclosingTypeElement(member),
> 99:                 member, content, null, false);

Depending how often you need to pass `null`, you might consider an overload that doesn't require a style.

Conversely, should all links have a style?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractExecutableMemberWriter.java line 114:

> 112:             Content tdSummary) {
> 113:         ExecutableElement ee = (ExecutableElement)member;
> 114:         Content memberLink = writer.getDocLink(context, te, ee, name(ee), HtmlStyle.memberNameLink);

nice; I like the way this review is headed ;-)

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 783:

> 781:      */
> 782:     public Content getCrossClassLink(TypeElement classElement, String refMemName,
> 783:                                     Content label, HtmlStyle style, boolean code) {

Yay!

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 875:

> 873:      * @param context the id of the context where the link will be added
> 874:      * @param typeElement the class to link to
> 875:      * @param style optional style for the link

Just asking: when do we not want a style?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/LinkFactoryImpl.java line 115:

> 113:                                 filename.fragment(classLinkInfo.where),
> 114:                                 label,
> 115:                                 classLinkInfo.style,

I guess we're lucky the code is downcasting the `LinkInfo` into `LinkInfoImpl`.

It would be a separate cleanup/rename, but this might be another place where the subtype might be better named `HtmlLinkInfo` rather than just `LinkInfoImpl` and this class might be `HtmlLinkFactory`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/Links.java line 163:

> 161:      * @param link      the details for the link
> 162:      * @param label     the content for the link
> 163:      * @param style    whether to wrap the {@code label} in a SPAN element

(very minor) white-space alignment

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/links/LinkInfo.java line 76:

> 74:      */
> 75:     public boolean excludeTypeBounds = false;
> 76:

I guess this is the counterpart on adding the style into the `LinkInfoImpl`.  Yes, I agree this is the least ugly way to handle the improvement.

test/langtools/jdk/javadoc/doclet/testHtmlTableTags/TestHtmlTableTags.java line 574:

> 572:         checkOutput("pkg1/C1.html", true,
> 573:                 """
> 574:                     <div class="col-first odd-row-color"><code><a href="../pkg2/C2.html" title="class in pkg2">C2</a></code></div>

I note that here is an example of a link without a style.

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

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2516
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8261499: Simplify HTML for javadoc links [v2]

Hannes Wallnöfer
In reply to this post by Hannes Wallnöfer
> This change gets rid of `<span>` elements used in conjunction with HTML links whose only purpose is to apply a CSS class to the link. Instead, the CSS class attribute is applied directly to the link's `<a href="...">` element.
>
> There  are three CSS styles for links previously used with embedded or embedding `<span>` elements:
>
>  - `type-name-link`, which was applied to a `<span>` element inside the link. This was generated by passing `true` as argument for the `strong` parameter in various get*Link methods.
>  - `member-name-link` and `search-tag-link`, which were applied to a `<span>` element wrapping the link.
>
> I could find no visual changes in the generated documentation with one exception:
>
> [Class and interface links in the second column of "Uses" page tables][1] were previously wrapped in a `<span class="member-name-link">` element causing the whole class name (including non-linked parts such as type parameters) to be displayed with a bold font. With this change, the `type-name-link` style is applied only to the active link, leaving non-linked parts such as type parameters with normal font-weight.
>
> [1]: https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/util/class-use/Map.html#java.util

Hannes Wallnöfer has updated the pull request incrementally with one additional commit since the last revision:

  JDK-8261499: Updates on feedback from code review
   - Updated comments in markup.Links
   - Rename LinkInfoImpl to HtmlLinkInfo and LinkFactoryImpl to HtmlLinkFactory
   - Add HtmlDocletWriter.getDocLink(context, typeElement, element, label) overload

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2516/files
  - new: https://git.openjdk.java.net/jdk/pull/2516/files/875fb7da..e9f1c29f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2516&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2516&range=00-01

  Stats: 203 lines in 24 files changed: 16 ins; 7 del; 180 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2516.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2516/head:pull/2516

PR: https://git.openjdk.java.net/jdk/pull/2516
Reply | Threaded
Open this post in threaded view
|

Integrated: JDK-8261499: Simplify HTML for javadoc links

Hannes Wallnöfer
In reply to this post by Hannes Wallnöfer
On Wed, 10 Feb 2021 18:20:02 GMT, Hannes Wallnöfer <[hidden email]> wrote:

> This change gets rid of `<span>` elements used in conjunction with HTML links whose only purpose is to apply a CSS class to the link. Instead, the CSS class attribute is applied directly to the link's `<a href="...">` element.
>
> There  are three CSS styles for links previously used with embedded or embedding `<span>` elements:
>
>  - `type-name-link`, which was applied to a `<span>` element inside the link. This was generated by passing `true` as argument for the `strong` parameter in various get*Link methods.
>  - `member-name-link` and `search-tag-link`, which were applied to a `<span>` element wrapping the link.
>
> I could find no visual changes in the generated documentation with one exception:
>
> [Class and interface links in the second column of "Uses" page tables][1] were previously wrapped in a `<span class="member-name-link">` element causing the whole class name (including non-linked parts such as type parameters) to be displayed with a bold font. With this change, the `type-name-link` style is applied only to the active link, leaving non-linked parts such as type parameters with normal font-weight.
>
> [1]: https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/util/class-use/Map.html#java.util

This pull request has now been integrated.

Changeset: da9895a0
Author:    Hannes Wallnöfer <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/da9895a0
Stats:     896 lines in 52 files changed: 44 ins; 49 del; 803 mod

8261499: Simplify HTML for javadoc links

Reviewed-by: jjg

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

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