changeset 287:b7684dc1fa3f

RT-18972: [ComboBox] value is not update, if editable mode switched off.
author jgiles
date Fri, 20 Jan 2012 10:53:14 +1300
parents da43988fb10d
children e5e4e7faf0cf
files javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java javafx-ui-controls/src/javafx/scene/control/ComboBox.java javafx-ui-controls/src/javafx/scene/control/ComboBoxBase.java javafx-ui-controls/test/javafx/scene/control/ComboBoxTest.java
diffstat 4 files changed, 128 insertions(+), 85 deletions(-) [+]
line wrap: on
line diff
--- a/javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Thu Jan 19 10:19:13 2012 +1300
+++ b/javafx-ui-controls/src/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Fri Jan 20 10:53:14 2012 +1300
@@ -172,7 +172,7 @@
                 textField.setText(stringValue);
             }
         } else {
-            int index = comboBox.getSelectionModel().getSelectedIndex();        
+            int index = getSelectedIndex();
             listCellLabel.updateListView(listView);
             listCellLabel.updateIndex(index);
         }
@@ -194,6 +194,12 @@
         }
     }
     
+    private int getSelectedIndex() {
+        T value = comboBox.getValue();
+        int index = comboBox.getItems().indexOf(value);
+        return index;
+    }
+    
     private ListCell<T> getListCellLabel() {
         if (listCellLabel != null) return listCellLabel;
         
@@ -270,7 +276,7 @@
 
         listView.getSelectionModel().selectedIndexProperty().addListener(new InvalidationListener() {
             @Override public void invalidated(Observable o) {
-                int index = listView.getSelectionModel().getSelectedIndex();
+                int index = getSelectedIndex();
                 comboBox.getSelectionModel().select(index);
                 comboBox.setValue(listView.getSelectionModel().getSelectedItem());
                 updateDisplayNode();
--- a/javafx-ui-controls/src/javafx/scene/control/ComboBox.java	Thu Jan 19 10:19:13 2012 +1300
+++ b/javafx-ui-controls/src/javafx/scene/control/ComboBox.java	Fri Jan 20 10:53:14 2012 +1300
@@ -148,10 +148,14 @@
         valueProperty().addListener(new ChangeListener<T>() {
             @Override public void changed(ObservableValue<? extends T> ov, T t, T t1) {
                 if (getItems() == null) return;
-                int index = getItems().indexOf(t1);
-                if (index > -1) {
-                    getSelectionModel().select(index);
-                }
+                getSelectionModel().setSelectedItem(t1);
+            }
+        });
+        
+        editableProperty().addListener(new InvalidationListener() {
+            @Override public void invalidated(Observable o) {
+                // when editable changes, we reset the selection / value states
+                getSelectionModel().clearSelection();
             }
         });
     }
@@ -258,6 +262,8 @@
      *                                                                         *
      **************************************************************************/    
     
+    // Listen to changes in the selectedItem property of the SelectionModel.
+    // When it changes, set the selectedItem in the value property.
     private ChangeListener<T> selectedItemListener = new ChangeListener<T>() {
         @Override public void changed(ObservableValue<? extends T> ov, T t, T t1) {
             if (! valueProperty().isBound()) {
--- a/javafx-ui-controls/src/javafx/scene/control/ComboBoxBase.java	Thu Jan 19 10:19:13 2012 +1300
+++ b/javafx-ui-controls/src/javafx/scene/control/ComboBoxBase.java	Fri Jan 20 10:53:14 2012 +1300
@@ -100,6 +100,9 @@
      * Specifies whether the ComboBox allows for user input. When editable is 
      * true, the ComboBox has a text input area that a user may type in to. This
      * input is then available via the {@link #valueProperty() value} property.
+     * 
+     * <p>Note that when the editable property changes, the value property is 
+     * reset, along with any other relevant state.
      */
     public BooleanProperty editableProperty() { return editable; }
     public final void setEditable(boolean value) { editableProperty().set(value); }
--- a/javafx-ui-controls/test/javafx/scene/control/ComboBoxTest.java	Thu Jan 19 10:19:13 2012 +1300
+++ b/javafx-ui-controls/test/javafx/scene/control/ComboBoxTest.java	Fri Jan 20 10:53:14 2012 +1300
@@ -26,9 +26,11 @@
 
 public class ComboBoxTest {
     private ComboBox<String> comboBox;
+    private SelectionModel<String> sm;
     
     @Before public void setup() {
         comboBox = new ComboBox<String>();
+        sm = comboBox.getSelectionModel();
     }
     
     /*********************************************************************
@@ -40,7 +42,7 @@
     }
     
     @Test public void noArgConstructorSetsNonNullSelectionModel() {
-        assertNotNull(comboBox.getSelectionModel());
+        assertNotNull(sm);
     }
     
     @Test public void noArgConstructorSetsNonNullItems() {
@@ -48,11 +50,11 @@
     }
     
     @Test public void noArgConstructor_selectedItemIsNull() {
-        assertNull(comboBox.getSelectionModel().getSelectedItem());
+        assertNull(sm.getSelectedItem());
     }
     
     @Test public void noArgConstructor_selectedIndexIsNegativeOne() {
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
+        assertEquals(-1, sm.getSelectedIndex());
     }
     
     @Test public void noArgConstructor_valueIsNull() {
@@ -171,124 +173,124 @@
         SelectionModel<String> sm = new ComboBox.ComboBoxSelectionModel<String>(comboBox);
         ObjectProperty<SelectionModel<String>> other = new SimpleObjectProperty<SelectionModel<String>>(sm);
         comboBox.selectionModelProperty().bind(other);
-        assertSame(sm, comboBox.getSelectionModel());
+        assertSame(sm, sm);
     }
 
     @Test public void selectionModelCanBeChanged() {
         SelectionModel<String> sm = new ComboBox.ComboBoxSelectionModel<String>(comboBox);
         comboBox.setSelectionModel(sm);
-        assertSame(sm, comboBox.getSelectionModel());
+        assertSame(sm, sm);
     }
     
     @Test public void canSetSelectedItemToAnItemEvenWhenThereAreNoItems() {
         final String randomString = new String("I AM A CRAZY RANDOM STRING");
-        comboBox.getSelectionModel().select(randomString);
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
-        assertSame(randomString, comboBox.getSelectionModel().getSelectedItem());
+        sm.select(randomString);
+        assertEquals(-1, sm.getSelectedIndex());
+        assertSame(randomString, sm.getSelectedItem());
     }
         
     @Test public void canSetSelectedItemToAnItemNotInTheDataModel() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         final String randomString = new String("I AM A CRAZY RANDOM STRING");
-        comboBox.getSelectionModel().select(randomString);
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
-        assertSame(randomString, comboBox.getSelectionModel().getSelectedItem());
+        sm.select(randomString);
+        assertEquals(-1, sm.getSelectedIndex());
+        assertSame(randomString, sm.getSelectedItem());
     }
         
     @Test public void settingTheSelectedItemToAnItemInItemsResultsInTheCorrectSelectedIndex() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select("Orange");
-        assertEquals(1, comboBox.getSelectionModel().getSelectedIndex());
-        assertSame("Orange", comboBox.getSelectionModel().getSelectedItem());
+        sm.select("Orange");
+        assertEquals(1, sm.getSelectedIndex());
+        assertSame("Orange", sm.getSelectedItem());
     }
     
     @Test public void settingTheSelectedItemToANonexistantItemAndThenSettingItemsWhichContainsItResultsInCorrectSelectedIndex() {
-        comboBox.getSelectionModel().select("Orange");
+        sm.select("Orange");
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        assertEquals(1, comboBox.getSelectionModel().getSelectedIndex());
-        assertSame("Orange", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(1, sm.getSelectedIndex());
+        assertSame("Orange", sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionClearsWhenAllItemsAreRemoved_selectIndex0() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(0);
+        sm.select(0);
         comboBox.getItems().clear();
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals(null, comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(-1, sm.getSelectedIndex());
+        assertEquals(null, sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionClearsWhenAllItemsAreRemoved_selectIndex2() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(2);
+        sm.select(2);
         comboBox.getItems().clear();
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals(null, comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(-1, sm.getSelectedIndex());
+        assertEquals(null, sm.getSelectedItem());
     }
     
     @Test public void ensureSelectedItemRemainsAccurateWhenItemsAreCleared() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(2);
+        sm.select(2);
         comboBox.getItems().clear();
-        assertNull(comboBox.getSelectionModel().getSelectedItem());
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
+        assertNull(sm.getSelectedItem());
+        assertEquals(-1, sm.getSelectedIndex());
         
         comboBox.getItems().addAll("Kiwifruit", "Mandarin", "Pineapple");
-        comboBox.getSelectionModel().select(2);
-        assertEquals("Pineapple", comboBox.getSelectionModel().getSelectedItem());
+        sm.select(2);
+        assertEquals("Pineapple", sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionShiftsDownWhenOneNewItemIsAdded() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(1);
-        assertEquals(1, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        sm.select(1);
+        assertEquals(1, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
         
         comboBox.getItems().add(0, "Kiwifruit");
-        assertEquals(2, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(2, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionShiftsDownWhenMultipleNewItemAreAdded() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(1);
-        assertEquals(1, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        sm.select(1);
+        assertEquals(1, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
         
         comboBox.getItems().addAll(0, Arrays.asList("Kiwifruit", "Pineapple", "Mandarin"));
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
-        assertEquals(4, comboBox.getSelectionModel().getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
+        assertEquals(4, sm.getSelectedIndex());
     }
     
     @Test public void ensureSelectionShiftsUpWhenOneItemIsRemoved() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(1);
-        assertEquals(1, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        sm.select(1);
+        assertEquals(1, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
         
         comboBox.getItems().remove("Apple");
-        assertEquals(0, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(0, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionShiftsUpWheMultipleItemsAreRemoved() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(2);
-        assertEquals(2, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Banana", comboBox.getSelectionModel().getSelectedItem());
+        sm.select(2);
+        assertEquals(2, sm.getSelectedIndex());
+        assertEquals("Banana", sm.getSelectedItem());
         
         comboBox.getItems().removeAll(Arrays.asList("Apple", "Orange"));
-        assertEquals(0, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Banana", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(0, sm.getSelectedIndex());
+        assertEquals("Banana", sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionIsCorrectWhenItemsChange() {
         comboBox.setItems(FXCollections.observableArrayList("Item 1"));
-        comboBox.getSelectionModel().select(0);
-        assertEquals("Item 1", comboBox.getSelectionModel().getSelectedItem());
+        sm.select(0);
+        assertEquals("Item 1", sm.getSelectedItem());
         
         comboBox.setItems(FXCollections.observableArrayList("Item 2"));
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals(null, comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(-1, sm.getSelectedIndex());
+        assertEquals(null, sm.getSelectedItem());
     }
     
     @Test(expected=NullPointerException.class) 
@@ -323,92 +325,92 @@
     @Test public void ensureSelectionModelUpdatesValueProperty_withSelectIndex() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().select(0);
+        sm.select(0);
         assertEquals("Apple", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelUpdatesValueProperty_withSelectItem() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().select("Apple");
+        sm.select("Apple");
         assertEquals("Apple", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelUpdatesValueProperty_withSelectPrevious() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().select(2);
-        comboBox.getSelectionModel().selectPrevious();
+        sm.select(2);
+        sm.selectPrevious();
         assertEquals("Orange", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelUpdatesValueProperty_withSelectNext() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().select("Apple");
-        comboBox.getSelectionModel().selectNext();
+        sm.select("Apple");
+        sm.selectNext();
         assertEquals("Orange", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelUpdatesValueProperty_withSelectFirst() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().selectFirst();
+        sm.selectFirst();
         assertEquals("Apple", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelUpdatesValueProperty_withSelectLast() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().selectLast();
+        sm.selectLast();
         assertEquals("Banana", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelClearsValueProperty() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().select(0);
+        sm.select(0);
         assertEquals("Apple", comboBox.getValue());
         
-        comboBox.getSelectionModel().clearSelection();
+        sm.clearSelection();
         assertNull(comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelClearsValuePropertyWhenNegativeOneSelected() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         assertNull(comboBox.getValue());
-        comboBox.getSelectionModel().select(0);
+        sm.select(0);
         assertEquals("Apple", comboBox.getValue());
         
-        comboBox.getSelectionModel().select(-1);
+        sm.select(-1);
         assertNull("Expected null, actual value: " + comboBox.getValue(), comboBox.getValue());
     }
     
     @Test public void ensureValueIsCorrectWhenItemsIsAddedToWithExistingSelection() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(1);
+        sm.select(1);
         
         comboBox.getItems().add(0, "Kiwifruit");
         
-        assertEquals(2, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(2, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
         assertEquals("Orange", comboBox.getValue());
     }
     
     @Test public void ensureValueIsCorrectWhenItemsAreRemovedWithExistingSelection() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().select(1);
+        sm.select(1);
         
         comboBox.getItems().remove("Apple");
         
-        assertEquals(0, comboBox.getSelectionModel().getSelectedIndex());
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals(0, sm.getSelectedIndex());
+        assertEquals("Orange", sm.getSelectedItem());
         assertEquals("Orange", comboBox.getValue());
     }
     
     @Test public void ensureValueIsUpdatedByCorrectSelectionModelWhenSelectionModelIsChanged() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        SelectionModel sm1 = comboBox.getSelectionModel();
+        SelectionModel sm1 = sm;
         sm1.select(1);
         assertEquals("Orange", comboBox.getValue());
         
@@ -428,32 +430,32 @@
         StringProperty sp = new SimpleStringProperty("empty");
         comboBox.valueProperty().bind(sp);
         
-        comboBox.getSelectionModel().select(1);
+        sm.select(1);
         assertEquals("empty", comboBox.getValue());
     }
     
     @Test public void ensureSelectionModelUpdatesWhenValueChanges() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        assertNull(comboBox.getSelectionModel().getSelectedItem());
+        assertNull(sm.getSelectedItem());
         comboBox.setValue("Orange");
-        assertEquals("Orange", comboBox.getSelectionModel().getSelectedItem());
+        assertEquals("Orange", sm.getSelectedItem());
     }
     
     @Test public void ensureSelectionModelUpdatesWhenValueChangesToNull() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
         comboBox.setValue("Kiwifruit");
-        assertNull(comboBox.getSelectionModel().getSelectedItem());
+        assertEquals("Kiwifruit", sm.getSelectedItem());
         assertEquals("Kiwifruit", comboBox.getValue());
         comboBox.setValue(null);
-        assertEquals(null, comboBox.getSelectionModel().getSelectedItem());
-        assertEquals(-1, comboBox.getSelectionModel().getSelectedIndex());
+        assertEquals(null, sm.getSelectedItem());
+        assertEquals(-1, sm.getSelectedIndex());
         assertEquals(null, comboBox.getValue());
     }
     
     @Test public void ensureValueEqualsSelectedItemWhenNotInItemsList() {
         comboBox.getItems().addAll("Apple", "Orange", "Banana");
-        comboBox.getSelectionModel().setSelectedItem("pineapple");
-        assertEquals("pineapple", comboBox.getSelectionModel().getSelectedItem());
+        sm.setSelectedItem("pineapple");
+        assertEquals("pineapple", sm.getSelectedItem());
         assertEquals("pineapple", comboBox.getValue());
     }
     
@@ -666,4 +668,30 @@
         strPr.setValue("newvalue");
         assertTrue("PromptText cannot be bound", comboBox.getValue().equals("newvalue"));
     }
+    
+    
+    /*********************************************************************
+     * Tests for bug reports                                             *
+     ********************************************************************/    
+    
+    @Test public void test_rt18972() {
+        comboBox.getItems().addAll("Apple", "Orange", "Banana");
+        sm.select(1);
+        assertTrue(sm.isSelected(1));
+        
+        comboBox.setEditable(true);
+        comboBox.setValue("New Value");
+        
+        // there should be no selection in the selection model, as "New Value" 
+        // isn't an item in the list, however, it is a totally valid value for
+        // the value property
+        assertFalse(sm.isSelected(1));      
+        assertEquals("New Value", sm.getSelectedItem());
+        assertEquals("New Value", comboBox.getValue());
+        
+        comboBox.setEditable(false);
+        assertEquals(-1, sm.getSelectedIndex());
+        assertEquals("New Value", sm.getSelectedItem());
+        assertEquals("New Value", comboBox.getValue());
+    }
 }