----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/366/#review527 -----------------------------------------------------------
The patch looks good overall, but there are some issues with it that need fixing before it can be committed. trunk/KDE/kdeplasma-addons/applets/notes/config.ui <http://reviewboard.kde.org/r/366/#comment313> Don't put all the fonts in here, the list should be filled automatically. It's probably done by designer, but needs cleaning up nevertheless trunk/KDE/kdeplasma-addons/applets/notes/config.ui <http://reviewboard.kde.org/r/366/#comment314> please post a screenshot when you do UI changes trunk/KDE/kdeplasma-addons/applets/notes/notes.h <http://reviewboard.kde.org/r/366/#comment315> leaved is no proper english, left is, probably cursorLeft() or mouseUnhovered() make more sense as names (the latter showing clearly that it's not about left/right) trunk/KDE/kdeplasma-addons/applets/notes/notes.h <http://reviewboard.kde.org/r/366/#comment316> separate lines for separate vars please trunk/KDE/kdeplasma-addons/applets/notes/notes.h <http://reviewboard.kde.org/r/366/#comment317> Does this even compile? It seems to be missing the :: trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp <http://reviewboard.kde.org/r/366/#comment318> textBackgroundColor, to be consistent with other options in the code (camel case) trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp <http://reviewboard.kde.org/r/366/#comment319> please use 4 spaces for indenting trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp <http://reviewboard.kde.org/r/366/#comment320> hardcoded colors are evil, try to avoid them - Sebastian On 2009-03-20 03:00:54, Zareth wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/366/ > ----------------------------------------------------------- > > (Updated 2009-03-20 03:00:54) > > > Review request for Plasma. > > > Summary > ------- > > This patch add to the plasmoid notes the possibility to choose a background > color > different from the general background of the Notes plasmoid for the line that > is currently edited. > > The idea came from a bug track concerning the bad view of the Notes when we > choose a black > background color because we can't see the cursor. This option is an > alternative to the problem. > > If you notice any problems with this new option just tell me > > > Diffs > ----- > > trunk/KDE/kdeplasma-addons/applets/notes/config.ui 941658 > trunk/KDE/kdeplasma-addons/applets/notes/notes.h 941658 > trunk/KDE/kdeplasma-addons/applets/notes/notes.cpp 941658 > > Diff: http://reviewboard.kde.org/r/366/diff > > > Testing > ------- > > > Thanks, > > Zareth > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
