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
