----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4486/#review6352 -----------------------------------------------------------
trunk/features/src/main/javascript/features/container.site.gadget/gadget_site.js <https://reviews.apache.org/r/4486/#comment13776> I didn't get the reasoning behind changing this. One scenario I can think of is a period of time where the loading holder is showing the content of the site because the current holder (which should probably be called the 'main' holder or something) is still loading (nothing yet to render) The completion of the load, iirc, removed the loading holder, and then all calls would route correctly to the current holder. With this new logic you may get the current holder before it is ready. trunk/features/src/test/javascript/features/container.url/url_site_test.js <https://reviews.apache.org/r/4486/#comment13777> Hmm, can we replace this test with another one? Why did this fail? - Dan On 2012-03-26 18:44:47, Henry Saputra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4486/ > ----------------------------------------------------------- > > (Updated 2012-03-26 18:44:47) > > > Review request for shindig and Dan Dumont. > > > Summary > ------- > > 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/site_holder.js > 1305481 > trunk/features/src/test/javascript/features/container.url/url_site_test.js > 1305481 > trunk/features/src/main/javascript/features/container.site/site.js 1305481 > > trunk/features/src/main/javascript/features/container.site.url/url_holder.js > 1305481 > > 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 > > 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 > >
