Opened 9 years ago

Closed 8 years ago

Last modified 4 years ago

#11251 closed enhancement (fixed)

implement watch() for widgets

Reported by: bill Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5.0b2
Keywords: Cc:
Blocked By: Blocking:

Description

Implement watch() for widgets, in addition to the get()/set() support implemented as part of #10839.

Part of this work involves calling handlers registered via watch() whenever set() is called (as dojo.Stateful does), and converting widgets to internally use set() more. For example, convert TitlePane's click handler to call this.set("open", !this.get("open")).

However, there are many complicated cases to deal with, such as MappedTextBox based widgets that have both a value and a displayedValue, which are dependent on each other.

A partial list is:

  • MappedTextBox based widgets (NumberTextBox, DateTextBox, FilteringSelect, etc.): user update of the widget's value or programmatic update of either value or displayedValue should trigger watch() handlers for both displayedValue and value
  • TextBox etc, Calendar, ColorPalette InlineEditBox: user update should trigger watch() handlers on value
  • dijit.form.Form: value (hard): user/programmatic change of any child widget (TextBox etc.) alters the value of the dijit.form.Form. Supporting this would require the child widgets to propagate value change events up to the parent Form (#9692 would be useful).
  • toggleable widgets (Checkbox etc): checked
  • TitlePane: opened
  • Tree: path, selected (another example of dependent attributes)
  • StackContainer: selectedChild
  • ContentPane: content, href (note that content can be set indirectly from an HREF; set("href", ...) should trigger the watch() handlers on "content")

Extra credit for making some of the current native/synthetic widget "events" available as watchable widget attributes. For example:

  • focused (equivalent of _onFocus/_onBlur calls from focus manager)
  • hovered (equivalent of mouseenter/mouseleave native events)

I was thinking of converting all widget "events" to be accessible via watch() but it doesn't seem like a good fit for things like onClick().

Finally, all of these features will need unit tests, so it's quite a big project.

Change History (45)

comment:1 Changed 9 years ago by bill

Milestone: tbd1.6
Owner: set to bill
Status: newassigned

comment:2 Changed 9 years ago by bill

(In [22718]) Infrastructure for watch() support, add adding new watchable "focused" attribute. Refs #11251 !strict.

comment:3 Changed 9 years ago by bill

(In [22719]) Support Editor.watch() of "value", "disabled" and "disableSpellCheck". Also some spacing/lint cleanup. Added tests for onChange() and watch("value", ...). Refs #11251 !strict.

comment:4 Changed 9 years ago by liucougar

alternatively, dijit.form.Form.value may be handled by watching each widget's value? (instead of asking each child widget to propagate it's own value upward)

comment:5 Changed 9 years ago by bill

(In [22751]) fix test, refs #11251 !strict.

comment:6 Changed 9 years ago by bill

(In [23042]) Don't call watch()-registered handlers unless the value has changed. Refs #11251 !strict.

comment:7 Changed 8 years ago by bill

(In [23069]) Comment/formatting cleanup discovered during watch conversion of form widgets. No code changes. Refs #11251 !strict.

comment:8 Changed 8 years ago by bill

(In [23172]) Implement most of form widget support for watch().

Value and displayedValue:

Made watch() of "value" and "displayedValue" work for form widgets. Switched form widgets to always store current value in this.value, rather than this._lastValue. References to this.valueNode.value can probably be changed to this.value.

Some widgets still have _getValueAttr() methods, although that probably doesn't make sense because the current value is computed on every keystroke in order to be reported by watch(), and also (for MappedTextBox widgets) to be copied into this.textbox.value.

Also, _getValueAttr() is inconsistent: DateTextBox._getValueAttr() etc. computes the value from the displayedValue, but FilteringSelect._getValueAttr() needs to reference this.value, since converting displayedValue --> value is asynchronous but _getValueAttr() is a synchronous call.

Message:

Promoted private _message attribute to a watch()-able public message attribute, representing either the current error message, or the current prompt message.

Like HTML5's "validationMessage" attribute, the message attribute maintains it's value even when the TextBox is blurred. However, displayMessage("") is still called onblur for backwards compatibility, and to make the tooltip disappear (assuming that app is displaying the message using the default tooltip.)

Modified validationMessages.html to not depend on displayMessage() getting called on blur.

Other changes:

Also includes some other updates for watching various attributes (like state, constraints, message, checked).

