> On 2011-12-28 19:37:51, Raminder Singh wrote:
> > I reviewed the patch and change works good.Code looks more organized now. 
> > Thanks Ankur. I have following comments 
> > 1. I dont think we need Total Votes as it adds new variable to the page and 
> > i am not able to find a use of that.
> > 2. Look and feel of previous version was better as it saves some space and 
> > still user know its a vote.

I agree with Raminder that Total Votes is redundant and increases entropy (even 
though I probably suggested it to Ankur originally).    I'm not sure which 
previous version is meant in #2.  I don't think putting the totals on the 
buttons is a good idea since this is confusing.


- Marlon


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


On 2011-12-23 22:26:48, Ankur Goyal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3300/
> -----------------------------------------------------------
> 
> (Updated 2011-12-23 22:26:48)
> 
> 
> Review request for rave and Marlon Pierce.
> 
> 
> Summary
> -------
> 
> Made the following changes:-
> -> Like, dislike and total votes displayed separately
> -> Changed the "change" function to "click" function to remove the double 
> click bug (Rave-359)
> -> Created a common handler function to update the widget ratings shown on 
> the page called through click functions
> 
> Files modified:-
> - messages.properties
> - rave_api.js
> - rave_store.js
> - store.jsp
> 
> 
> Diffs
> -----
> 
>   trunk/rave-portal-resources/src/main/webapp/script/rave_store.js 1222860 
>   trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1222860 
>   trunk/rave-portal-resources/src/main/resources/messages.properties 1222860 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/store.jsp 
> 1222860 
> 
> Diff: https://reviews.apache.org/r/3300/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ankur
> 
>

Reply via email to