I see.

So the getActiveSiteHolder code currently looks like this:

/**
 * @inheritDoc
 */
osapi.container.GadgetSite.prototype.getActiveSiteHolder = function() {
  return this.loadingGadgetHolder_ || this.currentGadgetHolder_;
};

This code is weird because it should be the other way:

   return this.currentGadgetHolder_ || this.loadingGadgetHolder_;

because after GadgetSite.onRender() is called and replacing
currentGadgetHolder_ with loadingGadgetHolder_ which makes it active.

So the close() function in gadget_site.js file could look like:

/**
 * @inheritDoc
 */
osapi.container.GadgetSite.prototype.close = function() {
 osapi.container.Site.prototype.close.call(this); // super.close();
 if (this.loadingGadgetEl_ && this.loadingGadgetEl_.firstChild) {
   this.loadingGadgetEl_.removeChild(this.loadingGadgetEl_.firstChild);
 }

  // dispose of our other holder if any
  if(loadingGadgetHolder_) {
    loadingGadgetHolder_.dispose();
  }
};

The odds of GadgetSite.onRender() has not finished executing when
someone call GadgetSite.close() I think is pretty rare. So in most
typical  scenarios there will be only one instance of gadget holder
active.


Thoughts?

- Henry


On Thu, Mar 22, 2012 at 8:52 PM, Dan Dumont <ddum...@us.ibm.com> wrote:
>  Yes it's the same as an if check.  The reason the code is duplicated is 
> because gadget sites have 2 holders.
> The second time it's called it should return the other one.
>
>  -----Henry Saputra <henry.sapu...@gmail.com> wrote: -----
>
>  =================
>  To: dev@shindig.apache.org@IBMUS@LOTUS
>  From: Henry Saputra <henry.sapu...@gmail.com>
>  Date: 03/22/2012 06:52PM
>  Cc: IBMUS@LOTUS
>  Subject: Question about gagdet_site.js GadgetSite.close impl for close() 
> function
>  =======================
>   In the gadget_site.js file there is this function:
>
> /**
>  * @inheritDoc
>  */
> osapi.container.GadgetSite.prototype.close = function() {
>  osapi.container.Site.prototype.close.call(this); // super.close();
>  if (this.loadingGadgetEl_ && this.loadingGadgetEl_.firstChild) {
>    this.loadingGadgetEl_.removeChild(this.loadingGadgetEl_.firstChild);
>  }
>
>  // dispose of our other holder.
>  var holder = this.getActiveSiteHolder();
>  holder && holder.dispose();
> };
>
>
> The parent site.js has this:
>
>
> /**
>  * Closes this site.
>  */
> osapi.container.Site.prototype.close = function() {
>  if (this.el_ && this.el_.firstChild) {
>    this.el_.removeChild(this.el_.firstChild);
>  }
>
>  var holder = this.getActiveSiteHolder();
>  holder && holder.dispose();
> };
>
>
> If am not sure what " holder && holder.dispose();" should do. I think
> its trying to do:
>
> if(holder) {
>  holder.dispose();
> }
>
> And the GadgetSite should not repeat the code I suppose.
>
> - Henry
>
>

Reply via email to