Review request: 8194871: Fix mistakes in FX API docs

Kevin Rushforth kevin.rushforth at oracle.com
Wed Jan 10 23:17:22 UTC 2018


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.rushforth at oracle.com <mailto:kevin.rushforth at 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.
>>
>>


More information about the openjfx-dev mailing list