Still need implementations for Form.watch() - should notify on change of any contained widget Need watch() tests for FilteringSelect value/displayedValue/item, CheckBox/RadioButton checked state, maybe update validationMessages test to use watch.

Refs #11251 !strict.

comment:9 Changed 8 years ago by bill

(In [23173]) watch() tests for FilteringSelect value, displayedValue, item. Plus fixing a typo.

This code updates displayedValue on every keystroke but value only when priorityChange is true. Should probably make it consistent, perhaps calling the watch() callbacks all the time but with the priorityChange flag as the fourth parameter?

Refs #11251 !strict.

comment:10 Changed 8 years ago by bill

(In [23174]) watch() support for layout widgets, plus some fixes to spacing/spelling/etc. There wasn't much to do since layout widgets don't have many mutable attributes.

Perhaps in the future child.set("selected", true) should be supported as the way to set the selected pane of a StackContainer, but that's separate from watch() support.

Refs #11251 !strict.

comment:11 Changed 8 years ago by bill

(In [23178]) Oops, accidentally added this file in [23172], refs #11251.

comment:12 Changed 8 years ago by haysmark

All of the "watch of item" tests are failing in ComboBox?; they are comparing the display value to the store's identity, which ComboBox? ignores. I thought the fix was just to change getIdentity to getLabel for ComboBox? but then the labelFunc tests fail. We might just have to skip that assert in ComboBox?.

comment:13 Changed 8 years ago by bill

(In [23183]) Fix ComboBox test failures about watch(item). Also avoid test failure for watch(displayedValue) on a !ComboBox with a labelFunc, which is just conceptually broken (see TODO comment in test). Refs #11251.

comment:14 Changed 8 years ago by bill

(In [23188]) Implement watch() for Tooltip, and also Tooltip.set("connectId", ...) as [the canonical] way to adjust which nodes Tooltip is connected to. Implementation is simplistic and could be optimized. One optimization would be to use dojo.connect() rather than this.connect() so that all the disconnect() calls are faster.

refs #11251.

comment:15 Changed 8 years ago by bill

(In [23189]) Implement watch() for Tree. Had to get rid of custom getters for item and path, and instead compute those values whenever the selected node changes.

Refs #11251 !strict.

comment:16 Changed 8 years ago by bill

(In [23190]) Implement watch() for Menu, except for targetNodeIds, which needs implementation for both set() and watch().

Refs #11251, #9610 !strict.

comment:17 Changed 8 years ago by bill

(In [23191]) Implement watch() for remaining general and infrastructure widgets (i.e. the files in top level dijit/ directory).

Refs #11251 !strict.

comment:18 Changed 8 years ago by bill

(In [23199]) Test watch("checked") for ToggleButton and CheckBox. Refs #11251.

comment:19 Changed 8 years ago by bill

(In [23213]) Implement dijit.form.Form watch() for "value" and "state". Form.value changes are tracked on onChange events from the children, which usually happen onblur of each child widget. "state" changes, OTOH, occur on every keystroke, so that apps can enable/disable the submit button in a timely manner.

get("value") continues to query the form for it's current value, including changes not yet reported from by onChange() because the field hasn't been blurred.

Notes: pressing the return key to submit a form should probably cause an onChange() event on the focused TextBox. Currently, it doesn't cause any onChange() event since the focused TextBox remains focused. However, due to the setTimeout(..., 0) in _handleOnChange() that onChange event would still come too late, after the form was submitted.

Refs #11251 !strict.

comment:20 Changed 8 years ago by bill

(In [23214]) More tests for watch(state), proper test for Form.validate(), and related bug fixes.

Refs #11251 !strict.

comment:21 Changed 8 years ago by bill

(In [23215]) Fix typo, refs #11251 !strict.

comment:22 Changed 8 years ago by bill

(In [23216]) Make connectChildren() send watch() notification to prevent test failure in ValidationState test, refs #11251 !strict.

comment:23 Changed 8 years ago by bill

(In [23217]) Fix another typo, refs #11251 !strict.

comment:24 Changed 8 years ago by bill

(In [23221]) Add tests and bug fixes for spurious watch() notifications from DateTextBox, refs #11251 !strict.

Not sure where is best for this code, but I added it to _DateTimeTextBox.js since most widgets don't deal with dates, and also since perhaps it's _DateTimeTextBox's fault that we end up with new Date objects representing the same date.

comment:25 Changed 8 years ago by bill

