[Dojo-checkins] bill - r20748 - in dijit/trunk: form tests/form

[dojo-checkins-admin at dojotoolkit.org]

Author: bill
Date: Mon Nov  9 08:12:26 2009
New Revision: 20748

Modified:
   dijit/trunk/form/Select.js
   dijit/trunk/tests/form/test_Select.html
Log:
Remove code to effectively disable a Select when there are no choices in the drop down.  Instead always show the drop down, with a blank drop down for when there are no options.  The disabling behavior could be implemented in user code or even as a subclass (or option) to Select, but it seems like the default behavior should mimic the native <select> behavior.

I can see the argument for disabling/setting readOnly on a Select with 0 or 1 elements -- to tell the user that there's no action possible -- but I can also see users getting confused about why they can't open the drop down.  With the native behavior at least it's clear what the possible options are.

Refs #10274 !strict.

Modified: dijit/trunk/form/Select.js
==============================================================================
--- dijit/trunk/form/Select.js	(original)
+++ dijit/trunk/form/Select.js	Mon Nov  9 08:12:26 2009
@@ -158,7 +158,15 @@
 			if(this.dropDown){
 				delete this.dropDown.focusedChild;
 			}
-			this.inherited(arguments);
+			if(this.options.length){
+				this.inherited(arguments);
+			}else{
+				// Drop down menu is blank but add one blank entry just so something appears on the screen
+				// to let users know that they are no choices (mimicing native select behavior)
+				dojo.forEach(this._getChildren(), function(child){ child.destroyRecursive(); });
+				var item = new dijit.MenuItem({label: "&nbsp;"});
+				this.dropDown.addChild(item);
+			}
 		}else{
 			this._updateSelection();
 		}
@@ -167,11 +175,6 @@
 		this._isLoaded = false;
 		this._childrenLoaded = true;
 
-		// Disable the select if there are no entries in the drop down, or re-enable
-		// it if entries have been added to the drop down.  (Unless there's been an explicit
-		// this.attr("disabled", true) call)
-		this._setDisabledAttr(this._iDisabled);
-
 		if(!this._loadingStore){
 			// Don't call this if we are loading - since we will handle it later
 			this._setValueAttr(this.value);
@@ -197,6 +200,9 @@
 		//		Called by oninit, onblur, and onkeypress.
 		// description:
 		//		Show missing or invalid messages if appropriate, and highlight textbox field.
+		
+		// TODO: how could a select have an invalid value?
+
 		var isValid = this.isValid(isFocused);
 		this.state = isValid ? "" : "Error";
 		this._setStateClass();
@@ -246,6 +252,9 @@
 	startup: function(){
 		if(this._started){ return; }
 
+		// TODO: I don't see this pattern being used in test_Select.html; should remove
+		// this code if it's not needed.
+
 		// the child widget from srcNodeRef is the dropdown widget.  Insert it in the page DOM,
 		// make it invisible, and store a reference to pass to the popup code.
 		if(!this.dropDown){
@@ -268,30 +277,6 @@
 		loadCallback();
 	},
 
-	_setDisabledAttr: function(/*Boolean*/ val){
-		// summary:
-		//		Effectively disables the widget if the user has explicitly set it to be disabled or
-		//		when there are no drop down entries to show.  This method should be called
-		//		whenever disabled, tabIndex, or the drop down menu changes.
-
-		// Save the actual user setting; we will need it later.
-		this._iDisabled = val;
-
-		// We disable the widget if there are no entries in the drop down, to avoid user confusion
-		this.inherited(arguments, [val || this.options.length == 0]);
-	},
-
-	_getDisabledAttr: function(){
-		return this._iDisabled;
-	},
-
-	_setTabIndexAttr: function(/*String*/ val){
-		// _setDisabledAttr() will set focusNode.tabIndex based on this.tabIndex, this.disabled, etc.
-		// so leverage that code
-		this.tabIndex = val;
-		this._setDisabledAttr(this._iDisabled);
-	},
-
 	uninitialize: function(preserveDom){
 		if(this.dropDown && !this.dropDown._destroyed){
 			this.dropDown.destroyRecursive(preserveDom);

Modified: dijit/trunk/tests/form/test_Select.html
==============================================================================
--- dijit/trunk/tests/form/test_Select.html	(original)
+++ dijit/trunk/tests/form/test_Select.html	Mon Nov  9 08:12:26 2009
@@ -91,8 +91,8 @@
 					[
 						function test_setValue(t){
 							var d = new doh.Deferred();
-							t.is({s1:"VA", s2:"CA", s3:"AL", s4: "AK", s5: "move", s7:"NY", s8:"AL",
-								s9:"CT", s10:"AL", s11:"AL", s12: "AL"}, form.attr("value"));
+							t.is(dojo.toJson({s1:"VA", s2:"CA", s3:"AL", s4: "AK", s5: "move", s6: "", s7:"NY", s8:"AL",
+								s9:"CT", s10:"AL", s11:"AL", s12: "AL"}), dojo.toJson(form.attr("value")));
 							s1.attr("value", "WA");
 							t.is("WA", s1.value);
 							setTimeout(function(){ try{ // allow onChange to fire
@@ -353,10 +353,6 @@
 			</div>
 		<hr>
 			<h4 class="testSubtitle"><label for="s6">Initially Empty</label></h4>
-			<p>
-				In particular this tests that enabled widgets are effectively disabled when there are no entries in the drop down,
-				but that they re-enable when something is added to the drop down (and re-disable when it's removed).
-			</p>
 			<select jsId="s6" name="s6" id="s6" maxHeight="100" dojoType="dijit.form.Select">
 			</select>
 			<button dojoType="dijit.form.Button" type="button">