You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2012/06/26 20:03:41 UTC

Re: Review Request: Cleanup the sites and holder close flow to delegate removing iframe of the gadget wrapper element to the holder from site responsibility

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4486/#review8614
-----------------------------------------------------------


Hey Henry, what's the status of this review?   Committed?  Not needed?

- Dan Dumont


On March 27, 2012, 4:09 p.m., Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4486/
> -----------------------------------------------------------
> 
> (Updated March 27, 2012, 4:09 p.m.)
> 
> 
> Review request for shindig and Dan Dumont.
> 
> 
> Description
> -------
> 
> As part of continuing refactoring of the site and holder code, this is another cleanup to simplify the close flow:
> 1. Move the responsibility of delete first element of the gadget HTML element wrapper to site_holder
> 2. The GadgetSite.getActiveSiteHolder now check for currentGadgetHolder_ then loadingGadgetHolder_ because its a public API that should modify currently active holder.
> 3. osapi.container.SiteHolder.prototype.dispose now responsible  to remove child element.
> 4. osapi.container.GadgetHolder.prototype.dispose and osapi.container.UrlHolder.prototype.dispose need to call parent function 
> 
> 
> Diffs
> -----
> 
>   trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1305481 
>   trunk/features/src/main/javascript/features/container.site.gadget/gadget_site.js 1305481 
>   trunk/features/src/main/javascript/features/container.site.url/url_holder.js 1305481 
>   trunk/features/src/main/javascript/features/container.site/site.js 1305481 
>   trunk/features/src/main/javascript/features/container.site/site_holder.js 1305481 
>   trunk/features/src/test/javascript/features/container.url/url_site_test.js 1305481 
> 
> Diff: https://reviews.apache.org/r/4486/diff/
> 
> 
> Testing
> -------
> 
> Modify the jsunit and pass the unit tests. Run it through common container test page.
> 
> 
> Thanks,
> 
> Henry Saputra
> 
>