> On June 20, 2013, 2:13 p.m., Aaron J. Seigo wrote: > > instead of this common idiom over and over (which also introduces the same > > strings repeatedly, which means changes in future need to be made in > > several places): > > > > textIsBright() ? "black" : "yellow" > > > > why not change bool textIsBright to QString defaultBackgroundColor() > > > > this would be more maintainable.
That's what I wanted to do first, but on line 58 in the original code the string "yellow-notes" (or "black-notes") is needed. I couldn't find an easy way to append the "-notes". (using the '+' operator gave me compiler errors) So I chose to do it differently. I can't remember the exact details, but I think I returned const char * instead of QString. Have changed that now. - Wolfgang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110701/#review34750 ----------------------------------------------------------- On June 20, 2013, 1:42 p.m., Wolfgang Bauer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110701/ > ----------------------------------------------------------- > > (Updated June 20, 2013, 1:42 p.m.) > > > Review request for Plasma and Anne-Marie Mahfouf. > > > Description > ------- > > The notes plasmoid by default takes the text color from the current plasma > theme but has "yellow" hardcoded as default background color. > This can lead to unreadable notes by default with certain plasma themes, e.g. > Produkt. > It's especially annoying if you're using such a theme because you can't > globally change the default notes colors but have to do it for each note on > its own (f.e. every time after pasting text to the desktop by pressing the > middle mouse button). > > This patch changes the default background color to black if the text color is > brighter than a certain threshold. > Also the background color is re-read from the config on plasma theme change > to make it change as well if necessary. > > The brightness is determined with QColor::lightness(). > I chose 100 as threshold, because yellow's lightness value is 128 and yellow > on yellow wouldn't be good either... ;-) > I don't know if 100 is the perfect value, but it works as intended with all > the themes I have installed. > > > This addresses bug 320350. > http://bugs.kde.org/show_bug.cgi?id=320350 > > > Diffs > ----- > > applets/notes/notes.cpp 5c2ed70 > > Diff: http://git.reviewboard.kde.org/r/110701/diff/ > > > Testing > ------- > > Created a note on the desktop (with default text and background color) and > switched the plasma theme to each one I have installed. (Air, Air for > Netbooks, Air openSUSE, Androbit, Aya, Produkt, Slim Glow, Tibanna, openSUSE) > Notes were readable with any theme, the background color changed to black > where necessary, was yellow as before otherwise. > > This patch is also part of openSUSE's plasma-addons package since a month ago. > > > File Attachments > ---------------- > > Note in Produkt theme without this patch > > http://git.reviewboard.kde.org/media/uploaded/files/2013/06/20/notes_Produkt_without_patch.png > Note in Produkt theme with this patch > > http://git.reviewboard.kde.org/media/uploaded/files/2013/06/20/notes_Produkt_with_patch.png > > > Thanks, > > Wolfgang Bauer > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
