Update common container JS to use Shindig gadget handler

[mhermanto, 2010-07-15 20:42:15]

Added small new APIs to container.js

[johnfargo, 2010-07-21 09:45:25]

Looks good. Mostly structuring nits; let's sync our CLs and complete this out.

http://codereview.appspot.com/1862041/diff/9002/15004
File features/src/main/javascript/features/container/container.js (right):

http://codereview.appspot.com/1862041/diff/9002/15004#newcode204
features/src/main/javascript/features/container/container.js:204: });
why not just have this method call:
this.getGadgetMetadata(gadgetUrls, function(response) { ...stuff... } );

http://codereview.appspot.com/1862041/diff/9002/15004#newcode214
features/src/main/javascript/features/container/container.js:214: gadgetUrl,
callback) {
...thus changing gadgetUrl to gadgetUrls

http://codereview.appspot.com/1862041/diff/9002/15003
File features/src/main/javascript/features/container/gadget_holder.js (right):

http://codereview.appspot.com/1862041/diff/9002/15003#newcode269
features/src/main/javascript/features/container/gadget_holder.js:269: uri =
this.updateBooleanQueryParam_(uri, 'testmode');
feel free to use shindig.uri once committed :)

http://codereview.appspot.com/1862041/diff/9002/15001
File features/src/main/javascript/features/container/util.js (right):

http://codereview.appspot.com/1862041/diff/9002/15001#newcode140
features/src/main/javascript/features/container/util.js:140: };
shindig.uri ftw

[mhermanto, 2010-07-21 19:17:07]

Updated again

[mhermanto, 2010-07-21 19:18:12]

On 2010/07/21 19:17:07, mhermanto wrote:
> Updated again

I uploaded another patch. Please pick up latest one to submit. Thanks.

[mhermanto, 2010-07-21 19:18:45]

http://codereview.appspot.com/1862041/diff/9002/15004
File features/src/main/javascript/features/container/container.js (right):

http://codereview.appspot.com/1862041/diff/9002/15004#newcode204
features/src/main/javascript/features/container/container.js:204: });
On 2010/07/21 09:45:26, johnfargo wrote:
> why not just have this method call:
> this.getGadgetMetadata(gadgetUrls, function(response) { ...stuff... } );

Good idea on having less code, but I think service itself should deal with more
raw/flexible request and caching/transport mechanism underneath, instead of the
actual request itself. Example use case we may want to pass filtering data in
request (as already supported by endpoint, not verified by me yet). This cannot
be expressed by just gadgetUrls. Clients will not use service directly, they
will do this via this method.

http://codereview.appspot.com/1862041/diff/9002/15004#newcode214
features/src/main/javascript/features/container/container.js:214: gadgetUrl,
callback) {
On 2010/07/21 09:45:26, johnfargo wrote:
> ...thus changing gadgetUrl to gadgetUrls

Same reason.

http://codereview.appspot.com/1862041/diff/9002/15003
File features/src/main/javascript/features/container/gadget_holder.js (right):

http://codereview.appspot.com/1862041/diff/9002/15003#newcode269
features/src/main/javascript/features/container/gadget_holder.js:269: uri =
this.updateBooleanQueryParam_(uri, 'testmode');
On 2010/07/21 09:45:26, johnfargo wrote:
> feel free to use shindig.uri once committed :)
> 

For sure. :)

http://codereview.appspot.com/1862041/diff/9002/15001
File features/src/main/javascript/features/container/util.js (right):

http://codereview.appspot.com/1862041/diff/9002/15001#newcode140
features/src/main/javascript/features/container/util.js:140: };
On 2010/07/21 09:45:26, johnfargo wrote:
> shindig.uri ftw

Yes. Once shindig.uri.committed.

[johnfargo, 2010-07-22 01:31:19]

LGTM, committed.

shindig.uri is too.

On 2010/07/21 19:18:45, mhermanto wrote:
> http://codereview.appspot.com/1862041/diff/9002/15004
> File features/src/main/javascript/features/container/container.js (right):
> 
> http://codereview.appspot.com/1862041/diff/9002/15004#newcode204
> features/src/main/javascript/features/container/container.js:204: });
> On 2010/07/21 09:45:26, johnfargo wrote:
> > why not just have this method call:
> > this.getGadgetMetadata(gadgetUrls, function(response) { ...stuff... } );
> 
> Good idea on having less code, but I think service itself should deal with
more
> raw/flexible request and caching/transport mechanism underneath, instead of
the
> actual request itself. Example use case we may want to pass filtering data in
> request (as already supported by endpoint, not verified by me yet). This
cannot
> be expressed by just gadgetUrls. Clients will not use service directly, they
> will do this via this method.
> 
> http://codereview.appspot.com/1862041/diff/9002/15004#newcode214
> features/src/main/javascript/features/container/container.js:214: gadgetUrl,
> callback) {
> On 2010/07/21 09:45:26, johnfargo wrote:
> > ...thus changing gadgetUrl to gadgetUrls
> 
> Same reason.
> 
> http://codereview.appspot.com/1862041/diff/9002/15003
> File features/src/main/javascript/features/container/gadget_holder.js (right):
> 
> http://codereview.appspot.com/1862041/diff/9002/15003#newcode269
> features/src/main/javascript/features/container/gadget_holder.js:269: uri =
> this.updateBooleanQueryParam_(uri, 'testmode');
> On 2010/07/21 09:45:26, johnfargo wrote:
> > feel free to use shindig.uri once committed :)
> > 
> 
> For sure. :)
> 
> http://codereview.appspot.com/1862041/diff/9002/15001
> File features/src/main/javascript/features/container/util.js (right):
> 
> http://codereview.appspot.com/1862041/diff/9002/15001#newcode140
> features/src/main/javascript/features/container/util.js:140: };
> On 2010/07/21 09:45:26, johnfargo wrote:
> > shindig.uri ftw
> 
> Yes. Once shindig.uri.committed.