The fix looks good to me.
Thanks,
Alexandr.
On 9/16/2016 5:34 PM, Semyon Sadetsky wrote:
On 15.09.2016 16:11, Alexander Scherbatiy wrote:
On 15/09/16 15:03, Semyon Sadetsky wrote:
On 15.09.2016 12:49, Alexandr Scherbatiy wrote:
On 9/15/2016 11:47 AM, Semyon Sadetsky wrote:
On 14.09.2016 21:00, Alexandr Scherbatiy wrote:
On 9/14/2016 6:41 PM, Alexandr Scherbatiy wrote:
On 9/14/2016 1:48 PM, Semyon Sadetsky wrote:
Hello,
Please review fix for JDK9:
bug: https://bugs.openjdk.java.net/browse/JDK-8154043
webrev: http://cr.openjdk.java.net/~ssadetsky/8154043/webrev/
The new RadioButton group focus traversal algorithm introduced
by JDK-8033699 doesn't take into account that group of radio
buttons can be lay-outed in several lines in container. In this
case the LayoutFocusTraversalPolicy may mix radio buttons of
the same group with other components in its focus traversal
order and break the assumption used in
BasicRadioButtonUI#getFocusTransferBaseComponent() that radio
buttons are sequenced in the focus policy order.
The fix propose to treat a group of toggle buttons as a single
focus cycle entry in the LayoutFocusTraversalPolicy which order
is determined by the first toggle button in the group.
Should the test for the fix JDK-8033699
test/javax/swing/JRadioButton/8033699/bug8033699.java be updated
as well?
Is it possible to move the focus from the current radio button
to the next component which does not belong to the current radio
buttons group by pressing tab key?
For example in the test FocusCycleRootTest from the issue
description, moving the focus from the first and the second radio
button to the text field 1 and from the third and the fourth
radio button to the text field 2.
It is possible. But then the group will get focus second (or more)
time during the cycle. Seems, this is not correct because option
in the group has already been selected.
It is not clear is it correct that pressing an arrow key selects
a radio button in a group. For example each radio button selection
can trigger some action. So trying to select the latest radio
button it is necessary to move the focus from the first button to
the last by arrow keys so radio buttons in the middle will also be
unnecessary selected. Usually the space key is responsible to a
button selection.
I did not get the concern. Arrow buttons are aimed to cycle all
buttons group consequently. Tab key should move focus outside the
group. Space key doesn't change focus.
Also it looks inconsistent when the second option is skipped in
the cycle while the third is not.
This is because the tab key is pressed which can move the focus
from the group and back. The arrow keys are used to move the focus
in the group.
It preserves the tab key behavior before the fix 8033699 except
radio buttons in the group.
Group of options can be treated as one control which should be
visited only once during single cycle. Because it is more correct
from usability point of view: if user has made a selection it is
unnecessary to return to this control again.
Also what do you propose seems to contradict the 8074883 which
requires only one selected option to be focused always in the group.
I see the point that the group of buttons should be treated as one
control from the focus point of view.
I read the description of the JDK-8033699 but it is not still clear
to me is selecting a radio button by pressing an arrow key follows to
the industry standard?
In native apps and in browsers it works that way.
One more question which I have relates to a group of radio buttons
there no one of them is selected. Should tab pressing moves the focus
to the first button or to the latest one which has been selected or
just using standard focus order for components?
It is not good style to leave all radio buttons unselected in one
group. I think the first one should be selected in this case.
Also, yes, the test/javax/swing/JRadioButton/8033699/bug8033699.java
should be modified.
The updated webrev:
http://cr.openjdk.java.net/~ssadetsky/8154043/webrev.01/
--Semyon
Thanks,
Alexandr.
--Semyon
Thanks,
Alexandr.
--Semyon
Thanks,
Alexandr.
Thanks,
Alexandr.
--Semyon
