changeset 1303:4b5bf956320b

RT-22386: ComboBox : cannot choose the same item multiple times
author jgiles
date Thu, 14 Jun 2012 11:05:06 +1200
parents e6d1c213886d
children ebbc5b495c06
files javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java javafx-ui-controls/test/javafx/scene/control/ComboBoxTest.java
diffstat 2 files changed, 65 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Wed Jun 13 16:18:07 2012 +1200
+++ b/javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Thu Jun 14 11:05:06 2012 +1200
@@ -156,12 +156,7 @@
         });
         
         // Fix for RT-19431 (also tested via ComboBoxListViewSkinTest)
-        updateValue(comboBox.getValue());
-        comboBox.valueProperty().addListener(new ChangeListener<T>() {
-            @Override public void changed(ObservableValue<? extends T> ov, T oldValue, T newValue) {
-                updateValue(newValue);
-            }
-        });
+        updateValue();
         
         registerChangeListener(comboBox.itemsProperty(), "ITEMS");
         registerChangeListener(comboBox.promptTextProperty(), "PROMPT_TEXT");
@@ -170,6 +165,7 @@
         registerChangeListener(comboBox.converterProperty(), "CONVERTER");
         registerChangeListener(comboBox.editorProperty(), "EDITOR");
         registerChangeListener(comboBox.buttonCellProperty(), "BUTTON_CELL");
+        registerChangeListener(comboBox.valueProperty(), "VALUE");
     }
     
     
@@ -209,6 +205,8 @@
             getEditableInputNode();
         } else if ("BUTTON_CELL".equals(p)) {
             updateButtonCell();
+        } else if ("VALUE".equals(p)) {
+            updateValue();
         }
     }
     
@@ -282,27 +280,41 @@
      *                                                                         *
      **************************************************************************/    
     
-    private void updateValue(T newValue) {
+    private void updateValue() {
+        T newValue = comboBox.getValue();
+        
+        SelectionModel sm = listView.getSelectionModel();
+        
         if (newValue == null) {
-            listView.getSelectionModel().clearSelection();
+            sm.clearSelection();
         } else {
-            int index = comboBox.getSelectionModel().getSelectedIndex();
-            if (index >= 0 && index < comboBox.getItems().size()) {
-                T itemsObj = comboBox.getItems().get(index);
-                if (itemsObj != null && itemsObj.equals(newValue)) {
-                    listView.getSelectionModel().select(index);
+            // RT-22386: We need to test to see if the value is in the comboBox
+            // items list. If it isn't, then we should clear the listview 
+            // selection
+            int indexOfNewValue = getIndexOfComboBoxValueInItemsList();
+            if (indexOfNewValue == -1) {
+                listSelectionLock = true;
+                sm.clearSelection();
+                listSelectionLock = false;
+            } else {
+                int index = comboBox.getSelectionModel().getSelectedIndex();
+                if (index >= 0 && index < comboBox.getItems().size()) {
+                    T itemsObj = comboBox.getItems().get(index);
+                    if (itemsObj != null && itemsObj.equals(newValue)) {
+                        sm.select(index);
+                    } else {
+                        sm.select(newValue);
+                    }
                 } else {
-                    listView.getSelectionModel().select(newValue);
-                }
-            } else {
-                // just select the first instance of newValue in the list
-                int listViewIndex = listView.getItems().indexOf(newValue);
-                if (listViewIndex == -1) {
-                    // RT-21336 Show the ComboBox value even though it doesn't
-                    // exist in the ComboBox items list (part one of fix)
-                    updateDisplayNode();
-                } else {
-                    listView.getSelectionModel().select(listViewIndex);
+                    // just select the first instance of newValue in the list
+                    int listViewIndex = listView.getItems().indexOf(newValue);
+                    if (listViewIndex == -1) {
+                        // RT-21336 Show the ComboBox value even though it doesn't
+                        // exist in the ComboBox items list (part one of fix)
+                        updateDisplayNode();
+                    } else {
+                        sm.select(listViewIndex);
+                    }
                 }
             }
         }
@@ -349,7 +361,7 @@
                 textField.setText(stringValue);
             }
         } else {
-            int index = getSelectedIndex();
+            int index = getIndexOfComboBoxValueInItemsList();
             if (index > -1) {
                 buttonCell.updateListView(listView);
                 buttonCell.updateIndex(index);
@@ -397,7 +409,7 @@
         comboBox.setValue(value);
     }
     
-    private int getSelectedIndex() {
+    private int getIndexOfComboBoxValueInItemsList() {
         T value = comboBox.getValue();
         int index = comboBox.getItems().indexOf(value);
         return index;
--- a/javafx-ui-controls/test/javafx/scene/control/ComboBoxTest.java	Wed Jun 13 16:18:07 2012 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/ComboBoxTest.java	Thu Jun 14 11:05:06 2012 +1200
@@ -35,13 +35,35 @@
     private ComboBox<String> comboBox;
     private SingleSelectionModel<String> sm;
     
+    /*********************************************************************
+     *                                                                   *
+     * Utility methods                                                   *
+     *                                                                   *
+     ********************************************************************/    
+    
+    public ListView getListView() {
+        return ((ComboBoxListViewSkin)comboBox.getSkin()).getListView();
+    }
+    
+    
+    
+    /*********************************************************************
+     *                                                                   *
+     * Setup                                                             *
+     *                                                                   *
+     ********************************************************************/      
+    
     @Before public void setup() {
         comboBox = new ComboBox<String>();
         sm = comboBox.getSelectionModel();
     }
     
+    
+    
     /*********************************************************************
+     *                                                                   *
      * Tests for the constructors                                        *
+     *                                                                   *
      ********************************************************************/
     
     @Test public void noArgConstructorSetsTheStyleClass() {
@@ -747,16 +769,16 @@
         comboBox.show();
         
         comboBox.setVisibleRowCount(5);
-        double initialHeight = ((ComboBoxListViewSkin)comboBox.getSkin()).getListView().getHeight();
+        double initialHeight = getListView().getHeight();
         assertFalse("initialHeight: " + initialHeight, Double.compare(0.0, initialHeight) == 0);
         
         comboBox.setVisibleRowCount(0);
-        double smallHeight = ((ComboBoxListViewSkin)comboBox.getSkin()).getListView().getHeight();
+        double smallHeight =    getListView().getHeight();
         assertTrue("smallHeight: " + smallHeight + ", initialHeight: " + initialHeight,
                 smallHeight != initialHeight && smallHeight < initialHeight);
         
         comboBox.setVisibleRowCount(7);
-        double biggerHeight = ((ComboBoxListViewSkin)comboBox.getSkin()).getListView().getHeight();
+        double biggerHeight = getListView().getHeight();
         assertTrue(biggerHeight != smallHeight && smallHeight < biggerHeight);
     } 
     
@@ -812,7 +834,7 @@
         comboBox.getSelectionModel().select(2);
         assertEquals("2", comboBox.getValue());
         
-        ListView listView = ((ComboBoxListViewSkin)comboBox.getSkin()).getListView();
+        ListView listView = getListView();
 //        listView.impl_processCSS(true);
         
         assertEquals("2", listView.getSelectionModel().getSelectedItem());
@@ -837,8 +859,7 @@
         comboBox.impl_processCSS(true);
         comboBox.show();
         
-        ListView listView = ((ComboBoxListViewSkin)comboBox.getSkin()).getListView();
-        SelectionModel sm = listView.getSelectionModel();
+        SelectionModel sm = getListView().getSelectionModel();
         
         comboBox.getSelectionModel().select(2);
         Object item = sm.getSelectedItem();