You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Ankur Goyal <as...@indiana.edu> on 2012/01/03 22:15:27 UTC
Review Request: Addressed issues related to the review request:
https://reviews.apache.org/r/3300/
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3358/
-----------------------------------------------------------
Review request for rave and Marlon Pierce.
Summary
-------
Addressed the following:-
-> removed total votes
-> changed else to else if(!islike)
prevRating is needed because I used it to check if the current user earlier selected one option and later changes to a different one. So, in that case, the previous rating must be decremented.
Alsp prevRating takes three values -1 (when the current user didn't select any option), 0 (dislike), 10 (like). So I also needed to check whether it is the first selection of the current user or not. If it is, then there is no need to decrement any of the rating.
Diffs
-----
trunk/rave-portal-resources/src/main/resources/messages.properties 1226929
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/store.jsp 1226929
trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1226929
trunk/rave-portal-resources/src/main/webapp/script/rave_store.js 1226929
Diff: https://reviews.apache.org/r/3358/diff
Testing
-------
Thanks,
Ankur
Re: Review Request: Addressed issues related to the review request:
https://reviews.apache.org/r/3300/
Posted by Marlon Pierce <mp...@cs.indiana.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3358/#review4180
-----------------------------------------------------------
Ship it!
- Marlon
On 2012-01-03 21:15:27, Ankur Goyal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3358/
> -----------------------------------------------------------
>
> (Updated 2012-01-03 21:15:27)
>
>
> Review request for rave and Marlon Pierce.
>
>
> Summary
> -------
>
> Addressed the following:-
> -> removed total votes
> -> changed else to else if(!islike)
>
> prevRating is needed because I used it to check if the current user earlier selected one option and later changes to a different one. So, in that case, the previous rating must be decremented.
>
> Alsp prevRating takes three values -1 (when the current user didn't select any option), 0 (dislike), 10 (like). So I also needed to check whether it is the first selection of the current user or not. If it is, then there is no need to decrement any of the rating.
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/resources/messages.properties 1226929
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/store.jsp 1226929
> trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1226929
> trunk/rave-portal-resources/src/main/webapp/script/rave_store.js 1226929
>
> Diff: https://reviews.apache.org/r/3358/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ankur
>
>
Re: Review Request: Addressed issues related to the review request:
https://reviews.apache.org/r/3300/
Posted by Jasha Joachimsthal <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3358/#review4202
-----------------------------------------------------------
trunk/rave-portal-resources/src/main/resources/messages.properties
<https://reviews.apache.org/r/3358/#comment9471>
Dutch translation:
Likes: -> Leuk:
Dislikes: -> Niet leuk:
Total Votes: -> Aantal stemmen:
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/store.jsp
<https://reviews.apache.org/r/3358/#comment9469>
Why keep an empty label?
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/store.jsp
<https://reviews.apache.org/r/3358/#comment9470>
Why keep an empty label?
trunk/rave-portal-resources/src/main/webapp/script/rave_api.js
<https://reviews.apache.org/r/3358/#comment9468>
Please use a data-* attribute to handle the number of likes/dislikes. Parsing a substring of a literal value is error prone and it disables the possibility to translate the portal.
See http://html5doctor.com/html5-custom-data-attributes/
trunk/rave-portal-resources/src/main/webapp/script/rave_api.js
<https://reviews.apache.org/r/3358/#comment9472>
if (widgetRating.isLike) {
}
else if (!widgetRating.isLike) {
}
why did you change else into else if?
trunk/rave-portal-resources/src/main/webapp/script/rave_api.js
<https://reviews.apache.org/r/3358/#comment9473>
Please no hard coded labels
- Jasha
On 2012-01-03 21:15:27, Ankur Goyal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3358/
> -----------------------------------------------------------
>
> (Updated 2012-01-03 21:15:27)
>
>
> Review request for rave and Marlon Pierce.
>
>
> Summary
> -------
>
> Addressed the following:-
> -> removed total votes
> -> changed else to else if(!islike)
>
> prevRating is needed because I used it to check if the current user earlier selected one option and later changes to a different one. So, in that case, the previous rating must be decremented.
>
> Alsp prevRating takes three values -1 (when the current user didn't select any option), 0 (dislike), 10 (like). So I also needed to check whether it is the first selection of the current user or not. If it is, then there is no need to decrement any of the rating.
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/resources/messages.properties 1226929
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/store.jsp 1226929
> trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1226929
> trunk/rave-portal-resources/src/main/webapp/script/rave_store.js 1226929
>
> Diff: https://reviews.apache.org/r/3358/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ankur
>
>