Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#13785 closed enhancement (fixed)

publish DOM events corresponding to widget events

Reported by: bill Owned by: bill
Priority: high Milestone: 1.8
Component: Dijit Version: 1.6.1
Keywords: Cc:
Blocked By: Blocking:

Description

Use on.emit() to publish DOM events corresponding to widget events (ex: "click"). This will allow app code to treat widgets more like native DOMNodes, and also to setup high level event handlers that can catch/handle events from multiple descendant widgets. (The same technique can be used by widgets themselves, such as a Menu catching events from child MenuItems).

Have to be careful to avoid an infinite loop in a case for a widget w/a template like this:

<div data-dojo-attach-event="onclick: _onClick">click me</div>

_onClick() publishing a click event on this.domNode would actually retrigger _onClick(). So probably need to temporarily disable that _onClick() handler. That's easy to do for any handlers registered via _WidgetBase.connect(), including handlers registered through data-dojo-attach-event, but might be a problem if any users are doing a direct dojo.connect(myWidget.domNode, "click", ...) for some reason.

Could alternately require the widget to put the attach-event on a child of this.domNode, but that's too much of a burden, especially for users with custom widgets.

Attachments (2)

emit1.patch (10.3 KB) - added by bill 8 years ago.
infrastructure plus using it for TabContainer
attr.patch (7.9 KB) - added by bill 8 years ago.
attrmodified event

Download all attachments as: .zip

Change History (20)

Changed 8 years ago by bill

Attachment: emit1.patch added

infrastructure plus using it for TabContainer

comment:1 Changed 8 years ago by bill

See #14728 and #14729 for enhancements to the event code that would make this easier, and http://thread.gmane.org/gmane.comp.web.dojo.devel/16773 for the mailing list discussion.

comment:2 Changed 8 years ago by bill

In [27937]:

Modify ondijitclick to convert keyboard ENTER/SPACE key events into actual click events, so that the click events bubble, and apps can catch the events on an ancestor node, using on(wrapperNode, "click", ...).

The other option was to tell apps to do on(wrapperNode, _OnDijitClick.a11yclick, ...), but I worry that's confusing, given the name of the existing event dijit.form.Button.onClick(), and so I worry that apps will end up doing on(wrapperNode, "click", ...), and thus being non-accessible.

For longer term, I'd like to convert a lot of the current clickable <span> and <div> nodes into <button> nodes, thus avoiding the need for ondijitclick altogether. However, that can't be done until we desupport IE6/IE7 (due to glitches they have rendering <button>'s), and also can't be done for cases like ComboButton and Menu where for formatting reasons the clickable nodes need to be <td> nodes.

Finally, there's a subtle bug both before and after this checkin that an app can't call evt.preventDefault() on a button's keypress/keydown event in order to stop the click event from being generated and stop the form from submitting. Probably that won't be an issue in practice because (to support mouse users) apps would instead call evt.preventDefault() on the click event.

Refs #13785, #14734 !strict.

comment:3 Changed 8 years ago by bill

In [27944]:

Process clicks to select or close tabs/panes in TabController/StackController, rather than the individual tabs/buttons.

Refs #13785 !strict.

comment:4 Changed 8 years ago by bill

In [27947]:

Check in benchmark for on.emit(), plus a few other test files. Making separate directory for dojo/on tests. Refs #13785 !strict.

Changed 8 years ago by bill

Attachment: attr.patch added

attrmodified event

comment:5 Changed 8 years ago by bill

In [27975]:

Enhance _WidgetBase to emit "attrmodified" DOM event whenever a widget's attribute is modified.

Update _WidgetBase.on("foo", ...) to monitor for events on this.domNode, unless the widget has an onFoo() method, in which case on() will connect to that, for backwards compatibility.

Update StackController to monitor bubbled events to tell when the StackContainer's child widget has been modified, and perform necessary updates to tab labels.

I do notice a lot of spurious events during page load, things like _KeyNavContainer setting the tabIndex on widgets after they've been created. Should try to avoid those events in the future.

Refs #13785 !strict.

comment:6 Changed 8 years ago by bill

In [28120]:

Add _WidgetBase._adoptHandles() method to track handles returned by dojo/on etc., and release them when the widget is destroyed.

Prevent memory leak on IE6/7 for connections made via _WidgetBase.on(), where the widget was destroyed but the connection was never explicitly remove()'d.

Refs #13785 !strict.

comment:7 Changed 8 years ago by ben hockey

in addition to the _started check when emitting events on the domNode (added in r27975) there should be a check for if the widget is destroyed - and/or a check to see if domNode exists.

comment:8 Changed 8 years ago by bill

OK sure, I wouldn't expect any event during teardown but perhaps you are seeing some?

comment:9 in reply to:  8 Changed 8 years ago by ben hockey

Replying to bill:

OK sure, I wouldn't expect any event during teardown but perhaps you are seeing some?

http://bugs.dojotoolkit.org/browser/dojo/dojox/trunk/mobile/_ItemBase.js?rev=28099#L254

comment:10 Changed 8 years ago by bill

In [28235]:

Avoid exception calling emit() after widget is destroyed (when there is no domNode), or during teardown, refs #13785 !strict. Depends on this._beingDestroyed remaining true after the widget is destroyed.

comment:11 Changed 8 years ago by bill

Cc: Dustin Machi Kris Zyp removed

Note: still thinking about replacing the "attrmodified" event with specific events for each attribute. Someone on dojo-contributors tried something with an event named "change:foo", but the colon clashed with dojo/on somehow, I guess because it looks like a pseudo-event, but maybe names like "change-foo" or "attrmodified-foo".

comment:12 Changed 8 years ago by bill

In [28328]:

Make separate event names for each modified widget attribute, ex: attrmodified-selectedchildwidget. Seems safer for performance. Refs #13785 !strict.

comment:13 Changed 8 years ago by bill

In [28329]:

Use event bubbling to monitor value, validity, etc. state changes in child form widgets. Refs #13785 !strict.

comment:14 Changed 8 years ago by bill

In [28568]:

Fix double bubble of Button click event, and add tests, refs #13785 !strict.

comment:15 Changed 8 years ago by bill

Resolution: fixed
Status: newclosed

comment:16 Changed 8 years ago by bill

In [28634]:

Test code was comparing a string to an array, and for some reason it worked until [27975], when it started failing on chrome windows. (It should never have worked anywhere.) Fixing it to compare string to string, refs #13785 tangentially.

comment:17 Changed 7 years ago by bill

In [29368]:

Revert signature changes of onButtonClick() and onCloseButtonClick() that could break user defined subclasses. Also add null check to prevent exception in subclasses. Refs #13785 !strict.

comment:18 Changed 7 years ago by bill

In [29391]:

Don't require buttonWidgetClass property; it makes things hard for subclasses of StackContainer. Refs #13785 !strict.

Note: See TracTickets for help on using tickets.