incubator-wave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yuri Zelikov" <vega...@gmail.com>
Subject Re: Review Request: Add color and backcolor buttons to edit toolbar
Date Sun, 07 Apr 2013 18:51:44 GMT


> 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?
> 
> Ali Lown wrote:
>     Not from me.
> 
> Yuri Zelikov wrote:
>     I got a shiny when tried to test this.
>     Stack trace:
>     
>     Token:  1365103711828
>      Repairer triggered, handleMissing
>       Unknown.$collect (JsArrayString.java:42)
>       Unknown.fillInStackTrace_2 (StackTraceCreator.java:147)
>       Unknown.fillInStackTrace_0 (StackTraceCreator.java:387)
>       Unknown.fillInStackTrace (Throwable.java:72)
>       Unknown.$$init_10 (Throwable.java:46)
>       Unknown.Throwable_2 (Throwable.java:56)
>       Unknown.Error_2 (Error.java:30)
>       Unknown.AssertionError_1 (AssertionError.java:51)
>       Unknown.throwAssertionError_Object (Exceptions.java:66)
>       Unknown.$handleMissing (Repairer.java:128)
>       Unknown.$handle (Repairer.java:106)
>       Unknown.$findNodeletWithOffset_0 (ContentTextNode.java:506)
>       Unknown.$implSplitText (ContentTextNode.java:235)
>       Unknown.$splitText_3 (ContentTextNode.java:213)
>       Unknown.$splitText_2 (ContentRawDocument.java:221)
>       Unknown.splitText_2 (ContentRawDocument.java:218)
>       Unknown.splitText_1 (PersistentContent.java:388)
>       Unknown.$splitCurrent (IndexedDocumentImpl.java:1256)
>       Unknown.$splitText_0 (IndexedDocumentImpl.java:1477)
>       Unknown.$splitText_1 (ContentDocument.java:1066)
>       Unknown.splitText_0 (ContentDocument.java:1063)
>       Unknown.ensureNodeBoundaryReturnNextNode (DocHelper.java:721)
>       Unknown.$ensureNodeBoundary (AnnotationPainter.java:370)
>       Unknown.$doRun (AnnotationPainter.java:193)
>       Unknown.execute_37 (AnnotationPainter.java:89)
>       Unknown.$workUnit (BrowserBackedScheduler.java:276)
>       Unknown.$workAll (BrowserBackedScheduler.java:323)
>       Unknown.$workSlice (BrowserBackedScheduler.java:371)
>       Unknown.run_17 (BrowserBackedScheduler.java:44)
>       Unknown.run_18 (GwtSimpleTimer.java:49)
>       Unknown.fire (Timer.java:141)
>       Unknown.anonymous (Timer.java:60)
>       Unknown.apply (Impl.java:168)
>       Unknown.entry0 (Impl.java:214)
>       Unknown.anonymous (Impl.java:57)
> 
> Yuri Zelikov wrote:
>     I noticed that it happens only when compiled in the dev mode using compile-gwt-dev,
it seems to work fine if compiled with compile-gwt
> 
> Vicente J. Ruiz Jurado wrote:
>     Works to me also with compile-gwt-dev (In Chrome).

Tried again and I can reproduce it when compiled with compile-gwt-dev. Just make sure to use
both new controls with overlapping boundaries.


- Yuri


-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message