Thanks for the review, Pavel.

> Am 05.10.2021 um 19:34 schrieb Pavel Rappo <[email protected]>:
> 
> We should use standard tools for standard tasks where possible. In this case 
> using a "placeholder" instead of the custom "watermark" workaround fixes the 
> problem, simplifies the code and looks better.
> 
> Update the copyright years before integrating.

Done.

> 
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Navigation.java
>  line 604:
> 
>> 602:                 .put(HtmlAttr.PLACEHOLDER, searchPlaceholder);
>> 603:         HtmlTree inputReset = HtmlTree.INPUT(reset, 
>> HtmlIds.RESET_BUTTON)
>> 604:                 .put(HtmlAttr.VALUE, reset);
> 
> Isn't it a mere coincidence that "reset" is both an input type and the value 
> of the "value" attribute: 
> 
> <input type="reset" id="reset-button" disabled value="reset">
> 
> Why do we need this "value" attribute?

I don’t think we need it, but I left it in as I’m not totally sure it is safe 
to remove and it isn’t shown anyway. By contrast the search input value caused 
some flickering during loading when the value was cleared by the search.js 
script and replaced by the placeholder.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java
>  line 534:
> 
>> 532:                 .put(HtmlAttr.TYPE, type)
>> 533:                 .setId(id)
>> 534:                 .put(HtmlAttr.DISABLED, "");
> 
> I'm curious: if such a call to `put` translates to a boolean HTML attribute, 
> what call to `put` translates to an HTML attribute with an empty value?

Good question, I think there is currently no way to create an attribute with an 
empty string as value.

> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties
>  line 226:
> 
>> 224: doclet.Implementation=Implementation(s):
>> 225: doclet.search=SEARCH:
>> 226: doclet.search_placeholder=Search
> 
> Later we should consider a better placeholder, such as an example input. We 
> already have the adjacent "SEARCH" text label and the magnifying glass icon. 
> Something tells me that the user has a pretty good idea what this field is 
> for and that they don't need yet another "Search".
> 

Yes, displaying example input may be a useful enhancement.

> -------------
> 
> Marked as reviewed by prappo (Reviewer).
> 
> PR: https://git.openjdk.java.net/jdk/pull/5825

Reply via email to