> 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). > > Vicente J. Ruiz Jurado wrote: > 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?
Not from me. - Ali ----------------------------------------------------------- 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 > >
