The changes look good to me for the most part. I only have one comment.
Node.java:
- * The rectangular bounds that should be used for layout ...
+ * The rectangular (cuboid for 3D nodes) bounds that should be used
for layout ...
While technically correct, in that the layout bounds of a TriangleMesh
or Sphere will be a 3D bounds, layout is a 2D operation, so this seems a
bit misleading. If you still feel that a comment is desired, then I
would recommend it not being in the opening sentence, but rather a note
later in the description...something like:
Note that for 3D shapes, the layout bounds is actually a rectangular
box with X, Y, and Z values, although only X and Y are used in layout
calculations.
-- Kevin
Kevin Rushforth wrote:
I just removed the trailing whitespace (using the handy
tools/scripts/checkWhiteSpace script with the '-F' option).
-- Kevin
Nir Lisker wrote:
Thanks,
modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java:209:
Trailing whitespace
That one is an empty line inside a code block, if it matters.
On Thu, Jan 11, 2018 at 12:14 AM, Kevin Rushforth
<kevin.rushfo...@oracle.com <mailto:kevin.rushfo...@oracle.com>> wrote:
> I'll review it, and sponsor the change. Since I will be pushing
it, I will need one more reviewer.
Actually, this is incorrect. As long as I list you as
contributor, jcheck is perfectly happy with just me as reviewer.
If anyone else wants to review it, too, that would be fine, but
not necessary for this type of fix.
-- Kevin
Kevin Rushforth wrote:
Thank you for providing the patch. I uploaded it to
cr.openjdk.java.net <http://cr.openjdk.java.net> for easy browsing:
http://cr.openjdk.java.net/~kcr/8194871/webrev.00/
<http://cr.openjdk.java.net/%7Ekcr/8194871/webrev.00/>
I'll review it, and sponsor the change. Since I will be pushing
it, I will need one more reviewer.
My quick sanity checking shows trailing whitespace in two files,
which would cause jcheck to fail:
$ hg jcheck
modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java:209:
Trailing whitespace
modules/javafx.graphics/src/main/java/javafx/animation/Transition.java:161:
Trailing whitespace
I can fix this before I push.
-- Kevin
Nir Lisker wrote:
Hi Kevin,
Please review the attached webrev.
I addressed a few fixes I found as I was working, so they are
not listed in the JIRA report.
About Transition#getParentTargetNode:
The current behavior of parent-child relationship is that an
animation can be added to multiple parent transitions. Each
parent transition will see that animation as its child,
however, the child will see only one of those animations as its
parent - the one to which is was added last. This asymmetry is
a recipe for trouble (and I argue should be addressed at some
point).
For this reason, the doc does not specify the "last one wins"
behavior, so that no contract is created. This means that it's
not clear which parent is going to be queried on each
(recursive) invocation.
Most of the changes could be backported to 8 and 9. In 9, the
methods getRangeShape and getUnderlineShape of TextAreaSkin are
also missing documentation.
