RFR: 8236935: Improve UX of the search control

[Pavel Rappo]

Replying to Jon's email rather than to Hannes's to acknowledge that I have had a chance to read both.

Many thanks for doing this, Hannes. This is certainly a big improvement as it addresses the issue of having no UI feedback in case of slow connections. I'd say that this change solves 80% of the original issue and the remaining 20% should be left for future improvements.

1. That said, I don't like the idea of cramming individual index files into a single file as a future improvement. Although I appreciate that it would make the solution more simple, it would be less user-friendly: the user will have to load the complete index even if they actually need only a part of it. Depending on the codebase individual index files can differ in size significantly. For example, here is the index for the upcoming JDK 15:

$ find build/macosx-x64/images/docs -iname "*-index.js" -exec du -h '{}' \;
24K	build/macosx-x64/images/docs/api/tag-search-index.js
4.0K	build/macosx-x64/images/docs/api/module-search-index.js
12K	build/macosx-x64/images/docs/api/package-search-index.js
4.3M	build/macosx-x64/images/docs/api/member-search-index.js
224K	build/macosx-x64/images/docs/api/type-search-index.js

Jon mentioned that "types" is probably the most searched category. If that is true, which I think it is, then to access those 224K of "types" index, a user will have to wait until all 4.6M have been loaded. While HTTP compression reduces absolute times required to load index, the relative differences remain significant:

 3K	tag-search-index.js
 1K	module-search-index.js
 2K	package-search-index.js
559K	member-search-index.js
35K	type-search-index.js

If the main motivation for coalescing index files was this:

> search results are updated multiple times - once for each search index file that is loaded. This is a bit annoying


then it could be approached differently.

2. If you care about empty files, maybe the doclet could inject the list of expected files into script.js? (The doclet generates JavaScript code anyway: the "*-index.js" files are JavaScript.)

3. I think it's the first time I noticed it: shouldn't the user-facing labels in script.js be localized?

var noResult = {l: "No results found"};
var loading = {l: "Loading search index..."};
var catModules = "Modules";
var catPackages = "Packages";
var catTypes = "Types";
var catMembers = "Members";
var catSearchTags = "SearchTags";

(If so, it should be addressed in a separate RFE, obviously.)




The below are ideas for future improvement.

4. Change the label to "Loading search index (results may be incomplete)..." or communicate that extra piece of information on "incompleteness" some other way.

5. Keep displaying that "Loading..." label on top (first line) of the dropdown until *the complete* index has been loaded and restore the selection every time results have been incrementally updated; currently the selection resets to the first line. That last bit might prove difficult since a position of the mouse pointer can interfere with the selection process.




I'm not fluent in JavaScript, but from what I can see the code *looks good*. The solution seems robust. Again, many thanks for doing this.

-Pavel

> On 11 Jun 2020, at 02:14, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
> 
> Hannes,
> 
> Code looks good and seems to work OK.
> 
> It would be nice to see a version without the 5 second delay, but I guess we will see soon enough.  The 5 second delay makes clear that the index files are loaded for every page. I hope that once the files have been cached, we don't get any flashing from a message popping up to say that files are being loaded.
> 
> I agree with deferring the discussion on using a single index file. The original intent was to make it faster to load types, for which the perception was that these are probably the most common search items.
> 
> -- Jon
> 
> Separate/bonus discussion: triggered by thinking of the 5 second delay ... would it work to have a "debug mode" such that the generated docs may contain an extra page which can be used to set cookies for the API presentation, which can be used to "configure" search?
> 
> On 6/10/20 2:44 PM, Hannes Wallnoefer wrote:
>> Please review a fix to display a „Loading search index…“ message in javadoc search while the search index files are loading and updating the results when loading is completed.
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8236935
>> Webrev: http://cr.openjdk.java.net/~hannesw/8236935/webrev.00/
>> 
>> This introduces an updateSearchResults function in script.js that is empty by default. When a search is performed before the index files have been fully loaded, the updateSearchResults function is replaced with a version that will update the search results when an index file is loaded and evaluated.
>> 
>> An invocation of updateSearchResults() is added at the end of each search index file. Also, as a consequence of this change all search index files are generated regardless of whether it is empty or not (provided the index option is enabled). This means a bit of added overhead for docs without modules or search tags, but the cost should be neglectible for a mostly empty script file.
>> 
>> JDK API docs generated with this patch at the URL below. These docs also feature an extra 5 second delay for loading the search index files to make it easier to test the new feature.
>> 
>> http://cr.openjdk.java.net/~hannesw/8236935/api.00/
>> 
>> One thing you might notice is that sometimes  search results are updated multiple times - once for each search index file that is loaded. This is a bit annoying, and it also reminded me that loading each index in a separate file probably adds quite a bit of network and evaluation overhead.  As an experiment, I created a version that combines all search indices in one single file. I’m not proposing that for 15 since it’s a bigger change that shouldn’t be rushed, but I’m including it here as a possible next step.
>> 
>> Single search index file (just for evaluation, not proposed for JDK 15):
>> Webrev: http://cr.openjdk.java.net/~hannesw/8236935/webrev.singleindex.00/
>> API docs: http://cr.openjdk.java.net/~hannesw/8236935/api.singleindex.00/
>> 
>> Hannes