Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#5417 closed defect (fixed)

Widgets: support more (all?) of the on* handlers

Reported by: bill Owned by: Douglas Hays
Priority: high Milestone: 1.2
Component: Dijit Version: 0.9
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

There are a myriad of events on native HTML form nodes (like <input>), such as

  • onkeyup
  • onkeydown
  • onfocus
  • onblur
  • onmousedown

It would be nice to support the same set of events on Form widgets, so you could do

<button dojoType=dijit.form.Button onmousedown=...>
<input dojoType=dijit.form.ValidationTextBox onkeyup=...>

or

<button dojoType=dijit.form.Button>
   <script type="dojo/connect" event="onmouseover">  ... </script>
   hello world
</button>
<input dojoType=dijit.form.ValidationTextBox>
   <script type="dojo/method" event="onmouseout">  ... </script>
</input>

(I guess dojo/connect and dojo/method would have the same effect in this case.)

... or even:

  var b = new dijit.form.Button();
  dojo.connect(b, "onmousemove", myFunc);

There are two issues which make this difficult:

unnecessary connects:

The form widgets don't support the ability to connect to all these events since those events are seldom used and I didn't want to do a bunch of unnecessary connects on every widget instance.

With attributeMap, we may be able to work around the unnecessary connects problem, although I think that would only address the first example I listed above, and wouldn't help when developers try to do a dojo.connect()

more complicated than just connect:

Take for example the spinner widget. Implementing onblur and onfocus is more complicated than connecting to the onblur and onfocus of Spinner.focusNode: clicking the spinner up/down buttons will cause the Spinner.focusNode to lose focus (on some browsers), but the *widget* is still conceptually focused, so onblur shouldn't be called. In that case the _onFocus and _onBlur handlers can be used to tell when the widget itself blurs or focuses, but there may be problems with other widgets.

Leaving for consideration for 2.0 (if not sooner).

See http://www.dojotoolkit.org/forum/dijit-dijit-0-9/dijit-development-discussion/forward-onkeyup-event-dijit-form-textbox

Attachments (1)

5417.patch (10.2 KB) - added by Douglas Hays 11 years ago.
updated patch with support for onMouseEnter/Leave, widget._onConnect, and a consolidated testcase function

Download all attachments as: .zip

Change History (16)

comment:1 Changed 11 years ago by alex

Milestone: 2.01.3

Milestone 2.0 deleted

comment:2 Changed 11 years ago by bill

Description: modified (diff)

Oh, one more thing here: in onClick(), "this" points to the widget, not the focusNode, which is why the comment in _Widget.js

attributeMap: {id:"", dir:"", lang:"", "class":"", style:"", title:""},  // TODO: add on* handlers?

probably doesn't make sense. It's not as simple as that.

comment:3 Changed 11 years ago by Douglas Hays

The attached patch needs to be reviewed carefully before being committed.
It creates a hook inside dojo.connect that allows dijit to redirect the connect from the widget to the widget domNode when necessary.
Within _Widget.js, the patch iterates thru the on* event names found in the attributeMap and adds any that are specified in the markup or programmatic properties but do not have corresponding widget definitions.
Lastly, the patch adds several examples within test_DateTextBox.html (mouseover).
console.log messages for q1, q2, and 2 for q4 should show up as the mouse is moved around.

comment:4 Changed 11 years ago by bill

Hi Doug,

It would be nice to see this feature go in. Unfortunately I do have some issues with this design.

One is that I really don't want to touch the code for dojo.connect(), or even to setup around advice modifying it's behavior. I see the point that dojo.connect(widget, "onMouseOver", ...) won't work because widget doesn't have an onMouseOver method, and even if it did it wouldn't be connected to anything (unless we did 15 connects for every widget on startup). I don't see a way I like to make that work. Widget.setAttribute("onMouseOver", ...) could do the right thing though, as could specifying onMouseOver as a parameter to the constructor.

