FormValueWidget: fix handling of readOnly when inputNode distinct from valueNode (and small doc addition). Fixes #344

[AdrianVasiliu]

See #344.

[wkeese]

I don't understand, why do you think that FormValueWidget has an inputNode property? Or is the idea that you want to add that property?

PS: Side note: I guess we should document the valueNode property, similar to how FormWidget documents focusNode.

[AdrianVasiliu]

I don't understand, why do you think that FormValueWidget has an inputNode property? Or is the idea that you want to add that property?

The starting point is that valueNode is documented as "likely hidden", hence for a widget with readOnly at true, the current code in FormValueWidget which sets valueNode's readOnly property only prevents the form submission but does not prevent interaction, because in such a case the interaction is handled by a distinct element (obviously can't be handled by the hidden valueNode). In this context, I indeed made my (quick) PR under the wrong assumption inputNode is already in the picture, but it wasn't. So I added it as documented property via https://github.com/AdrianVasiliu/delite/commit/dfc0264e55bab0fc1cf9f062738a092a3bd6c5c3.

Now, setting inputNode's readOnly property would only have the effect to prevent the interaction if the element supports it. But this is already the case for valueNode: even some types of input element ignore this property/attribute, not speaking of other elements than input that may not even have this property. So from this point of view the situation is not that much different from inputNode than for valueNode. Also, some widgets may want to use the disabled property for the inputNode, but I think this should be left to widget's choice.

PS: Side note: I guess we should document the valueNode property, similar to how FormWidget documents focusNode.

valueNode was already documented, see https://github.com/ibm-js/delite/blob/0.5.1/FormWidget.js#L77-88.

[AdrianVasiliu]

I should add that these changes are not required for deliteful/Combobox, which does use a inputNode distinct from the hidden valueNode but handles already by itself the setting of readOnly for inputNode. I just thought it may be a useful extension of Form(Value)Widget, hence the PR, but you know better if it makes sense or not.

[wkeese]

So I added [inputNode] as documented property via AdrianVasiliu@dfc0264.

OK, but my question is whether or not it makes sense to add an inputNode property. Some form widgets don't have an "input node" at all, for example the slider or StarRating. For other widgets (in dijit) like Spinner and DateTextBox, the "input node" is already accessible via this.focusNode.

Perhaps the code should be changed to just set the disabled property on the focusNode [if the focusNode is defined and has such a property]?

valueNode was already documented, see https://github.com/ibm-js/delite/blob/0.5.1/FormWidget.js#L77-88.

Oh OK, thanks.

[AdrianVasiliu]

Some form widgets don't have an "input node" at all, for example the slider or StarRating. [...] Perhaps the code should be changed to just set the disabled property on the focusNode

Defaulting the "inputNode" to "focusNode" would be fine in my eyes, and at least for the Combobox they are actually the same node. But having a FormWidget.inputNode property would also allow a widget to have a distinct node for inputNode vs focusNode. As you know, there is a constraint on focusNode that it should be a child, not the root node itself - thus if there's no distinct inputNode this constraint also holds for it, likely without a good reason (that said, I'm not sure how likely is that a widget wants to use the root node as inputNode).

Also, in my mind inputNode wasn't required - widgets are free to set it or not - while if inputNode defaults to focusNode we'd have FormWidget setting readOnly/disabled on focusNode, which isn't necessarly what the widget needs.

Finally, you refer to setting the disabled property - I was also concerned by setting inputNode's readOnly property, such that a readOnly form widget gets its input node in readOnly mode. Some widgets might want to also set the inputNode as disabled (without the valueNode being set disabled), but other widgets may prefer to just set readOnly, because the disabled state typically sets a low opacity which decreases the readability - the graphic feedback for readOnly might need to be different than for disabled.

[wkeese]

I don't see how it would be possible for focusNode and inputNode to be different. How could a user write input into a node that wasn't focused?

[AdrianVasiliu]

Well, I just tested with a modified Combobox such that it has a distinct <input> set as focusNode, different than the editable input, and I was able to write into the later... That said, I agree it might not be a typical use case.

Anyway, since my main concern was that FormWidget currently sets readOnly on valueNode although this is likely to be hidden thus setting readOnly on it is often useless, if this would be changed to set it on focusNode as you said it would still be better than the present situation, I would say.

[wkeese]

OK, fair enough, I checked in 3feed7775fbff5965f6f7df98e159dd1e40e3bce to set readOnly on focusNode rather than valueNode, and to set disabled on both nodes.