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