Event Bubbling: Big (Extension) Breaking Change for 0.4.1?

26 views
Skip to first unread message

Sean Gilligan

unread,
Aug 28, 2012, 4:01:47 AM8/28/12
to iui-dev...@googlegroups.com
After finishing up the 0.4 release, I took a fresh look at the iUI event
code used by extensions and realized that writing extensions could be
greatly simplified by using Event Bubbling.

If you look at Pull Request #6 on GitHub, you'll see a set of changes:
https://github.com/iui/iUI/pull/6

The very first commit is the important one:
https://github.com/msgilligan/iUI/commit/adffc5811706b26df6258f2490b9ee2ab3e61a7e
A one-line change in iui.js (set the bubbling flag to true when creating
iUI events) results in the elimination of 50 lines of code from 3 sample
extensions. It also makes understanding and writing extensions much easier.

However, I discovered that sending bubbling events can be problematic
if your names collide with existing events (e.g. "load" on Firefox) so
to make things safer I put a leading "iui." on all sent events and
updated all the extensions to listen for iui.<eventname>
The namespace change is here:
https://github.com/msgilligan/iUI/commit/d7b24d62dececee7ca27ef663c1ef8fdd4a199ea

This is a huge improvement to the event mechanism (which still needs to
be better documented) but will require any 3rd-party or site-specific
extensions to be updated to listen for iui.<eventname> I don't like to
make breaking changes in iUI (although several people have convinced me
that I've been too reluctant to make them), but I think the benefit of
this change is significant and worth the cost.

Should we do it? Does anyone have an extension or site that would be
adversely affected? (beyond the requirement of adding "iui." to your
addEventListener calls when updating iUI)

I would like to put this feature in an 0.4.1 release and release 0.4.1
as soon as possible.

Thoughts?

-- Sean

Seth Johnson

unread,
Aug 28, 2012, 10:53:28 AM8/28/12
to iui-dev...@googlegroups.com

I'm more of a lerker 'round these parts but I say.... Break It

--
You received this message because you are subscribed to the Google Groups "iui-developers" group.
To post to this group, send email to iui-developers@googlegroups.com.
To unsubscribe from this group, send email to iui-developers+unsubscribe@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/iui-developers?hl=en.

Remi Grumeau 

unread,
Aug 28, 2012, 12:13:56 PM8/28/12
to iui-dev...@googlegroups.com
Gosh! Modifications in iUI.js... did you covered your head o the sun this weekend?! :)

First, if we modify domething about events, let's do it for real. Bye bye addEventListener, we may create our own function (iui.addEvent) to deal with addEventListener/attachEvent.

addEvent : function(el, ev, cb) {
if(el.addEventListener)
el.addEventListener(ev, cb, true);
else
{
el['e'+ev+cb] = cb;
el[ev+cb] = function() {
el['e'+ev+cb](window.event);
};
el.attachEvent('on'+ev, el[ev+cb]);
}
return;
}


This may lead to a better crossbrowsers support (aka IE / WinPhone and others old craps). This is not an iOS world anymore out there as when Joe started this out.

For the rest, i'm always +1k for more documentation.
Please also keep in mind that you now may test all extensions in iui/ext-sandbox to work just fine with this modification. It's ok if it's not the case for "nighlty builds" but it has to be for public releases.

Remi
> --
> You received this message because you are subscribed to the Google Groups "iui-developers" group.
> To post to this group, send email to iui-dev...@googlegroups.com.
> To unsubscribe from this group, send email to iui-developer...@googlegroups.com.

Sean Gilligan

unread,
Aug 28, 2012, 1:04:37 PM8/28/12
to iui-dev...@googlegroups.com, Remi Grumeau 
On 8/28/12 9:13 AM, Remi Grumeau  wrote:
> Gosh! Modifications in iUI.js... did you covered your head o the sun this weekend?! :)


Change a 'false' to a 'true' and greatly simplify extensions? That's a
big win. My head was well-covered? :)


>
> First, if we modify domething about events, let's do it for real. Bye bye addEventListener,


Rewriting iUI to support the IE event model is not something I want to
do right now -- if ever. I also do not want to rewrite jQuery inside
iui.js. attachEvent is on it's way out, even for IE -- so I think we
should stick with addEventListener for now.

If we do decide to support attachEvent for some reason, that would be a
completely separate change from what is currently proposed.


>
> For the rest, i'm always +1k for more documentation.

This change should simplify the documentation (as it simplifies the
sample extensions) Do you see any issues with the change itself?


> Please also keep in mind that you now may test all extensions in iui/ext-sandbox to work just fine with this modification. It's ok if it's not the case for "nighlty builds" but it has to be for public releases.

I've tested the change with most of the sandbox and non-sandbox (aka
iui-event-log.js) extensions in 2-3 browsers and they seem OK. And I
haven't even pulled the extension in to the main branch...

If no one brings up any issues, I'll accept the pull request and merge
the code into the main branch. This will automatically trigger a CI
build (aka "nightly", but they happen on every change rather than once a
day) and then non-Git-users can download and test the zip.

We can do additional testing before making an 0.4.1 release.

-- Sean

Reply all
Reply to author
Forward
0 new messages