I presume this is a "pre" review before sending out an actual review
(which the Skara tooling will do automatically when you generate a pull
request)...
My concern with this approach is that it leaks what should be an
implementation detail into a visible property of the control (and
really, using the properties map in this manner is a bit of a
workaround). An alternative would be to use package scope methods and a
Helper class, which is done in many other places. Worth noting is that
if this is information that a custom skin would also need, then some
sort of way to make this visible to the Skin will eventually be needed.
-- Kevin
On 2/7/2020 8:33 AM, Abhinay Agarwal wrote:
The issue is caused because of a fix which was pushed as a part of
https://bugs.openjdk.java.net/browse/JDK-8159044
The root cause of the issue is that ContextMenuSkin#performPopupShifts doesn't
consider the cases where shiftX / shiftY can be negative, just as in case of
the example provided by Robert in 8228363. Moreover, these shifts are only
required for Side.TOP and Side.LEFT.
My fix tries to find a solution to this problem using properties map as side,
dx, dy passed to ContextMenu#show(Node anchor, Side side, double dx, double dy)
are not exposed by the control and cannot be accessed by Skin. I am not sure if
it is the best way to do it.
Changes:
https://github.com/abhinayagarwal/jfx/commit/148b1ba2b0d3c6bc748df24b1b635e964a7b8001
Any feedback on the changes are appreciated.
Best regards,
Abhinay
