From 035324503f5e04b53d99573a664fd1367b7ccf30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Walln=C3=B6fer?= Date: Wed, 13 Mar 2024 15:13:35 +0000 Subject: [PATCH] 8325874: Improve checkbox-based interface in summary pages Reviewed-by: prappo --- .../formats/html/DeprecatedListWriter.java | 35 +++++++------------ .../formats/html/NewAPIListWriter.java | 19 ++++------ .../formats/html/PreviewListWriter.java | 24 +++++-------- .../formats/html/SummaryListWriter.java | 23 ++++++++++++ .../internal/doclets/formats/html/Table.java | 19 ++-------- .../formats/html/resources/script.js.template | 27 +++++++------- .../html/resources/standard.properties | 3 ++ .../doclet/testNewApiList/TestNewApiList.java | 20 +++++++---- .../doclet/testPreview/TestPreview.java | 7 ++-- 9 files changed, 90 insertions(+), 87 deletions(-) diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java index bc0f1e52bbd..684e198ef1b 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/DeprecatedListWriter.java @@ -32,8 +32,6 @@ import javax.lang.model.element.Element; import com.sun.source.doctree.DeprecatedTree; import jdk.javadoc.internal.doclets.formats.html.Navigation.PageMode; -import jdk.javadoc.internal.doclets.formats.html.markup.HtmlAttr; -import jdk.javadoc.internal.doclets.formats.html.markup.HtmlId; import jdk.javadoc.internal.doclets.formats.html.markup.HtmlStyle; import jdk.javadoc.internal.doclets.formats.html.markup.HtmlTree; import jdk.javadoc.internal.doclets.formats.html.markup.Text; @@ -83,10 +81,19 @@ public class DeprecatedListWriter extends SummaryListWriter 1) { Content tabs = HtmlTree.DIV(HtmlStyle.checkboxes, contents.getContent( "doclet.Deprecated_API_Checkbox_Label")); - for (int i = 0; i < releases.size(); i++) { - // Table column ids are 1-based - tabs.add(Text.of(" ")).add(getReleaseCheckbox(releases.get(i), i + 1)); + // Table column ids are 1-based + int index = 1; + for (String release : releases) { + // Empty string represents other/uncategorized releases. Since we can't make any assumptions + // about release names this is arguably the safest way to avoid naming collisions. + Content label = !release.isEmpty() + ? Text.of(release) + : contents.getContent("doclet.Deprecated_API_Checkbox_Other_Releases"); + String id = release.isEmpty() ? ID_OTHER : String.valueOf(index++); + tabs.add(Text.of(" ")).add(getCheckbox(label, id, "release-")); } + tabs.add(Text.of(" ")).add(getCheckbox( + contents.getContent("doclet.Deprecated_API_Checkbox_All_Releases"), ID_ALL, "release-")); target.add(tabs); } } @@ -97,23 +104,6 @@ public class DeprecatedListWriter extends SummaryListWriter 1) { table.setDefaultTab(getTableCaption(headingKey)) - .setAlwaysShowDefaultTab(true) .setRenderTabs(false); for (String release : releases) { Content tab = TERMINALLY_DEPRECATED_KEY.equals(headingKey) diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NewAPIListWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NewAPIListWriter.java index d035e0f1d95..a976ad21548 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NewAPIListWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/NewAPIListWriter.java @@ -79,21 +79,17 @@ public class NewAPIListWriter extends SummaryListWriter { @Override protected void addContentSelectors(Content content) { - List releases = configuration.newAPIPageBuilder.releases; + List releases = builder.releases; if (releases.size() > 1) { Content tabs = HtmlTree.DIV(HtmlStyle.checkboxes, contents.getContent("doclet.New_API_Checkbox_Label")); - for (int i = 0; i < releases.size(); i++) { - int releaseIndex = i + 1; - String release = releases.get(i); - HtmlId htmlId = HtmlId.of("release-" + releaseIndex); - tabs.add(Text.of(" ")).add(HtmlTree.LABEL(htmlId.name(), - HtmlTree.INPUT(HtmlAttr.InputType.CHECKBOX, htmlId) - .put(HtmlAttr.CHECKED, "") - .put(HtmlAttr.ONCLICK, - "toggleGlobal(this, '" + releaseIndex + "', 3)")) - .add(HtmlTree.SPAN(Text.of(release)))); + // Table column ids are 1-based + int index = 1; + for (String release : releases) { + tabs.add(Text.of(" ")).add(getCheckbox(Text.of(release), String.valueOf(index++), "release-")); } + Content label = contents.getContent("doclet.New_API_Checkbox_All_Releases"); + tabs.add(Text.of(" ")).add(getCheckbox(label, ID_ALL, "release-")); content.add(tabs); } } @@ -104,7 +100,6 @@ public class NewAPIListWriter extends SummaryListWriter { List releases = builder.releases; if (releases.size() > 1) { table.setDefaultTab(getTableCaption(headingKey)) - .setAlwaysShowDefaultTab(true) .setRenderTabs(false); for (String release : releases) { table.addTab( diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java index 0f14098a326..911df132de4 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/PreviewListWriter.java @@ -33,8 +33,7 @@ import javax.lang.model.element.Element; import com.sun.source.doctree.DocTree; import jdk.javadoc.internal.doclets.formats.html.Navigation.PageMode; -import jdk.javadoc.internal.doclets.formats.html.markup.HtmlAttr; -import jdk.javadoc.internal.doclets.formats.html.markup.HtmlId; +import jdk.javadoc.internal.doclets.formats.html.markup.ContentBuilder; import jdk.javadoc.internal.doclets.formats.html.markup.HtmlStyle; import jdk.javadoc.internal.doclets.formats.html.markup.HtmlTree; import jdk.javadoc.internal.doclets.formats.html.markup.Text; @@ -80,21 +79,17 @@ public class PreviewListWriter extends SummaryListWriter protected void addContentSelectors(Content target) { Set jeps = builder.getJEPs(); if (!jeps.isEmpty()) { - int index = 0; + int index = 1; target.add(HtmlTree.P(contents.getContent("doclet.Preview_API_Checkbox_Label"))); - Content list = HtmlTree.UL(HtmlStyle.previewFeatureList); + Content list = HtmlTree.UL(HtmlStyle.previewFeatureList).addStyle(HtmlStyle.checkboxes); for (var jep : jeps) { - index++; - HtmlId htmlId = HtmlId.of("feature-" + index); String jepUrl = resources.getText("doclet.Preview_JEP_URL", jep.number()); - list.add(HtmlTree.LI(HtmlTree.LABEL(htmlId.name(), - HtmlTree.INPUT(HtmlAttr.InputType.CHECKBOX, htmlId) - .put(HtmlAttr.CHECKED, "") - .put(HtmlAttr.ONCLICK, - "toggleGlobal(this, '" + index + "', 3)")) - .add(HtmlTree.SPAN(Text.of(jep.number() + ": ")) - .add(HtmlTree.A(jepUrl, Text.of(jep.title() + " (" + jep.status() + ")")))))); + Content label = new ContentBuilder(Text.of(jep.number() + ": ")) + .add(HtmlTree.A(jepUrl, Text.of(jep.title() + " (" + jep.status() + ")"))); + list.add(HtmlTree.LI(getCheckbox(label, String.valueOf(index++), "feature-"))); } + Content label = contents.getContent("doclet.Preview_API_Checkbox_Toggle_All"); + list.add(HtmlTree.LI(getCheckbox(label, ID_ALL, "feature-"))); target.add(list); } } @@ -113,7 +108,6 @@ public class PreviewListWriter extends SummaryListWriter protected void addTableTabs(Table table, String headingKey) { table.setGridStyle(HtmlStyle.threeColumnSummary) .setDefaultTab(getTableCaption(headingKey)) - .setAlwaysShowDefaultTab(true) .setRenderTabs(false); for (PreviewAPIListBuilder.JEP jep : builder.getJEPs()) { table.addTab(Text.EMPTY, element -> jep == builder.getJEP(element)); @@ -122,7 +116,7 @@ public class PreviewListWriter extends SummaryListWriter @Override protected Content getExtraContent(Element element) { - PreviewAPIListBuilder.JEP jep = configuration.previewAPIListBuilder.getJEP(element); + PreviewAPIListBuilder.JEP jep = builder.getJEP(element); return jep == null ? Text.EMPTY : Text.of(jep.title()); } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java index c478907270c..ffe0350678d 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java @@ -32,6 +32,7 @@ import javax.lang.model.element.ModuleElement; import javax.lang.model.element.PackageElement; import jdk.javadoc.internal.doclets.formats.html.markup.ContentBuilder; +import jdk.javadoc.internal.doclets.formats.html.markup.HtmlAttr; import jdk.javadoc.internal.doclets.formats.html.markup.HtmlId; import jdk.javadoc.internal.doclets.formats.html.markup.HtmlStyle; import jdk.javadoc.internal.doclets.formats.html.markup.Script; @@ -54,6 +55,9 @@ import jdk.javadoc.internal.doclets.toolkit.util.SummaryAPIListBuilder.SummaryEl */ public abstract class SummaryListWriter extends SubWriterHolderWriter { + final protected String ID_OTHER = "other"; + final protected String ID_ALL = "all"; + protected String getHeadingKey(SummaryElementKind kind) { return switch (kind) { case MODULE -> "doclet.Modules"; @@ -294,6 +298,25 @@ public abstract class SummaryListWriter extends return writer.getSummaryLink(e); } + /** + * Create a checkbox input element and associated label for selecting content on + * a summary page. + * + * @param label The label + * @param id the id of the selected content + * @param htmlPrefix the prefix for the HTML id + * @return a content object containing the checkbox input + */ + protected Content getCheckbox(Content label, String id, String htmlPrefix) { + String htmlId = htmlPrefix + id; + return HtmlTree.LABEL(htmlId, + HtmlTree.INPUT(HtmlAttr.InputType.CHECKBOX, HtmlId.of(htmlId)) + .put(HtmlAttr.CHECKED, "") + .put(HtmlAttr.ONCLICK, + "toggleGlobal(this, '" + id + "', 3)")) + .add(HtmlTree.SPAN(label)); + } + /** * Add an extra optional section to the content. * diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java index cd401cbad95..339cfbf6f72 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Table.java @@ -84,7 +84,6 @@ public class Table extends Content { private HtmlStyle gridStyle; private final List bodyRows; private HtmlId id; - private boolean alwaysShowDefaultTab = false; /** * A record containing the data for a table tab. @@ -147,16 +146,6 @@ public class Table extends Content { return this; } - /** - * Sets whether to display the default tab even if tabs are empty or only contain a single tab. - * @param showDefaultTab true if default tab should always be shown - * @return this object - */ - public Table setAlwaysShowDefaultTab(boolean showDefaultTab) { - this.alwaysShowDefaultTab = showDefaultTab; - return this; - } - /** * Allows to set whether tabs should be rendered for this table. Some pages use their * own controls to select table categories, in which case the tabs are omitted. @@ -385,7 +374,7 @@ public class Table extends Content { } var table = HtmlTree.DIV(tableStyle).addStyle(gridStyle); - if ((tabs == null || occurringTabs.size() == 1) && !alwaysShowDefaultTab) { + if ((tabs == null || occurringTabs.size() == 1) && renderTabs) { if (tabs == null) { main.add(caption); } else { @@ -401,15 +390,13 @@ public class Table extends Content { HtmlId defaultTabId = HtmlIds.forTab(id, 0); if (renderTabs) { tablist.add(createTab(defaultTabId, HtmlStyle.activeTableTab, true, defaultTab)); - } else { - tablist.add(getCaption(defaultTab)); - } - if (renderTabs) { for (var tab : tabs) { if (occurringTabs.contains(tab)) { tablist.add(createTab(HtmlIds.forTab(id, tab.index()), HtmlStyle.tableTab, false, tab.label())); } } + } else { + tablist.add(getCaption(defaultTab)); } if (id == null) { throw new IllegalStateException("no id set for table"); diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js.template b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js.template index e14a2ef34cb..ef09e6df90f 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js.template +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js.template @@ -97,20 +97,23 @@ function sortTable(header, columnIndex, columns) { // Toggles the visibility of a table category in all tables in a page function toggleGlobal(checkbox, selected, columns) { - var display = checkbox.checked ? '' : 'none'; - document.querySelectorAll("div.table-tabs").forEach(function(t) { - var id = t.parentElement.getAttribute("id"); - var selectedClass = id + "-tab" + selected; - // if selected is empty string it selects all uncategorized entries - var selectUncategorized = !Boolean(selected); + const display = checkbox.checked ? '' : 'none'; + const selectOther = selected === "other"; + const selectAll = selected === "all"; + if (selectAll) { + document.querySelectorAll('.checkboxes input[type="checkbox"]').forEach(c => { + c.checked = checkbox.checked; + }); + } + document.querySelectorAll("div.table-tabs").forEach(t => { + const id = t.parentElement.getAttribute("id"); + const selectedClass = id + "-tab" + (selectOther ? "" : selected); var visible = 0; - document.querySelectorAll('div.' + id) + t.parentElement.querySelectorAll('div.' + id) .forEach(function(elem) { - if (selectUncategorized) { - if (elem.className.indexOf(selectedClass) === -1) { - elem.style.display = display; - } - } else if (elem.classList.contains(selectedClass)) { + if (selectAll + || (!selectOther && elem.classList.contains(selectedClass)) + || (selectOther && elem.className.indexOf(selectedClass) < 0)) { elem.style.display = display; } if (elem.style.display === '') { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties index 23bf1f3c194..8e224cecb36 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/standard.properties @@ -116,17 +116,20 @@ doclet.tag.invalid_input=invalid input: ''{0}'' doclet.tag.invalid=invalid @{0} doclet.Deprecated_API=Deprecated API doclet.Deprecated_API_Checkbox_Label=Show API deprecated in: +doclet.Deprecated_API_Checkbox_All_Releases=all doclet.Deprecated_API_Checkbox_Other_Releases=other doclet.Deprecated_Elements=Deprecated {0} doclet.Deprecated_Elements_Release_Column_Header=Deprecated in doclet.Deprecated_In_Release=Deprecated in {0} doclet.New_API=New API doclet.New_API_Checkbox_Label=Show API added in: +doclet.New_API_Checkbox_All_Releases=all doclet.New_Elements=New {0} doclet.New_Elements_Release_Column_Header=Added in doclet.New_Label=New doclet.Preview_API=Preview API doclet.Preview_API_Checkbox_Label=Show preview API for: +doclet.Preview_API_Checkbox_Toggle_All=Toggle all doclet.Preview_JEP_URL=https://openjdk.org/jeps/{0} doclet.Preview_Label=Preview doclet.Preview_Mark=PREVIEW diff --git a/test/langtools/jdk/javadoc/doclet/testNewApiList/TestNewApiList.java b/test/langtools/jdk/javadoc/doclet/testNewApiList/TestNewApiList.java index e757ddc0eed..bf89f377d24 100644 --- a/test/langtools/jdk/javadoc/doclet/testNewApiList/TestNewApiList.java +++ b/test/langtools/jdk/javadoc/doclet/testNewApiList/TestNewApiList.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* * @test - * @bug 8263468 8269401 8268422 8287524 + * @bug 8263468 8269401 8268422 8287524 8325874 * @summary New page for "recent" new API * @library ../../lib * @modules jdk.javadoc/jdk.javadoc.internal.tool @@ -115,7 +115,9 @@ public class TestNewApiList extends JavadocTester { 3.2 + 5

Contents

  • Modules
    • @@ -604,9 +606,11 @@ public class TestNewApiList extends JavadocTester {
    Show API deprecated in:
    + 5

    Contents

    • Terminally Deprecated
      • @@ -677,7 +681,9 @@ public class TestNewApiList extends JavadocTester { 5 + 6

      Contents

      • Classes
        • diff --git a/test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java b/test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java index f9a17947f0c..247ca0f7874 100644 --- a/test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java +++ b/test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8250768 8261976 8277300 8282452 8287597 8325325 + * @bug 8250768 8261976 8277300 8282452 8287597 8325325 8325874 * @summary test generated docs for items declared using preview * @library ../../lib * @modules jdk.javadoc/jdk.javadoc.internal.tool @@ -78,10 +78,13 @@ public class TestPreview extends JavadocTester { checkOutput("preview-list.html", true, """ -