Hi Christian,
1)
$(window).on('click', ".sakai_add_content_overlay", initialize);
Doing this is basically doing a .live() written with an .on() [1].
We have a pretty heavy DOM atm and this would mean that all those
click events will have to bubble up to the top.
Since it probably needs to be globally available, I'm not sure
whether there is an easy fix for it.
I understand the concern, but I'm not sure there's anything we can do
and it's probably also out of scope for this ticket. This line already
exists in the current widget, as it listens to everything that wants
to initialize the overlay. I can't really think of anything that would
allow us to get rid of it.
Apart from that, doing a $(window).on('click' ... probably doesn't
work in IE8 [2]
Instead (if we really need it), it would be better to do $
(document).on('click' ...)
It would be good to test the lazy loading in IE8/9.
Cool, I was not aware of that issue. I will make the change and test
it in IE8/9.
2)
"events": ["init.newaddcontent.sakai"],
"selectors": [".sakai_add_content_overlay"],
Doing this feels a bit like duplicating behavior, wouldn't it be
good to stick with one or the other?
I would recommend going for 'events'.
If not, it would probably be good to use the widget id in the
selector (e.g. '.sakai_newaddcontent_overlay')
I think you're right in saying that each widget will probably only
want to have an events property or a selectors property. However, I
think it's useful to have both of them available. Events can be used
for more complex cases where parameters need to be passed in, or just
when you don't want to respond to a click event.
Selectors can be useful for things like the sharecontent widget, where
we can just mark-up the HTML to tell it where to offer that
functionality. If we'd use events for that, we'd have to have a copied
block of JS for each of those instances that captures the event and
triggers the event.
3) We should make sure the Unit tests are still passing for the
widget configuration.
We'll just need to add the 'trigger' property and note that it will
be an object.
Currently there is a PR standing open to fix the current tests [3].
Good point. I'll get that fixed.
Hope that helps,
Nicolaas
On Jun 26, 2012, at 8:21 AM, Nicolaas Matthijs wrote:
Hi everyone,
I'm ready to submit a pull request that will allow us to lazy load
widgets. However, before doing so, I wanted to ask the list how they
feel about the way in which this has been implemented, as it has not
been the most straightforward thing to do.
I've extended the widget configs to have the following property:
"trigger": {
"events": ["init.newaddcontent.sakai"],
"selectors": [".sakai_add_content_overlay"],
"onLoad": false
},
The events property defines an event or list of events the widget
will
respond to, and the widget will be loaded on the fly when that event
is triggered. The selectors property defines a jQuery selector or a
list of jQuery selectors the widget will respond to, and the widget
will be loaded when elements that match these selectors are clicked.
The onLoad property, which is a boolean, defines whether or not the
widget needs to be included and loaded on page load (for all pages).
This allows for the introduction of widgets like "Terms & Conditions"
or "Feedback", without having to change all of the core HTML pages.
Obviously, all of this is optional for a widget.
Because it's only loading one widget at a time when needed, the added
delay is hardly noticeable, even on a slow connection. After that,
the
widget is present and there is no delay at all. That'll become even
more valuable once we move to a single page app implementation.
The downside is that inside of the widget, the widget itself is still
binding to the events and selectors as well:
$(window).on('init.newaddcontent.sakai', initialize);
$(window).on('click', ".sakai_add_content_overlay", initialize);
This is necessary as the widget determines what it needs to call, but
it does mean there can be a tiny amount of duplication.
Doing this lazy loading allows us to cut the number of requests on a
typical non-cached page load down from 100+ to about 35 and removes
all of the background widgets out of topnavigation. Once cached, this
still saves us a big non-cached POST request (120KB) which reduces
page load time significantly on a slow connection.
Kind regards,
Nicolaas
_______________________________________________
oae-dev mailing list
[email protected]
http://collab.sakaiproject.org/mailman/listinfo/oae-dev
_______________________________________________
oae-dev mailing list
[email protected]
http://collab.sakaiproject.org/mailman/listinfo/oae-dev