----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4976/#review7557 -----------------------------------------------------------
trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js <https://reviews.apache.org/r/4976/#comment16735> Did you mean 'or' instead of 'and'? I think it would also be good to create constants for target type and display type that the container can use. trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/4976/#comment16736> single quotes trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/4976/#comment16737> single quotes trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/4976/#comment16738> single quotes trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/4976/#comment16739> is there a reason why you separate out associatedContext? You check for the associatedContext in opt_containerContext and set the value to associatedContext then after you loops is done you just add it to openSocialContext. Why not remove that special check for the associated context and just have the "else" part take care of it for you? trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/4976/#comment16741> If this function is supposed to be private you should move it outside of the osapi.container.Container.addMixin. Actually the other "private" functions in here should probably be moved there as well. trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js <https://reviews.apache.org/r/4976/#comment16744> you should just return null, or you can let the function return undefined. Right now the function could return a string, null, or undefined. trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js <https://reviews.apache.org/r/4976/#comment16745> spacing is a little off here, I think you need to move this line in a bit, being picky :) - Ryan On 2012-05-04 07:28:05, Henry Saputra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4976/ > ----------------------------------------------------------- > > (Updated 2012-05-04 07:28:05) > > > Review request for shindig and Ryan Baxter. > > > Summary > ------- > > Add code to handle EE extension for OpenSocial 2.5.0 core gadget spec: > http://docs.opensocial.org/display/OSD/More+precision+for+EE+data+model > http://docs.opensocial.org/display/OSD/Core-Gadget+-+Embedded+Experiences > > I remove the unneeded model for EmbeddedExperience because ActivityEntry uses > ExtendableBean, which act like a Map, to store opensocial and its extension > so there is no need to explicitly map embed or preferredExperience with > physical class: > java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java > java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java > > > This addresses bug SHINDIG-1762. > https://issues.apache.org/jira/browse/SHINDIG-1762 > > > Diffs > ----- > > trunk/content/samplecontainer/examples/embeddedexperiences/BlogViewer.xml > PRE-CREATION > trunk/content/samplecontainer/examples/embeddedexperiences/EEContainer.js > 1333592 > trunk/content/sampledata/canonicaldb.json 1333592 > trunk/features/src/main/javascript/features/embeddedexperiences/constant.js > 1333592 > > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js > 1333592 > > trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_gadgets.js > 1333592 > trunk/features/src/main/javascript/features/embeddedexperiences/feature.xml > 1333592 > > trunk/features/src/main/javascript/features/open-views.ee/open-views-ee-container.js > 1333592 > > trunk/features/src/test/javascript/features/embeddedexperiences/embedded_experiences_container_test.js > 1333592 > > trunk/java/social-api/src/main/java/org/apache/shindig/social/core/model/EmbeddedExperienceImpl.java > 1333592 > > trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/model/EmbeddedExperience.java > 1333592 > > trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/RestfulJsonActivityEntryTest.java > 1333592 > > trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonDelete.json > 1333592 > > trunk/java/social-api/src/test/java/org/apache/shindig/social/dataservice/integration/fixtures/ActivityEntryJsonGroup.json > 1333592 > > trunk/java/social-api/src/test/java/org/apache/shindig/social/sample/spi/JsonDbOpensocialServiceTest.java > 1333592 > > Diff: https://reviews.apache.org/r/4976/diff > > > Testing > ------- > > Updated the unit tests and modify EE sample gadget to test passing additional > context and respect preferredExperience display link text. > > > Thanks, > > Henry > >
