On 7/18/2016 7:29 PM, Phil Race wrote:
/**
- * Sets the {@code TreeCellRenderer} to {@code tcr}. This invokes
- * {@code updateRenderer}.
+ * Called when the {@code cellRenderer} property is changed in
+ * the drawn tree component.
*
- * @param tcr the new value
+ * @param tcr the new value of the {@code cellRenderer} property
*/
protected void setCellRenderer(TreeCellRenderer tcr) {
completeEditing();
@@ -468,10 +471,10 @@
}
why did you remove the reference to updateRenderer() ? Was it wrong ?
I'm not sure that we want that level of the implementation details. Do
you think the updateRenderer() call is more important than
completeEditing() and updateSize() which called in the method as well?
Also "Called when ... is changed" occurs a few times in this webrev
.. but changed by what ?
Is it incorrect phrase in English? The source of the change may be any
user or internal code. Those methods update UI upon the change notification.
/**
- * Returns the row height.
+ * Returns the height of each row in the drawn tree component. If
the
+ * returned value is less than or equal to 0 the height for each
row is
+ * determined by the renderer.
*
- * @return the row height
+ * @return the height of each row, in pixels
*/
protected int getRowHeight() {
return (tree == null) ? -1 : tree.getRowHeight();
}
Seems like it also returns -1 if there is no tree .. I don't see how the
spec. words account for that.
It seems this check is redundant and it was added to avoid questions
about NPE. The tree field is null only if UI is not installed yet or is
uninstalled already. So, getRowHeight() method does not need to be
called if tree == null, but if called the value it returns doesn't have
any sense.
/**
- * Returns {@code true} if the tree root is visible.
+ * Returns whether the root node of the drawn tree component is
displayable.
*
- * @return {@code true} if the tree root is visible
+ * @return {@code true} if the root node of the tree is displayed
*/
protected boolean isRootVisible() {
return (tree != null) ? tree.isRootVisible() : false;
}
The API says "visible". Why change to "displyable", which usually is
reserved
to mean whether something that has a native peer ?
Maybe some clarification of what "visible" means is in order.
O.k. I would use "displayed" because it simply proxies
tree.isRootVisible() which has "displayed" in its specs.
Returns whether the root node of the drawn tree component *should be
displayed*.
?
--Semyon
-phil.
On 06/21/2016 04:55 AM, Alexandr Scherbatiy wrote:
On 6/14/2016 5:37 PM, Semyon Sadetsky wrote:
Hello,
Please review fix for JDK9:
bug: https://bugs.openjdk.java.net/browse/JDK-7020860
webrev: http://cr.openjdk.java.net/~ssadetsky/7020860/webrev.00/
The methods mentioned in JIRA are not getters/setters according to
JavaBeans specs, nevertheless some of them got wrong javadocs. The
proposed fix improves the situation.
I am not sure about the phrase "that is rendering each cell" in
474 * Returns the current instance of the {@link
TreeCellRenderer} that is
475 * rendering each cell.
Otherwise, the fix looks good to me.
Thanks,
Alexandr.
--Semyon
