Review request: 8194871: Fix mistakes in FX API docs

[Kevin Rushforth]

Uploaded here:

http://cr.openjdk.java.net/~kcr/8194871/webrev.01/

This looks good.

+1

I'll push it tomorrow.

-- Kevin


Nir Lisker wrote:
> Attached a new webrev.
>
> On Thu, Jan 11, 2018 at 2:27 AM, Kevin Rushforth 
> <kevin.rushforth at oracle.com <mailto:kevin.rushforth at oracle.com>> wrote:
>
>     Since we are talking about the layout bounds of a node, I would
>     avoid using the term '3D scene' (or subscene) since in this case
>     it is the node that has the characteristic of 3D (geometry or
>     transforms) associated with it.
>
>     Anyway, let's go with the note at the end somewhere. Since layout
>     is a 2D concept, I think it's fine if 3D users don't see anything
>     about 3D until later.
>
>     Do you want to send a delta patch for that? Or you can send an
>     entire updated webrev. Whichever you prefer.
>
>
>     -- Kevin
>
>
>     Nir Lisker wrote:
>>     Yes, I initially had it as a note in the end saying something
>>     like this:
>>
>>          Note that for nodes in a 3D Scene (or SubScene),
>>     layoutBounds is cuboid. 
>>
>>     but thought that for someone working with 3D, seeing a 2D
>>     discussion all the way until the end will be confusing. (Also
>>     thought about putting a footnote at the end of the first
>>     sentence, but that's not a recommended style as far as I know.)
>>     My only slight concern is that "3D shapes" might hint at the
>>     Shape3D class, while any node in a 3D environment will have 3D
>>     bounds. This is also a technicality, so I wouldn't mind either
>>     way. Feel free to make a final verdict.
>>
>>     On Thu, Jan 11, 2018 at 1:17 AM, Kevin Rushforth
>>     <kevin.rushforth at oracle.com <mailto:kevin.rushforth at oracle.com>>
>>     wrote:
>>
>>         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.
>>>>
>>>>
>>
>