Opened 9 years ago

Closed 6 years ago

#11767 closed enhancement (duplicate)

[patch][cla] ability to rerender a widget (w/out destroying it)

Reported by: ben hockey Owned by: bill
Priority: high Milestone: 2.0
Component: Dijit Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

this is as a result of the discussion from http://article.gmane.org/gmane.comp.web.dojo.devel/13178/

i'm attaching a patch that should provide a way to disconnect the attach points and events that are added when a template is rendered. this would then open up the possibility of rendering the template again which can be useful if your template will render differently if your widget is in a different state.

i'm also attaching a sample class that would make use of these changes. you'll see from this class that since quite a lot of code was copy/pasted from dijit._Templated there is also possibly room for defining more reusable/overridable functions in dijit._Templated.

the sample class uses mustache to render the template (based on http://gist.github.com/474430). i'll also attach the zip of all the files which provide a complete self contained demo of this.

i've run the _Templated specific tests in Chrome 6 with this patch and no errors were reported.

let me know if i've overlooked anything in terms of memory management and feel free to ask for any clarification.

Attachments (3)

11767.diff (1.1 KB) - added by ben hockey 9 years ago.
patch for dijit._Templated
_Templated.js (2.3 KB) - added by ben hockey 9 years ago.
sample class using new functionality
template.zip (5.5 KB) - added by ben hockey 9 years ago.
zip of self contained demo

Download all attachments as: .zip

Change History (16)

Changed 9 years ago by ben hockey

Attachment: 11767.diff added

patch for dijit._Templated

Changed 9 years ago by ben hockey

Attachment: _Templated.js added

sample class using new functionality

Changed 9 years ago by ben hockey

Attachment: template.zip added

zip of self contained demo

comment:1 Changed 9 years ago by ben hockey

Component: GeneralDijit
Owner: anonymous deleted
severity: normalminor

comment:2 Changed 9 years ago by bill

Milestone: tbd1.6

Doesn't it need to call _detachEvents()? I don't see that in the patch. Also, any reason that _detachPoints() and _detachEvents() are separate methods? From conversations with Dustin I was thinking of wire() and unwire() as the methods to setup/teardown attach points and attach events.

comment:3 in reply to:  2 Changed 9 years ago by ben hockey

Replying to bill:

Doesn't it need to call _detachEvents()? I don't see that in the patch.

i thought you'd ask that ;) - it doesn't need to be called because destroy will disconnect those events. if you wanted to be really cautious, you could do this._attachEvents = null; in destroyRendering.

Also, any reason that _detachPoints() and _detachEvents() are separate methods? From conversations with Dustin I was thinking of wire() and unwire() as the methods to setup/teardown attach points and attach events.

the only reason that they are separate methods is because destroyRendering only needs to take care of attachPoints without any need to disconnect the attachEvents. of course, in 2.0 we will have the liberty to change things to work more smoothly for this but for now to be backwards compatible, it was simpler (more efficient?) to have 2 methods and call them at the appropriate times. it would be possible to also have wire and unwire call these methods if wire/unwire are added before 2.0

comment:4 Changed 9 years ago by bill

Owner: set to bill
Status: newassigned

Ben,

The summary of this ticket is "provide a way to "detach" from a rendered template" but it seems like what you really want to destroy the rendering and recreate it (including unsetting/resetting dojoAttachPoint/dojoAttachEvent stuff).

So would just fixing destroyRendering() to remove the dojoAttachEvent connections do it for you? Or am I missing something? I don't see why you need any extra methods added.

In your email you mentioned "avoid the changes from destroyRendering calling this.inherited" but I don't see any bad effects from _Widget.destroyRendering(), can you tell me what _Widget.destroyRendering() erases that buildRendering() won't recreate?

comment:5 in reply to:  4 Changed 9 years ago by ben hockey

Replying to bill:

Ben,

The summary of this ticket is "provide a way to "detach" from a rendered template" but it seems like what you really want to destroy the rendering and recreate it (including unsetting/resetting dojoAttachPoint/dojoAttachEvent stuff).

yes, overall i would like to be able to repeatedly call some functions that would update the rendering based on the current state of the widget. this would need to properly unset and reset dojoAttachPoint/dojoAttachEvent stuff. so, implementing buildRendering and destroyRendering to work this way would seem to be one way to achieve it.

So would just fixing destroyRendering() to remove the dojoAttachEvent connections do it for you? Or am I missing something? I don't see why you need any extra methods added.

i think my first thought was to try and fix destroyRendering the way you're suggesting but iirc i wondered if that might not be a good idea since destroyRendering in _Widget destroys bgIframe.

i had 2 guidelines i was trying to follow to achieve what i'm trying to do:

  • only call code that is directly related to the template, no other side effects.
  • whatever functions get called need to be exact inverses of each other.

bgIframe is not strictly a part of the template and can be rather expensive to create and destroy often so that broke the first guideline. the other problem is that in all of dijit, bgIframe is never generated in buildRendering so this means that buildRendering and destroyRendering are not exact inverses of each other - breaks the 2nd guideline. this is probably why i gave up on the buildRendering/destroyRendering combination.

you'll see in the _Templated.js that i've attached to this ticket, i ended up choosing to have a single function (render) to build/destroy the template rather than having 2 functions which are exact inverses but the effect is the same.

In your email you mentioned "avoid the changes from destroyRendering calling this.inherited" but I don't see any bad effects from _Widget.destroyRendering(), can you tell me what _Widget.destroyRendering() erases that buildRendering() won't recreate?

i guess i just answered that above - bgIframe is the problem.

fwiw, i ended up implementing my own _Templated that doesn't try to extend dijit._Templated (https://gist.github.com/742067 - feel free to look at it, you can use any of the code or take ideas from it if you like it, there are no licensing issues). for what i was doing, none of the templates needed to contain widgets, or if they did, i just created them in render and placed them at an attach point. so, the code is simplified (compared to dijit._Templated) but might help show what i'm trying to achieve.

also, i don't have as much need for this feature now since it was simpler to implement my own Templated class. if you feel like this is too sticky/messy to try and include before 2.0 then i can live without it for now. it would be good to keep the idea in mind for 2.0 though.

comment:6 Changed 9 years ago by bill

Thanks for the detailed response.

I agree that it's asymmetric to destroy the bgIframe in destroyRendering() since it isn't created by buildRendering(), but OTOH won't destroyRendering remove the <iframe> DOMNode, because it's under this.domNode? So you'd be left with the bgIframe javascript object without the <iframe>. So not sure how to resolve that wart.

I checked out the _Templated.js you attached and render() seems to have that same problem, i.e. it seems like calling render() to re-render the widget will destroy the <iframe>, and it won't get recreated/reattached. Did I miss something?

comment:7 Changed 9 years ago by ben hockey

you didn't miss anything - the same problem exists in my code and perhaps i had not even considered accounting for it in my code. i know i was able to get away with it because, unlike code that exists in a library like dijit, i know every intended use case for this specific code :) - there was only 1 widget (an overlay/dialog container) that needed a bgIframe and it's template was "static" so i just used dijit._Templated for that widget.

for now, i'm focused in other directions and so this particular issue has less importance for me now than it did when i opened the ticket and it's seeming like it might be more trouble than i had expected. we can continue to discuss this if you want - i really don't mind - but if it's taking you away from things which have a higher priority then feel free to give this a lower priority and we can just keep the idea under consideration for 2.0

comment:8 Changed 9 years ago by bill

Milestone: 1.62.0

OK, well I guess it should wait until 2.0. I checked the existing dijit code and the cases of BackgroundIFrame are:

  • DialogUnderlay - Creates and destroys BackgroundIFrame on every show()/hide(). I think that's because:
    1. it's counting on popup.js to do caching of the actual <iframe> nodes
    2. to defer creating the <iframe> until it's needed (on the first show)
  • Tooltip: BackgroundIFrame created in postCreate(), although as per our discussion above it would make more sense in buildRendering()
  • dijit.popup.open() - This is also a special case because the <iframe> is attached to the wrapper (the parent of the widget being popped up), so it doesn't need to get deleted on a destroyRendering() call.

comment:9 Changed 9 years ago by bill

(In [23491]) Keep track of dojoAttachEvent's and remove them in destroyRendering(). This is the start of support for re-rendering a widget without destroying/recreating it, like would be done if using mustache.

Refs #11767 !strict.

comment:10 Changed 9 years ago by bill

(In [23496]) just fixing typo in comment, refs #11767 !strict

comment:11 Changed 7 years ago by bill

Summary: [patch][cla] provide a way to "detach" from a rendered template[patch][cla] ability to rerender a widget (w/out destroying it)

comment:12 Changed 6 years ago by bill

In [30703]:

Add a _rendered parameter to _TemplatedMixin subclasses to tell them to bypass the template substitution (ie: swapping out the srcNodeRef for the template DOM). This is useful if the template has already been rendered on the server (see http://jamesthom.as/blog/2013/01/15/server-side-dijit/).

Fixes #16768, refs #11767 !strict

comment:13 Changed 6 years ago by bill

Resolution: duplicate
Status: assignedclosed

Duplicate of #16889.

Note: See TracTickets for help on using tickets.