On 9/4/2014 10:56 PM, Vivi An wrote:
Hello,
Please review fix for JDK-8033699. Changes made:
1) When tab or shift-tab key pressed, focus moved to next/previous
component outside of the radio button group
2) Using up/down or left/right arrow key to change selection inside
radio button group
Bug:https://bugs.openjdk.java.net/browse/JDK-8033699
Webrev:http://cr.openjdk.java.net/~van/8033699/webrev.00/
56 private KeyListener keyListener = null;
57 private KeyHandler handler = null;
It seems that these both fields point to the same object. Is it possible
to get rid of one of them?
152 if ( keyListener != null ) {
153 button.removeKeyListener( keyListener );
154 }
Some UIs also set the listener to null in the
uninstallUI()/uninstallListeners() methods (like BasicComboBoxUI or
BasicColorChooserUI).
Should the keyListener also be set to null to free the KeyHandler object?
369 if (!(eventSrc instanceof JRadioButton &&
370 ((JRadioButton) eventSrc).isVisible() &&
((JRadioButton) eventSrc).isEnabled()))
This check is used several times. It can be moved to a separate method.
373 // Get the button model from the source.
374 ButtonModel model = ((AbstractButton) eventSrc).getModel();
375 if (!(model instanceof DefaultButtonModel))
376 return;
377
378 // If the button model is DefaultButtonModel, and use it,
otherwise return.
379 DefaultButtonModel bm = (DefaultButtonModel) model;
380 if (bm == null)
381 return;
The second check is not necessary because (model instanceof
DefaultButtonModel) returns false for null model.
404 AbstractButton curElement = null;
The curElement variable declaration could be moved into the 'while'
block.
408 if (curElement instanceof JRadioButton &&
409 ((JRadioButton) curElement).isVisible() &&
410 ((JRadioButton) curElement).isEnabled()){
It is possible to use 'continue' here to not put the other code
inside the 'if' block.
418 else if (!srcFound){
422 } else if (srcFound && nextBtn == null){
It is not necessary to check the srcFound in the second 'if' because it
should already have true value.
444 if (newSelectedBtn != null){
445 newSelectedBtn.requestFocusInWindow();
446 newSelectedBtn.setSelected(true);
447 }
Is it possible here that newSelectedBtn == eventSrc?
522 private void jumpToNextComponent(JRadioButton btn, boolean
next){
The code that retrieves elements from a button group is also used in the
selectRadioButton()
and can be moved to a separate method.
Thanks,
Alexandr.
JPRT build succeeded, JCK tests for JRadiobutton and JCheckBox all
passed.
Thank you
Regards,
Vivi
