You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Erin Noe-Payne <er...@mitre.org> on 2012/04/16 20:41:14 UTC
Review Request: Rave 547, 549, 550
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/
-----------------------------------------------------------
Review request for rave.
Summary
-------
Rave 547, 549, 550
Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
Diffs
-----
trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
Diff: https://reviews.apache.org/r/4739/diff
Testing
-------
Thanks,
Erin
Re: Review Request: Rave 547, 549, 550
Posted by mf...@apache.org.
> On 2012-04-16 19:18:43, Jasha Joachimsthal wrote:
> > trunk/rave-portal-resources/src/main/webapp/script/rave.js, line 436
> > <https://reviews.apache.org/r/4739/diff/1/?file=102150#file102150line436>
> >
> > We had some issues with console.log before if there was no console for that browser. Can you please remove it?
Please use rave.log instead. It checks for console and can be overridden by developers to do whatever you want from a common logging facility.
> On 2012-04-16 19:18:43, Jasha Joachimsthal wrote:
> > trunk/rave-portal-resources/src/main/webapp/script/rave.js, line 63
> > <https://reviews.apache.org/r/4739/diff/1/?file=102150#file102150line63>
> >
> > Please solve the TODO :)
We are moving to bootstrap (being worked in a branch. These styles and the markup will need to change. IMO, it would be best for you to remove the TODO and create a subtask for RAVE-477 to update this code before merging into trunk.
- mfranklin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review6944
-----------------------------------------------------------
On 2012-04-16 18:41:14, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-16 18:41:14)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>
Re: Review Request: Rave 547, 549, 550
Posted by Jasha Joachimsthal <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review6944
-----------------------------------------------------------
trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql
<https://reviews.apache.org/r/4739/#comment15489>
duplicate variable?
trunk/rave-portal-resources/src/main/webapp/script/rave.js
<https://reviews.apache.org/r/4739/#comment15490>
Please solve the TODO :)
trunk/rave-portal-resources/src/main/webapp/script/rave.js
<https://reviews.apache.org/r/4739/#comment15491>
We had some issues with console.log before if there was no console for that browser. Can you please remove it?
- Jasha
On 2012-04-16 18:41:14, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-16 18:41:14)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>
Re: Review Request: Rave 547, 549, 550
Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review7014
-----------------------------------------------------------
I didn't see the patch attached to JIRA. We will need attached to the ticket in order to apply.
trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js
<https://reviews.apache.org/r/4739/#comment15589>
Are these necessary? If so, please complete.
- mfranklin
On 2012-04-18 18:15:28, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-18 18:15:28)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/css/default.css 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>
Re: Review Request: Rave 547, 549, 550
Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review7024
-----------------------------------------------------------
trunk/rave-portal-resources/src/main/webapp/css/default.css
<https://reviews.apache.org/r/4739/#comment15610>
Lost/lonely 's' character?
- Anthony
On 2012-04-19 02:49:13, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-19 02:49:13)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/css/default.css 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>
Re: Review Request: Rave 547, 549, 550
Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review7025
-----------------------------------------------------------
Ship it!
Found one minor thing - extra random 's' character in the default.css. I will remove when I apply the patch.
- Anthony
On 2012-04-19 02:49:13, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-19 02:49:13)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/css/default.css 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>
Re: Review Request: Rave 547, 549, 550
Posted by Erin Noe-Payne <er...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/
-----------------------------------------------------------
(Updated 2012-04-19 02:49:13.906047)
Review request for rave.
Changes
-------
Provided valid rave_opensocial.VIEW_TARGETS.
Summary
-------
Rave 547, 549, 550
Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
Diffs (updated)
-----
trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
trunk/rave-portal-resources/src/main/webapp/css/default.css 1326730
trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
Diff: https://reviews.apache.org/r/4739/diff
Testing
-------
Thanks,
Erin
Re: Review Request: Rave 547, 549, 550
Posted by Erin Noe-Payne <er...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/
-----------------------------------------------------------
(Updated 2012-04-18 18:15:28.905783)
Review request for rave.
Changes
-------
Updated diff based on feedback.
Summary
-------
Rave 547, 549, 550
Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
Diffs (updated)
-----
trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
trunk/rave-portal-resources/src/main/webapp/css/default.css 1326730
trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
Diff: https://reviews.apache.org/r/4739/diff
Testing
-------
Thanks,
Erin
Re: Review Request: Rave 547, 549, 550
Posted by Erin Noe-Payne <er...@mitre.org>.
> On 2012-04-16 19:31:07, mfranklin wrote:
> > trunk/rave-portal-resources/src/main/webapp/script/rave.js, line 418
> > <https://reviews.apache.org/r/4739/diff/1/?file=102150#file102150line418>
> >
> > Why is this return statement commented out?
The behavior is not currently defined in the case that a user takes an action to open multiple instances of a singleton popup. For example the slideout popup - it does not make sense to have multiple slideouts open at a time. But nothing prevents a user from clicking to open a new slideout while one is already active.
We could just replace the old content with the new, we could give a warning, we could just ignore the new request.
- Erin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review6946
-----------------------------------------------------------
On 2012-04-16 18:41:14, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-16 18:41:14)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>
Re: Review Request: Rave 547, 549, 550
Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4739/#review6946
-----------------------------------------------------------
trunk/rave-portal-resources/src/main/webapp/script/rave.js
<https://reviews.apache.org/r/4739/#comment15492>
Why is this return statement commented out?
- mfranklin
On 2012-04-16 18:41:14, Erin Noe-Payne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4739/
> -----------------------------------------------------------
>
> (Updated 2012-04-16 18:41:14)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Rave 547, 549, 550
>
> Implement container.views.createElementForGadget to support gadgets.views.openGadget, supporting slideout, modal, and dialog popups.
>
>
> Diffs
> -----
>
> trunk/rave-demo-gadgets/src/main/webapp/open_views_demo.xml PRE-CREATION
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave.js 1326730
> trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1326730
> trunk/rave-providers/rave-opensocial-provider/rave-opensocial-client/src/main/java/org/apache/rave/provider/opensocial/config/OpenSocialEnvironment.java 1326730
>
> Diff: https://reviews.apache.org/r/4739/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Erin
>
>