shindig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From johnfa...@gmail.com
Subject Re: Simpler taming of opensocial, gadgets and related apis
Date Tue, 20 Oct 2009 21:53:57 GMT
Aside from the rather minor comments below, my primary concern is that
this adds bulk to every gadget render, not just those that are cajoled.

This said, I agree it's the right move to include tamings with features
themselves. It appears that tamings for gadget and container context are
equivalent - if so, a new RenderingContext.CAJA might be in order:
<tamings>
   <script src="..."/>
</tamings>

Or we could try to genericize this by adding extensible syntax:
<ext:caja>
   <script src="..."/'>
</ext:caja>
...the content of which could be read by a Rewriter
(CajaContentRewriter) and injected as needed.

...or something similar. Thoughts?


http://codereview.appspot.com/135051/diff/1027/48
File features/src/main/javascript/features/caja/taming.js (right):

http://codereview.appspot.com/135051/diff/1027/48#newcode105
Line 105: var tamings___ = tamings___ || [];
Not that it's a big deal in this case, but maybe it should be. This is
one of a few use cases I've seen arise that call for a clearer
representation of the feature dependency tree.

http://codereview.appspot.com/135051/diff/1027/48#newcode115
Line 115: ___.grantFunc(imports.outers.console, 'log');
nit: I wonder if this is necessary, vs. just encouraging people to use
gadgets.log.

http://codereview.appspot.com/135051/diff/1027/46
File features/src/main/javascript/features/flash/taming.js (right):

http://codereview.appspot.com/135051/diff/1027/46#newcode1
Line 1: /*
Missing a corresponding feature.xml update for flash.

http://codereview.appspot.com/135051/diff/1027/46#newcode36
Line 36: var O = ifr.contentWindow.Object;
possible mini-optimization: pull A and O above var cleanse to avoid
having to recreate the frame.

http://codereview.appspot.com/135051/diff/1027/54
File
features/src/main/javascript/features/opensocial-jsonrpc/jsonrpccontainer.js
(right):

http://codereview.appspot.com/135051/diff/1027/54#newcode59
Line 59: var JsonRpcRequestItem = function(rpc, opt_processData) {
nit: spacing 2 to the left.

http://codereview.appspot.com/135051

Mime
View raw message