> On Aug. 26, 2012, 9:36 p.m., David Edmundson wrote: > > applets/weather/package/contents/ui/Notice.qml, line 27 > > <http://git.reviewboard.kde.org/r/106225/diff/1/?file=81341#file81341line27> > > > > Unless there's a good reason, use PlasmaComponents.Label rather than > > Text. Otherwise font size won't be consistent. > > > > Saves you setting the theme colour yourself too :) > > > > Same for all instances of Text > > Luis Gabriel Lima wrote: > The C++ version uses the QApplication::font() instead of the plasma theme > font. So after notice the difference when I was trying the > PlasmaComponents.Label, I decided to keep the same way as the C++ version. > > David Edmundson wrote: > That's sort of a good reason. > > Though I would argue that if the C++ one is wrong, there's no need to > copy it. > > Luis Gabriel Lima wrote: > I don't know if the C++ version is wrong, actually all the three plasmoid > that I've ported was using QApplication::font().
It's still (IMHO) rather wrong. For example if a user had bad eyesight and increases their plasma font size in the Workspace Appearance KCM, you'd expect it to actually work. However, as it's not a regression so I'm not calling it a showstopper to merging. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106225/#review18054 ----------------------------------------------------------- On Aug. 30, 2012, 1:53 a.m., Luis Gabriel Lima wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106225/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2012, 1:53 a.m.) > > > Review request for Plasma and Marco Martin. > > > Description > ------- > > This patch is part of the work done in my GSoC 2012 project. > > > Diffs > ----- > > applets/weather/CMakeLists.txt 4ae9c6f > applets/weather/package/contents/ui/DetailsView.qml PRE-CREATION > applets/weather/package/contents/ui/FiveDaysView.qml PRE-CREATION > applets/weather/package/contents/ui/Notice.qml PRE-CREATION > applets/weather/package/contents/ui/NoticesView.qml PRE-CREATION > applets/weather/package/contents/ui/TopPanel.qml PRE-CREATION > applets/weather/package/contents/ui/Utils.js PRE-CREATION > applets/weather/package/contents/ui/WeatherListView.qml PRE-CREATION > applets/weather/package/contents/ui/main.qml PRE-CREATION > applets/weather/package/metadata.desktop PRE-CREATION > applets/weather/weatherapplet.h a7e7f8d > applets/weather/weatherapplet.cpp 9fd3e5f > applets/weather/weatherdelegate.h 8a27f0d > applets/weather/weatherdelegate.cpp 1cd987b > applets/weather/weatherview.h f177501 > applets/weather/weatherview.cpp 5234cd3 > > Diff: http://git.reviewboard.kde.org/r/106225/diff/ > > > Testing > ------- > > - Tested inside panels and floating on desktop > - Tested with various providers and locations > > > Screenshots > ----------- > > > http://git.reviewboard.kde.org/r/106225/s/696/ > > http://git.reviewboard.kde.org/r/106225/s/698/ > > http://git.reviewboard.kde.org/r/106225/s/699/ > > http://git.reviewboard.kde.org/r/106225/s/700/ > > > Thanks, > > Luis Gabriel Lima > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel