RFR: 8228363 Fix shifts for ContextMenu shown on TOP/LEFT side

Kevin Rushforth kevin.rushforth at oracle.com
Fri Feb 7 16:44:53 UTC 2020

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 

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

More information about the openjfx-dev mailing list