(In [23228]) Use watch() in _CssStateMixin rather than connecting to set(); it's a bit simpler and faster.

Also, for performance reasons, move initial class setting code to occur before widget DOM is added to the document.

Refs #11251, #11635 !strict.

comment:26 Changed 8 years ago by bill

(In [23229]) For form widget state change (error vs. OK), replace direct calls to _setStateClass() with indirect calls, triggered by watch("state"). Refs #11251 !strict.

comment:27 Changed 8 years ago by bill

(In [23230]) Remove some code from [13519] for calling _setStateClass() after open/close of drop down. AFAICT opening/closing a drop down does not affect the CSS class on the main widget's DOMNode. (It doesn't seem to affect the classes back at [13519] either.)

Refs #6365, #11251 !strict.

comment:28 Changed 8 years ago by bill

(In [23231]) Make hovering/attribute watch()-able (readonly) attributes, and replace direct calls to _setStateClass() with indirect calls, triggered by changes to hovering/active. This will all go away though in 2.0 when we'll simply use :hover and :active in CSS rules.

Also removing code to set _hovering/_active from dojox.mobile.app, since dojox.mobile.app doesn't even use _CssStateMixin, and even if it did, it wouldn't use it to track hover/active state.

Refs #11251 !strict.

comment:29 Changed 8 years ago by bill

(In [23232]) Testing handle.unwatch isn't a valid way to differentiate between watch() handles and _Widget.connect() handles because (at least on FF) every Object has a native unwatch() method.

Refs #11251 !strict.

comment:30 Changed 8 years ago by bill

(In [23233]) Convert AccordionContainer and TabContainer connect("set", ...) calls into watch() calls. Also fixing bug where changes to attributeMap attributes weren't reported to watch() handlers.

Refs #11251 !strict.

comment:31 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

Closing this ticket. Theoretically at least, the implementation is complete. I expect people will find bugs which they can open as separate tickets.

comment:32 Changed 8 years ago by bill

(In [23240]) Missing watch() notification for readOnly, refs #11251, fixes #11983 !strict.

comment:33 Changed 8 years ago by bill

(In [23243]) Make Form take advantage of "Incomplete" / "Error" / "" form widget state changes to recompute state of Form itself.

Since each child widget keeps track of it's state there's no need for the _invalidWidgets array in Form anymore; I think performance is OK without it.

Refs #11970, #11251 !strict.

comment:34 Changed 8 years ago by bill

(In [23318]) Make Dialog.open watchable, refs #11251

comment:35 Changed 8 years ago by bill

(In [23612]) fix connectId array specification using old dojoType syntax, thanks dante, refs #11251, fixes #12192 !strict.

comment:36 Changed 8 years ago by Douglas Hays

(In [25142]) Refs #11251. On a fast browser, _set could be called after setUp watch test and before formSet test, messing up the calls count.

comment:37 Changed 8 years ago by bill

(In [25264]) Avoid spurious timing related failures, refs #11251.

comment:38 Changed 8 years ago by bill

(In [25267]) Another try to avoid spurious timing related failures, refs #11251.

comment:39 Changed 8 years ago by bill

(In [25764]) Fix regression from [23172], PasswordValidator._getValueAttr() was calling this.inherited() but _FormValueWidget no longer has a _getValueAttr() method. This makes PasswordValidator.get("value") work as before, returning a String for the new password, although it's debatable whether it should instead return a hash with new and old passwords. Refs #11251, #13172 !strict.

comment:40 Changed 6 years ago by bill

In [30567]:

for _DateTimeTextBox, use this._set() that so that watch("dropDownDefaultValue", ...) works, refs #11251 !strict

comment:41 Changed 6 years ago by bill

In [30569]:

use this._set() to save value in custom setter, refs #11251

comment:42 Changed 6 years ago by bill

In [30570]:

Call this._set("xyz", ...) instead of directly setting this.xyz = ..., both so watch() works and for future support of ES5 native setters/getters. Refs #11251, #16693 !strict.

comment:43 Changed 6 years ago by bill

In [30577]:

use this._set() to save value in custom setter, and this._get() to get it, refs #11251, #16693 !strict.

comment:44 Changed 4 years ago by Bill Keese <bill@…>

In c6d9f7bb6578fc6e22a19fc1175e24094a39ebb9/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:45 Changed 4 years ago by Bill Keese <bill@…>

In e3c79a352abfc5f9350dc3336eb83c51868e3011/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.