We don't have to remove them straight away, we can deprecate them.
Also, most institutional widget would not be widgets that are lazy
loaded (page widgets, dashboard widgets, etc.).
Nonetheless, making this change is out of scope. On top of that, we
don't have the right answer as to what the best approach is yet.
Nicolaas
On 2 Jul 2012, at 16:52, Christian Vuerings wrote:
Hey Nico,
Thanks for your reply.
We should definitely try to avoid the second scenario
+1 - letting widgets access & modify global variable is definitely
not a best practice.
Nonetheless, I think this is outside of the scope of this ticket
anyway. The submitted patch is only trying to support what's
already there, without changing the way in which the actual widgets
work. Once we decide on a strategy, we can evolve the individual
widgets to comply with the strategy and remove lazy loading
configurations options when possible.
Removing configuration options is something I would like us to avoid
when possible.
Institutions / new developers may add the 2 different configuration
options when they are moving to 1.4 to only find out that in 1.5 one
of those might be removed.
Christian
On Jul 2, 2012, at 7:23 AM, Nicolaas Matthijs wrote:
Hi Christian,
Currently, there are 4 different scenarios in our codebase:
- A number of widgets can be initialized without passing data. An
example would be the account preferences widget.
- A number of widgets are initialized without passing data, but
they do use data that's globally available. An example would be the
newaddcontent widget.
- A widget can be initialized using an event that passes in data.
- A widget can be initialized by using a specific id or CSS class
and some HTML5 data attributes
Currently we have multiple ways to pass data around, using only
'events' would enforce a more coherent way.
I agree that we might want to have a better overall strategy for
this. We should definitely try to avoid the second scenario,
however I'm not sure that recommending events for everything is
necessarily the right answer, as there are clear pros and cons for
each of these approaches. Maybe we should discuss this during one
of the UI dev calls and send a proposal to the list.
Nonetheless, I think this is outside of the scope of this ticket
anyway. The submitted patch is only trying to support what's
already there, without changing the way in which the actual widgets
work. Once we decide on a strategy, we can evolve the individual
widgets to comply with the strategy and remove lazy loading
configurations options when possible.
Hope that helps,
Nicolaas
On 26 Jun 2012, at 20:08, Christian Vuerings wrote:
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.
Is there currently a use case where we only use a selector without
passing data?
It seems like most of the current widgets (that would be
applicable for lazy loading) use some kind of mechanism to pass
data around.
1) The way we currently do this in the sharecontent widget is by
using a selector combined with an HTML5 data attribute (data-
entityid) to pass data to the widget.
2) In the newaddcontent widget we use global variables as a
mechanism to know where we need to add the content to. (isn't
really good practice)
3) With ready.contentprofile.sakai we pass the data with the event.
Wouldn't it be better if we had 1 consistent way of handling this?
It would be more straightforward for developers & make it easier
to understand how things are working.
Currently we have multiple ways to pass data around, using only
'events' would enforce a more coherent way.
- Christian
On Jun 26, 2012, at 11:07 AM, Nicolaas Matthijs wrote:
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
