dijit.form widgets do not disable it's corresponding valueNode

[Pete Smith]

Many form dijits do this on their own __setDisabledAttr methods, like the validationTextBox,

      if(this.valueNode){
                this.valueNode.disabled = value;
       }

but it really should do this in _FormValueWidget. Right now my dijit.form.Select's hidden valueNodes don't disable when I do an attr('disabled', true) on it.

We could probably remove the other dijits that do this themselves.

[Nathan Toone]

Only ValidationTextBox? does this. Bill - is there a reason why other widgets aren't doing it? Couldn't we just move that logic into _FormValueWidget as suggested?

[Nathan Toone]

Patch which fixes this issue

[bill]

Apparently the actual bug is that certain widgets, although disabled, still send their value when a form is submitted?

Looking at the code, seems like the offenders are dijit.form.Select, dijit.form.Slider, and dojox.widget.ColorPicker. Also dojox.gauge.Gauge. The widgets that already extend MappedTextBox are fine, since it has code to disable valueNode already.

I guess moving that code into _FormValueWidget makes sense. Doug what do you think?

Should also add a comment to the top of FormValueWidget defining what valueNode is, for documentation purposes:

// valueNode: [protected] DomNode
//		Hidden DomNode (usually an <input>) containing the value of this widget.
//		Used to send the value to the server when the form is submitted.
/*=====
valueNode: null,
=====*/

or something like that.

In any case I think that the code in ValidationTextBox can be removed.

On a related note:

• shouldn't dojox.widget.ColorPicker extend FormValueWidget rather than FormWidget?
• dojox.Gauge also has a valueNode (a hidden <input>) but it doesn't even extend FormWidget. Maybe it should also extend FormValueWidget.

[Douglas Hays]

The patch looks OK but it seems like adding the valueNode IF to the existing _setDisabledAttr in _FormWiget instead of creating a new _setDisabledAttr to FormValueWidget? might be better. Also, using this.attr('disabled',blah) might be better than just setting node.disabled.

[Nathan Toone]

Updated patch - follows doug's suggestions

[Nathan Toone]

Doug - is that second patch a bit more what you were thinking?

[Douglas Hays]

Looks right to me. I'm not sure what to do with Bill's suggestion on documented valueNode in _FormWidget.js. It doesn't seem to really belong there and should be in widgets that introduce it but maybe he's trying to go with defining it once and not in several places.

[bill]

Yah, mainly I wanted to make sure it was defined somewhere, although _FormWidget seems like a reasonable place since (after the patch) it's referenced from there. The comment should say something like "Optionally defines a ...".

I'm OK with this in 1.4

[Nathan Toone]

(In [20826]) Fixes #10324 - disable the underlying value widget when disabling the widget

[Pete Smith]

Gents, related to this, is if I this.attr('name', 'somethingelse') the valueNode does not change it's name either. Shouldn't the valueNode respect all of the attr's on the parent Widget?

[Douglas Hays]

I don't think IE supports changing the name attribute. If you find that IE does support this, then please open a separate ticket that we can work on in 1.5.