You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Henry Saputra <hs...@apache.org> on 2012/01/27 23:11:34 UTC
Review Request: SHINDIG-1686 The gadgets.views.init does not check for the
view name if it contains secondary view
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3666/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
See jira: SHINDIG-1686 (https://issues.apache.org/jira/browse/SHINDIG-1686) for detail about the issue.
This addresses bug SHINDIG-1686.
https://issues.apache.org/jira/browse/SHINDIG-1686
Diffs
-----
trunk/features/src/main/javascript/features/views/views.js 1236784
trunk/features/src/test/javascript/features/views/views-init-test.js 1236784
Diff: https://reviews.apache.org/r/3666/diff
Testing
-------
Modify the JS unit test to test the subview. Run simple apps in common container with different views and subviews.
Thanks,
Henry
Re: Review Request: SHINDIG-1686 The gadgets.views.init does not check for
the view name if it contains secondary view
Posted by Stanton Sievers <si...@gmail.com>.
> On 2012-01-28 14:51:06, Stanton Sievers wrote:
> > Quick question to help clarify. This patch assumes that the conf cannot have views that have subviews. In the code, you query supportedViews for the viewMajor, for instance, "canvas". Is this saying that it's invalid for me in my container.js to have view configuration for "canvas.help"?
>
> Henry Saputra wrote:
> Yes, thats what I understand from the spec for gadgets.views.getSupportedViews in the core gadget http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Gadget.xml#gadgets.views.getSupportedViews:
> "This function only returns the primary views and does not return any secondary views. Example: if the markup indicates a set of views named Canvas.About, Profile.About, Home.About, Canvas.Help the returned views will only be Canvas, Profile, Home."
>
> So looks like the list of supported views will only return the primary list of view names without the secondary ones. I think this is bc you can have a lot of secondary views that are variants of the primary views. It would be really hard to limit what possible secondary views in the conf file.
Sounds good to me. Ship it!
- Stanton
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3666/#review4671
-----------------------------------------------------------
On 2012-01-27 22:11:34, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3666/
> -----------------------------------------------------------
>
> (Updated 2012-01-27 22:11:34)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> See jira: SHINDIG-1686 (https://issues.apache.org/jira/browse/SHINDIG-1686) for detail about the issue.
>
>
> This addresses bug SHINDIG-1686.
> https://issues.apache.org/jira/browse/SHINDIG-1686
>
>
> Diffs
> -----
>
> trunk/features/src/main/javascript/features/views/views.js 1236784
> trunk/features/src/test/javascript/features/views/views-init-test.js 1236784
>
> Diff: https://reviews.apache.org/r/3666/diff
>
>
> Testing
> -------
>
> Modify the JS unit test to test the subview. Run simple apps in common container with different views and subviews.
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: SHINDIG-1686 The gadgets.views.init does not check for
the view name if it contains secondary view
Posted by Henry Saputra <hs...@apache.org>.
> On 2012-01-28 14:51:06, Stanton Sievers wrote:
> > Quick question to help clarify. This patch assumes that the conf cannot have views that have subviews. In the code, you query supportedViews for the viewMajor, for instance, "canvas". Is this saying that it's invalid for me in my container.js to have view configuration for "canvas.help"?
Yes, thats what I understand from the spec for gadgets.views.getSupportedViews in the core gadget http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Gadget.xml#gadgets.views.getSupportedViews:
"This function only returns the primary views and does not return any secondary views. Example: if the markup indicates a set of views named Canvas.About, Profile.About, Home.About, Canvas.Help the returned views will only be Canvas, Profile, Home."
So looks like the list of supported views will only return the primary list of view names without the secondary ones. I think this is bc you can have a lot of secondary views that are variants of the primary views. It would be really hard to limit what possible secondary views in the conf file.
- Henry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3666/#review4671
-----------------------------------------------------------
On 2012-01-27 22:11:34, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3666/
> -----------------------------------------------------------
>
> (Updated 2012-01-27 22:11:34)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> See jira: SHINDIG-1686 (https://issues.apache.org/jira/browse/SHINDIG-1686) for detail about the issue.
>
>
> This addresses bug SHINDIG-1686.
> https://issues.apache.org/jira/browse/SHINDIG-1686
>
>
> Diffs
> -----
>
> trunk/features/src/main/javascript/features/views/views.js 1236784
> trunk/features/src/test/javascript/features/views/views-init-test.js 1236784
>
> Diff: https://reviews.apache.org/r/3666/diff
>
>
> Testing
> -------
>
> Modify the JS unit test to test the subview. Run simple apps in common container with different views and subviews.
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: SHINDIG-1686 The gadgets.views.init does not check for
the view name if it contains secondary view
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3666/#review4671
-----------------------------------------------------------
Quick question to help clarify. This patch assumes that the conf cannot have views that have subviews. In the code, you query supportedViews for the viewMajor, for instance, "canvas". Is this saying that it's invalid for me in my container.js to have view configuration for "canvas.help"?
- Stanton
On 2012-01-27 22:11:34, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3666/
> -----------------------------------------------------------
>
> (Updated 2012-01-27 22:11:34)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> See jira: SHINDIG-1686 (https://issues.apache.org/jira/browse/SHINDIG-1686) for detail about the issue.
>
>
> This addresses bug SHINDIG-1686.
> https://issues.apache.org/jira/browse/SHINDIG-1686
>
>
> Diffs
> -----
>
> trunk/features/src/main/javascript/features/views/views.js 1236784
> trunk/features/src/test/javascript/features/views/views-init-test.js 1236784
>
> Diff: https://reviews.apache.org/r/3666/diff
>
>
> Testing
> -------
>
> Modify the JS unit test to test the subview. Run simple apps in common container with different views and subviews.
>
>
> Thanks,
>
> Henry
>
>
Re: Review Request: SHINDIG-1686 The gadgets.views.init does not check for
the view name if it contains secondary view
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3666/#review4691
-----------------------------------------------------------
Ship it!
LGTM
- Ryan
On 2012-01-27 22:11:34, Henry Saputra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3666/
> -----------------------------------------------------------
>
> (Updated 2012-01-27 22:11:34)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> See jira: SHINDIG-1686 (https://issues.apache.org/jira/browse/SHINDIG-1686) for detail about the issue.
>
>
> This addresses bug SHINDIG-1686.
> https://issues.apache.org/jira/browse/SHINDIG-1686
>
>
> Diffs
> -----
>
> trunk/features/src/main/javascript/features/views/views.js 1236784
> trunk/features/src/test/javascript/features/views/views-init-test.js 1236784
>
> Diff: https://reviews.apache.org/r/3666/diff
>
>
> Testing
> -------
>
> Modify the JS unit test to test the subview. Run simple apps in common container with different views and subviews.
>
>
> Thanks,
>
> Henry
>
>