> On March 21, 2013, 2:05 p.m., Ali Lown wrote: > > 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).
Another simple color picker that I want to develop is one that remembers the previous color selected (a GDocs do). In kune.cc we have another gwt color picker, but we are not very happy to add it to Wave (you can test it there). In the future, with other versions of GWT, we can use some other color pickers: http://www.subshell.com/en/subshell/blog/Implementing-a-Color-Picker-Dialog-With-Canvas-and-GWT100.html Any other comment to this review? - Vicente J. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9841/#review18206 ----------------------------------------------------------- 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 > >
