RFR: 8265433: IGV: add group, graph, and node tooltips with properties
Tobias Holenstein
tholenstein at openjdk.org
Fri Aug 5 11:26:51 UTC 2022
On Thu, 4 Aug 2022 08:52:58 GMT, Koichi Sakata <ksakata at openjdk.org> wrote:
> This pull request adds tooltips that show properties to groups and graphs in the Outline window and remaining nodes in the graph view window.
>
> # Tests
>
> There are no test classes correspond to classes that this PR changed. So I tested manually. Screenshots are as follows.
>
> ## Screenshots
> ### Group in Outline Window
> <img width="500" alt="スクリーンショット 2022-07-15 17 19 28" src="https://user-images.githubusercontent.com/60008/182804933-3df1ebdb-c0eb-4df6-ae45-aa4ccfe9f9fc.png">
>
> ### Graph in Outline Window
> <img width="500" alt="スクリーンショット 2022-07-15 17 17 29" src="https://user-images.githubusercontent.com/60008/182805052-96824754-e9f5-41c6-9e6c-eadd62238f12.png">
>
> ### Graph View Window
> <img width="500" alt="スクリーンショット 2022-07-15 17 18 31" src="https://user-images.githubusercontent.com/60008/182804997-b69af84e-9c83-4747-83d8-171162c8072c.png">
>
> # Details
>
> I added a new class, which is `PropertiesConverter`, to format text in tooltip. We can use HTML tags for formatting tooltip text. `PropertiesConverter` converts Properties object to HTML string. If there is a more appropriate class for processing it, please let me know.
>
> This PR has one side effect. When an item in Outline window is selected, the description in the properties window, which is at the bottom right of each image, shows the same content as the tooltip. Before applying this PR, only the name was shown in that. The reason is that the property window shows the return value of `getShortDescription()` of the node object by default. The value is also used for tooltip. That is explained in `org.openide.nodes.Node` class. So I overrided the method.
>
>
> /** Set the short description of the node. Fires a property change event.
> * <p>This description may be used for tool tips, etc.
> * @param s the new description
> */
> @Override
> public void setShortDescription(String s) {
>
> https://github.com/apache/netbeans/blob/master/platform/openide.nodes/src/org/openide/nodes/Node.java
>
> In the case of Graph View window, the properties window stays as it was. Because Widget class has `setToolTipText` method for tooltip.
>
> # Concerns
>
> As stated earlier, contents of the description depends on the window the selected item is in. I'm concerned about this behavior.
Nice work! @jyukutyo thanks for working on this.
Look good to me. My only suggestion is about the spacing of the text (see comment)
Note: I am not a Reviewer
src/utils/IdealGraphVisualizer/Util/src/main/java/com/sun/hotspot/igv/util/PropertiesConverter.java line 43:
> 41: .append(p.getValue())
> 42: .append("</td></tr>");
> 43: }
Suggestion:
StringBuilder sb = new StringBuilder("<html><body><table cellpadding="0" cellspacing="0">");
for (Property p : properties) {
sb.append("<tr><td>")
.append(p.getName())
.append("</td><td width="10"></td><td>")
.append(p.getValue())
.append("</td></tr>");
}
Perhaps the spacing between the text could be reduced?
*Currently:*
*Suggestion:*
-------------
PR: https://git.openjdk.org/jdk/pull/9740
More information about the hotspot-compiler-dev
mailing list