On Thu, 19 Jan 2023 12:54:55 GMT, Karthik P K <k...@openjdk.org> wrote:
> No check was present in `RadioMenuItem` accelerator to see if it is in a
> `ToggleGroup` or not.
>
> Made changes to check if `RadioMenuItem` is part of `ToggleGroup` or not. If
> it is part of a `ToggleGroup`, then it can not be cleared using accelerator.
>
> Added test to validate the change.
Tested with the MonkeyTester app.
I'd like to have a discussion on a new ToggleGroup's property (i'll send an
email to the mailing list).
This looks good, given the decision to not to allow deselection by an
accelerator.
But I wonder if the right approach is to have a property in the ToggleGroup
which determines whether to allow all items to be deselected. If set, the
radio menu items should be allowed to be deselected by both keyboard and mouse;
if not set, the first item added to a group should be automatically selected,
and radio menu items would not get deselected by the mouse or keyboard.
I think it might qualify as a separate enhancement (and a welcome one, since it
saves some boilerplate code).
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ControlAcceleratorSupport.java
line 178:
> 176: if (!menuitem.isDisable()) {
> 177: if (menuitem instanceof RadioMenuItem) {
> 178:
> if(((RadioMenuItem)menuitem).getToggleGroup() == null){
minor: this group insists on adding spaces after "if" and before "{"
-------------
Marked as reviewed by angorya (Committer).
PR: https://git.openjdk.org/jfx/pull/1002
