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

Ship it!


Tested locally and seems to work as expected.

Code is fine, allows extending to more complex colour pickers in future if 
needed. (Though not sure if this is really necessary).

- Ali Lown


On March 11, 2013, 12:13 p.m., Vicente J. Ruiz Jurado wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9841/
> -----------------------------------------------------------
> 
> (Updated March 11, 2013, 12:13 p.m.)
> 
> 
> Review request for wave, Michael MacFadden, Yuri Zelikov, Ali Lown, and 
> hegsie.
> 
> 
> Description
> -------
> 
> This patch add the possibility to set color and background colors of wave 
> text. It's something that our users in kune were asking for and finally I 
> decided to implement it.
> 
> Now that I'm filling this review, I see that there is some related bug and 
> some work in progress in #WAVE-270 (Jan-2012). I hope that can be reused and 
> integrated with this patch (my approach allow to integrate other color 
> pickers). Sorry hegsie! Feel free to remix/review/improve this patch.
> 
> You should put the new icons in 
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/images/edit/
> 
> 
> This addresses bug WAVE-270.
>     https://issues.apache.org/jira/browse/WAVE-270
> 
> 
> Diffs
> -----
> 
>   src/org/waveprotocol/wave/client/testing/UndercurrentHarness.java 25abe0c 
>   src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/EditToolbar.css 
> 8bf9970 
>   src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/EditToolbar.java 
> 3e196b5 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/EditorToolbarResources.java
>  b5e616e 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/AbstractColorPicker.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/ColorHelper.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/ColorPopup.java 
> PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/ComplexColorPicker.css
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/ComplexColorPicker.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/OnColorChooseListener.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/SampleCustomColorPicker.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/SimpleColorPicker.css
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/SimpleColorPicker.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/i18n/ColorPickerMessages.java
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/i18n/ColorPickerMessages_en.properties
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/color/i18n/ColorPickerMessages_es.properties
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/images/edit/backcolor.png
>  PRE-CREATION 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/toolbar/images/edit/color.png 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9841/diff/
> 
> 
> Testing
> -------
> 
> Some text editions... setting/unsetting colors.
> 
> 
> Thanks,
> 
> Vicente J. Ruiz Jurado
> 
>

Reply via email to