Speaking of parameters, another issues is that presumably onMouseOver etc. won't show up in the API docs because there's no onMouseOver method in the Widget (not even inside those special /*===== comments). People won't know those parameters exist.

And also, you've got Widget.js directly reading parameter values from the srcNodeRef. Only the parser is allowed to read parameter values because only the parser knows the format... in theory there might be an alternative parser with some weird format like

<div dojo="{type: dijit.Button, onMouseOver: function(){ ... }">

Admittedly that's unlikely, but so far we've kept that wall between the parser and widgets and I don't want to break it now. (Also, there's fancy code in parser.js for dealing with function parameters, perhaps dealing w/brower differences, and I would assume you would have to duplicate that.)

One final issue is that when onMouseOver is called, "this" will point to the domNode, rather than the widget itself like in Button.onClick. Oh, one more complication is supporting onFocus/onBlur. Widgets have special code to detect a logical onfocus/onblur, which is different than the actual focus/blur event because, for example, clicking a FilteringSelect's drop down button will defocus the <input> but of course the widget still logically has focus.

comment:5 Changed 11 years ago by bill

Talked to Doug about this ticket. He pointed out that to truly make dojo.connect() work for all events on widgets (without incurring the performance overhead listed above), must alter dojo.connect() is some way. That leaves the options as:

  1. just close this ticket, and make users dojo.connect(widget.domNode, ...)
  2. support onMouseOver etc. the same way we support onClick today (leading to potential performance problem listed above)
  3. only support passing handler to widget constructor (ex: new Button({onMouseOver: function(){ ... } } ), but not connect/disconnect paradigm
  4. Have special connect/disconnect type methods for widgets, like Button.listen("onMouseOver", obj, func)
  5. intercept calls to dojo.connect(myWidget, "onMouseOver", ...)

Also, Doug pointed out to me that onFocus/onBlur is already working.

Changed 11 years ago by Douglas Hays

Attachment: 5417.patch added

updated patch with support for onMouseEnter/Leave, widget._onConnect, and a consolidated testcase function

comment:6 Changed 11 years ago by Douglas Hays

Milestone: 1.31.2
Status: newassigned

comment:7 Changed 11 years ago by Douglas Hays

Summary: Form widgets: support more (all?) of the on* handlersWidgets: support more (all?) of the on* handlers

This change should apply to all widgets that inherit from _Widget.js.
The current list of events being covered by this ticket:
onClick
onDblClick
onKeyDown
onKeyPress
onKeyUp
onMouseMove
onMouseDown
onMouseOut
onMouseOver
onMouseLeave
onMouseEnter
onMouseUp

comment:8 Changed 11 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [14473]) Fixes #5417 !strict. Added lazy connecting event handlers to _Widget.js

comment:9 Changed 11 years ago by bill

See also [14485].

comment:10 Changed 11 years ago by bill

(In [14556]) Add/fix comments, and simplify one block of code. Refs #5417 !strict.

comment:11 Changed 11 years ago by bill

(In [14557]) Move code in constructor into create(), where all the other code is. Refs #5417 !strict.

comment:12 Changed 8 years ago by bill

(In [25033]) Refactor widget deferred connect code to leverage new event modules. As a fringe benefit this adds an on() method to _WidgetBase, so code can do myWidget.on("click", func) instead of dojo.connect(myWidget, "onClick", ...).

Refs #5417, #12451. Fixes #12986 !strict.

comment:13 Changed 8 years ago by bill

In [26493]:

Since dojo.connect() no longer calls _Widget.on(), need to be more explicit to get deferred connect code to work. Refs #5417, #12451 !strict.

comment:14 Changed 8 years ago by bill

In [26497]:

Using originalHandler passed to aroundAdvice, and guard against if dojo.connect() isn't defined because of has("dojo-1x-base") flag. Refs #5417, #12451 !strict.

comment:15 Changed 8 years ago by bill

In [26508]:

Safeguard against null first argument to dojo.connect(), refs #5417, #12451 !strict.

Note: See TracTickets for help on using tickets.