You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by "Dennis Laan, van der" <d....@rug.nl> on 2012/03/23 16:53:36 UTC

Review Request: Support height attribute of open social gadget definition

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

Review request for rave.


Summary
-------

An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well. 


This addresses bug RAVE-526.
    https://issues.apache.org/jira/browse/RAVE-526


Diffs
-----

  trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528 

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


Testing
-------

Tested on gadgets: 
- without height attribute and without dynamic height, 
- without height and with dynamic height, 
- with height and without dynamic height,
- with height and with dynamic height


Thanks,

Dennis


Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4468/#review6288
-----------------------------------------------------------

Ship it!


Looks good.  I will apply it later today unless anyone else reviews with changes.

- mfranklin


On 2012-03-23 16:27:52, Dennis Laan, van der wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4468/
> -----------------------------------------------------------
> 
> (Updated 2012-03-23 16:27:52)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
> Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well. 
> 
> 
> This addresses bug RAVE-526.
>     https://issues.apache.org/jira/browse/RAVE-526
> 
> 
> Diffs
> -----
> 
>   trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528 
> 
> Diff: https://reviews.apache.org/r/4468/diff
> 
> 
> Testing
> -------
> 
> Tested on gadgets: 
> - without height attribute and without dynamic height, 
> - without height and with dynamic height, 
> - with height and without dynamic height,
> - with height and with dynamic height
> 
> 
> Thanks,
> 
> Dennis
> 
>


Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by Jasha Joachimsthal <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4468/#review6437
-----------------------------------------------------------

Ship it!


Thanks for the patch, I'll apply it. 
Please make a diff from the src tree next time, took me a few mins to find out that I had patched the js file in the target dir.

- Jasha


On 2012-03-23 16:27:52, Dennis Laan, van der wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4468/
> -----------------------------------------------------------
> 
> (Updated 2012-03-23 16:27:52)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
> Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well. 
> 
> 
> This addresses bug RAVE-526.
>     https://issues.apache.org/jira/browse/RAVE-526
> 
> 
> Diffs
> -----
> 
>   trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528 
> 
> Diff: https://reviews.apache.org/r/4468/diff
> 
> 
> Testing
> -------
> 
> Tested on gadgets: 
> - without height attribute and without dynamic height, 
> - without height and with dynamic height, 
> - with height and without dynamic height,
> - with height and with dynamic height
> 
> 
> Thanks,
> 
> Dennis
> 
>


Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by "Dennis Laan, van der" <d....@rug.nl>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4468/
-----------------------------------------------------------

(Updated 2012-03-23 16:27:52.624720)


Review request for rave.


Changes
-------

Made changes based on comments from matt franklin:
- moved all sizing code to getSizeFromElement and renamed it calculateSize
- removed commented code about width and set size to 100% in new calculateSize


Summary
-------

An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well. 


This addresses bug RAVE-526.
    https://issues.apache.org/jira/browse/RAVE-526


Diffs (updated)
-----

  trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528 

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


Testing
-------

Tested on gadgets: 
- without height attribute and without dynamic height, 
- without height and with dynamic height, 
- with height and without dynamic height,
- with height and with dynamic height


Thanks,

Dennis


Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4468/#review6284
-----------------------------------------------------------



trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js
<https://reviews.apache.org/r/4468/#comment13627>

    Why not move this logic into getSizeFromElement and rename the function calculateSize ?



trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js
<https://reviews.apache.org/r/4468/#comment13628>

    if you don't mind, set width to size.width and just replace elem.clientWidth - OFFSET with "100%" in the function.
    
    I know this one isn't your doing but, nothing wrong with fixing it while you are there :)


- mfranklin


On 2012-03-23 15:55:22, Dennis Laan, van der wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4468/
> -----------------------------------------------------------
> 
> (Updated 2012-03-23 15:55:22)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
> Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well. 
> 
> 
> This addresses bug RAVE-526.
>     https://issues.apache.org/jira/browse/RAVE-526
> 
> 
> Diffs
> -----
> 
>   trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528 
> 
> Diff: https://reviews.apache.org/r/4468/diff
> 
> 
> Testing
> -------
> 
> Tested on gadgets: 
> - without height attribute and without dynamic height, 
> - without height and with dynamic height, 
> - with height and without dynamic height,
> - with height and with dynamic height
> 
> 
> Thanks,
> 
> Dennis
> 
>


Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by Jasha Joachimsthal <j....@onehippo.com>.
I think it's not Jira but review board. If I look in the Jira config,
everything is configured for dev@rave

