Review Request: Notes plasmoid: change text color before any text, new color is now honored

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/679/#review1098
-----------------------------------------------------------



/trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp
<http://reviewboard.kde.org/r/679/#comment720>

    probably more efficient than calling toPlainText constantly is to use the QTextDocument in QTextEdit:
    
    if (m_textEdit->nativeWidget()->document()->characterCount() == 1)
    
    hmm.. and maybe this should actually be in lineChanged? that method looks a bit suspect as well.. *sigh*



/trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp
<http://reviewboard.kde.org/r/679/#comment719>

    couldn this just move into delayedSaveNote() directly? that would get rid of the start(1) on the timer ...
    
    still, it seems odd that the entire text needs to be selected and the color set on it on every change. does it only need to be done on the first character (would sort of make sense: there's no character to actually color until there is at least one).
    
    i'd expect this to just be in delayedSaveNote and only done on the first character.


- Aaron


On 2009-05-09 04:47:06, Anne-Marie Mahfouf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/679/
> -----------------------------------------------------------
> 
> (Updated 2009-05-09 04:47:06)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> The upstream Qt bug is fixed but it did not fix the Notes applet. This patch makes a change of color before any text honored. Also when you remove all text and change color, the new text gets the new color. A bit ugly as it messes with QTimer but I don't see another way around this.
> Please check it: before the patch, set a Notes applet, change text color, write text: text is black. After patch: text is the new color.
> 
> 
> This addresses bug 176266.
>     https://bugs.kde.org/show_bug.cgi?id=176266
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp 965525 
> 
> Diff: http://reviewboard.kde.org/r/679/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Anne-Marie
> 
>