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
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
More information about the openjfx-dev
mailing list