On 27 March 2012 20:29, Henry Saputra <he...@gmail.com> wrote:

> Looks like jira updates for Rave still sent to the incubator list.
>
> - Henry
>
> On Fri, Mar 23, 2012 at 8:55 AM, Dennis Laan, van der
> <d....@rug.nl> wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4468/
> > -----------------------------------------------------------
> >
> > (Updated 2012-03-23 15:55:22.362174)
> >
> >
> > Review request for rave.
> >
> >
> > Changes
> > -------
> >
> > Added issue number to summary
> >
> >
> > Summary (updated)
> > -------
> >
> > An optional attribute for the ModulePrefs element in a gadget definition
> is 'height', which states the static or default height of a gadget. Rave
> currently has a default height of 250px, which is added to the iframe
> element in which a gadget is created. It does not check if a height
> attribute is set on the gadget to render.
> > Because iGoogle supports the height attribute and a lot of available
> gadgets set their height statically, it would be a nice feature for Rave to
> support the height attribute as well.
> >
> >
> > This addresses bug RAVE-526.
> >    https://issues.apache.org/jira/browse/RAVE-526
> >
> >
> > Diffs
> > -----
> >
> >  trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js
> 1295528
> >
> > Diff: https://reviews.apache.org/r/4468/diff
> >
> >
> > Testing
> > -------
> >
> > Tested on gadgets:
> > - without height attribute and without dynamic height,
> > - without height and with dynamic height,
> > - with height and without dynamic height,
> > - with height and with dynamic height
> >
> >
> > Thanks,
> >
> > Dennis
> >
>

Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by Henry Saputra <he...@gmail.com>.
Looks like jira updates for Rave still sent to the incubator list.

- Henry

On Fri, Mar 23, 2012 at 8:55 AM, Dennis Laan, van der
<d....@rug.nl> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4468/
> -----------------------------------------------------------
>
> (Updated 2012-03-23 15:55:22.362174)
>
>
> Review request for rave.
>
>
> Changes
> -------
>
> Added issue number to summary
>
>
> Summary (updated)
> -------
>
> An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
> Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well.
>
>
> This addresses bug RAVE-526.
>    https://issues.apache.org/jira/browse/RAVE-526
>
>
> Diffs
> -----
>
>  trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528
>
> Diff: https://reviews.apache.org/r/4468/diff
>
>
> Testing
> -------
>
> Tested on gadgets:
> - without height attribute and without dynamic height,
> - without height and with dynamic height,
> - with height and without dynamic height,
> - with height and with dynamic height
>
>
> Thanks,
>
> Dennis
>

Re: Review Request: RAVE526. Support height attribute of open social gadget definition

Posted by "Dennis Laan, van der" <d....@rug.nl>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4468/
-----------------------------------------------------------

(Updated 2012-03-23 15:55:22.362174)


Review request for rave.


Changes
-------

Added issue number to summary


Summary (updated)
-------

An optional attribute for the ModulePrefs element in a gadget definition is 'height', which states the static or default height of a gadget. Rave currently has a default height of 250px, which is added to the iframe element in which a gadget is created. It does not check if a height attribute is set on the gadget to render.
Because iGoogle supports the height attribute and a lot of available gadgets set their height statically, it would be a nice feature for Rave to support the height attribute as well. 


This addresses bug RAVE-526.
    https://issues.apache.org/jira/browse/RAVE-526


Diffs
-----

  trunk/rave-portal-resources/src/main/webapp/script/rave_opensocial.js 1295528 

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


Testing
-------

Tested on gadgets: 
- without height attribute and without dynamic height, 
- without height and with dynamic height, 
- with height and without dynamic height,
- with height and with dynamic height


Thanks,

Dennis