incubator-wave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Denis Konovalchik" <dyu...@gmail.com>
Subject Re: Review Request 25230: Text marked as deleted keeps the style properties of the source text.
Date Wed, 03 Sep 2014 09:56:48 GMT


> On Сен. 2, 2014, 6:26 п.п., Andrew Kaplanov wrote:
> > src/org/waveprotocol/wave/client/editor/content/DiffHighlightingFilter.java, line
351
> > <https://reviews.apache.org/r/25230/diff/2/?file=673782#file673782line351>
> >
> >     This case for not formatted text?

Rather, it's for the text having the same formatting on whole its length. (Not formatted text
can be such a case, too.)


> On Сен. 2, 2014, 6:26 п.п., Andrew Kaplanov wrote:
> > src/org/waveprotocol/wave/client/editor/content/misc/StyleAnnotationHandler.java,
line 94
> > <https://reviews.apache.org/r/25230/diff/2/?file=673783#file673783line94>
> >
> >     Can we place KEYS to AnnotationConstants.STYLE_KEYS?

Sure. Maybe, AnnotationConstants.TEXT_STYLE_KEYS would be a better name?
Accordingly this, the constant keeping annotation keys for deleted text style could be renamed
into AnnotationConstants.DELETED_TEXT_STYLE_KEYS.


> On Сен. 2, 2014, 6:26 п.п., Andrew Kaplanov wrote:
> > src/org/waveprotocol/wave/model/conversation/AnnotationConstants.java, line 51
> > <https://reviews.apache.org/r/25230/diff/2/?file=673784#file673784line51>
> >
> >     DELETED_TEXT_STYLE -> STYLES_OF_DELETED_TEXT

As I stated above, the names could be as follows:
annotation keys for text style => AnnotationConstants.TEXT_STYLE_KEYS,
annotation keys for deleted text style => AnnotationConstants.DELETED_TEXT_STYLE_KEYS.


> On Сен. 2, 2014, 6:26 п.п., Andrew Kaplanov wrote:
> > src/org/waveprotocol/wave/model/conversation/AnnotationConstants.java, line 54
> > <https://reviews.apache.org/r/25230/diff/2/?file=673784#file673784line54>
> >
> >     Should we to place here also AnnotationConstants.STYLE_TEXT_DECORATION and AnnotationConstants.STYLE_VERTICAL_ALIGN?

Yes, we can place here STYLE_VERTICAL_ALIGN.
But STYLE_TEXT_DECORATION shouldn't be here because accordingly to CSS, spans with deleted
text have "text-decoration: line-through" attribute and it shouldn't be overriden by text's
own decoration attribute value.


> On Сен. 2, 2014, 6:26 п.п., Andrew Kaplanov wrote:
> > src/org/waveprotocol/wave/client/editor/content/DiffHighlightingFilter.java, line
348
> > <https://reviews.apache.org/r/25230/diff/2/?file=673782#file673782line348>
> >
> >     Can we to pass ai.annotations() to createDeleteElement() to avoid rescanning?

Yes, I tried to avoid this.
Now I used iterator of RangedAnnotation. It contains not only start() and end(), but key()
and value() as well, so you can avoid repetitious scanning for getting of anotation value.


- Denis


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


On Сен. 2, 2014, 9:55 д.п., Denis Konovalchik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25230/
> -----------------------------------------------------------
> 
> (Updated Сен. 2, 2014, 9:55 д.п.)
> 
> 
> Review request for wave, Andrew Kaplanov and Yuri Zelikov.
> 
> 
> Repository: wave
> 
> 
> Description
> -------
> 
> At present moment the text marked as deleted within blip is displayed with standard style,
and any formatting of it made before is lost. This patch is designed to display deleted text
with the same formatting as it had before deletion.
> If it's necessary, instead of one span keeping deleted text with standard style several
spans are created, each of them has its own style attributes (color, fontFamily, fontSize,
fontStyle and fontWeight). Because attributes "backgroundColor" (light red) and "textDecoration"
(strike through) for any deleted text are defined by css, they are not copied from the source
text formatting.
> 
> 
> Diffs
> -----
> 
>   src/org/waveprotocol/wave/client/editor/content/DiffHighlightingFilter.java 30b4af2

>   src/org/waveprotocol/wave/client/editor/content/misc/StyleAnnotationHandler.java f0a68f8

>   src/org/waveprotocol/wave/model/conversation/AnnotationConstants.java 3fe9284 
> 
> Diff: https://reviews.apache.org/r/25230/diff/
> 
> 
> Testing
> -------
> 
> Create blip with text and apply to it different formatting attributes (make some parts
of it italic, bold, some different font family, size and color). Then delete this text by
another user, and check that source formatting in deleted area is the same as it was before
deletion.
> 
> 
> Thanks,
> 
> Denis Konovalchik
> 
>


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