Opened 13 years ago
Closed 13 years ago
#10324 closed defect (fixed)
dijit.form widgets do not disable it's corresponding valueNode
Reported by: | Pete Smith | Owned by: | Nathan Toone |
---|---|---|---|
Priority: | high | Milestone: | 1.4 |
Component: | Dijit | Version: | 1.4.0b |
Keywords: | Cc: | Douglas Hays | |
Blocked By: | Blocking: |
Description (last modified by )
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.
Attachments (2)
Change History (13)
comment:1 Changed 13 years ago by
Cc: | wildbill added |
---|---|
Milestone: | tbd → 1.5 |
Owner: | set to Nathan Toone |
Priority: | highest → normal |
severity: | critical → normal |
Summary: | dijit.__setDisabledAttr does not disable it's corresponding valueNode → dijit.form widgets do not disable it's corresponding valueNode |
comment:2 Changed 13 years ago by
Cc: | bill added; wildbill removed |
---|
comment:3 Changed 13 years ago by
Cc: | Douglas Hays added; bill removed |
---|---|
Description: | modified (diff) |
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.
comment:4 Changed 13 years ago by
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.
Changed 13 years ago by
Attachment: | formValueDisable.2.patch added |
---|
Updated patch - follows doug's suggestions
comment:6 Changed 13 years ago by
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.
comment:7 Changed 13 years ago by
Description: | modified (diff) |
---|
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
comment:8 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 Changed 13 years ago by
Milestone: | 1.5 → 1.4 |
---|
comment:10 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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?
comment:11 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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.
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?