You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2011/08/31 21:09:24 UTC

Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/
-----------------------------------------------------------

(Updated 2011-08-31 19:09:24.515118)


Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.


Changes
-------

Adding larger audience.


Summary
-------

It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.


This addresses bug SHINDIG-1607.
    https://issues.apache.org/jira/browse/SHINDIG-1607


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1161182 

Diff: https://reviews.apache.org/r/1682/diff


Testing
-------


Thanks,

Dan


Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

Posted by Dan Dumont <dd...@us.ibm.com>.

> On 2011-08-31 21:09:46, Ryan Baxter wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js, line 427
> > <https://reviews.apache.org/r/1682/diff/3/?file=36312#file36312line427>
> >
> >     check that gadgetSites is not undefined

Ahh, thank you for catching that.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/#review1704
-----------------------------------------------------------


On 2011-08-31 19:09:24, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1682/
> -----------------------------------------------------------
> 
> (Updated 2011-08-31 19:09:24)
> 
> 
> Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.
> 
> 
> Summary
> -------
> 
> It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.
> 
> 
> This addresses bug SHINDIG-1607.
>     https://issues.apache.org/jira/browse/SHINDIG-1607
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1161182 
> 
> Diff: https://reviews.apache.org/r/1682/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan
> 
>


Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/#review1704
-----------------------------------------------------------



http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js
<https://reviews.apache.org/r/1682/#comment3873>

    check that gadgetSites is not undefined


- Ryan


On 2011-08-31 19:09:24, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1682/
> -----------------------------------------------------------
> 
> (Updated 2011-08-31 19:09:24)
> 
> 
> Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.
> 
> 
> Summary
> -------
> 
> It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.
> 
> 
> This addresses bug SHINDIG-1607.
>     https://issues.apache.org/jira/browse/SHINDIG-1607
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1161182 
> 
> Diff: https://reviews.apache.org/r/1682/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan
> 
>


Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/#review1734
-----------------------------------------------------------

Ship it!


Committed revision 1164527.  Please close the review and the JIRA.

- Ryan


On 2011-09-02 13:19:47, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1682/
> -----------------------------------------------------------
> 
> (Updated 2011-09-02 13:19:47)
> 
> 
> Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.
> 
> 
> Summary
> -------
> 
> It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.
> 
> 
> This addresses bug SHINDIG-1607.
>     https://issues.apache.org/jira/browse/SHINDIG-1607
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/conservcontainer/sample-actions-voip.xml 1163664 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1163664 
> 
> Diff: https://reviews.apache.org/r/1682/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan
> 
>


Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/
-----------------------------------------------------------

(Updated 2011-09-02 13:19:47.959759)


Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.


Changes
-------

Now that actions have been fixed, the voip gadget was broken because it was declaring an action with a view target it did not have.

Removing the view attribute on the declared actions solved the issue Ryan was seeing.


Summary
-------

It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.


This addresses bug SHINDIG-1607.
    https://issues.apache.org/jira/browse/SHINDIG-1607


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/conservcontainer/sample-actions-voip.xml 1163664 
  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1163664 

Diff: https://reviews.apache.org/r/1682/diff


Testing
-------


Thanks,

Dan


Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/#review1733
-----------------------------------------------------------


Dan when I apply your patch the actions in the reference implementation no longer work, could you check it out?

- Ryan


On 2011-09-01 12:47:32, Dan Dumont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1682/
> -----------------------------------------------------------
> 
> (Updated 2011-09-01 12:47:32)
> 
> 
> Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.
> 
> 
> Summary
> -------
> 
> It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.
> 
> 
> This addresses bug SHINDIG-1607.
>     https://issues.apache.org/jira/browse/SHINDIG-1607
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1163664 
> 
> Diff: https://reviews.apache.org/r/1682/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan
> 
>


Re: Review Request: Declared actions get fired in the first instance of the gadget, ignoring any specified view target in the declaration

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1682/
-----------------------------------------------------------

(Updated 2011-09-01 12:47:32.468086)


Review request for shindig, Ryan Baxter, Matthew Hatem, and Stanton Sievers.


Changes
-------

Updated diff with fix for undefined check.


Summary
-------

It looks like the current impl of declarative actions pick the first gadget site where the URL matches and fires the action in that.  The actions spec lets you specify a target view, so we should make sure to check if the site has that view rendered.   It may also be rendered more than once, so we should fire the action in all of them.


This addresses bug SHINDIG-1607.
    https://issues.apache.org/jira/browse/SHINDIG-1607


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/actions/actions_container.js 1163664 

Diff: https://reviews.apache.org/r/1682/diff


Testing
-------


Thanks,

Dan