----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127193/#review92991 -----------------------------------------------------------
I am now satisfied with the current state and would like to propose it for merge into master. >From what I saw there is one issue still left: sometimes after changing the >config there is no immediate data update coming to the applet. As the applet >resets its UI to empty by default (there is no different handling of weather >station change and unit changes), this then results in an empty applet, until >the next data is delivered (also results in the busy indicator on the applet, >as it waits for the data and times out only after 2min). But this is old logic of the code, so not a regression, at least in the applet (I have yet to see why the data is not pushed to the applet). Thus I would argue this should not be a showstopper for the merge. Will look into fixing this anyway, i.e. split adapting to config changes. But for 5.6 I would like to concentrate on getting more weather ion plugins ported still today. So please give this applet another test and consider getting it in for 5.6, I surely am not the only one missing it a lot :) - Friedrich W. H. Kossebau On March 1, 2016, 12:43 p.m., Friedrich W. H. Kossebau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127193/ > ----------------------------------------------------------- > > (Updated March 1, 2016, 12:43 p.m.) > > > Review request for Plasma and Marco Martin. > > > Repository: kdeplasma-addons > > > Description > ------- > > So the weather applet in branch kossebau/weatherapplet works okayish now > here, both on panel and on containment: > https://share.kde.org/index.php/s/MxwDYtmRw9TtQ8X > > This revival is done as minimal porting, after more considerations I for now > kept libplasmaweather as it is where possible (also because I use classes > from the lib in the QML plugin and in the applet, so some shared unit is > needed). > I propose to do further modernizing & refactoring of the applet code only > once it works again completely and can be part of 5.6 release. > > Please help with a quick review. And also with a few issues that still need > to be solved for a perfect solution. Where I need your, the experts', help, > please read on. > > Layout: > `configGeneral.qml` needs larger minimal sizes of the input fields. How can > this be done? I am lost with the QQ2 layout approach for now. > > Size of applet in fullRepresentation mode when expanded from panel: > while the expanded applet had some kind of sane size all the time I was > playing (cmp. the screenshot), at one point the size of the expanded > representation only turned into some unusable 16x16 or similar size. For > existing applets and new ones. > > > Q: Is this ported correctly (a hack I found elsewhere), or is there a better > way meanwhile? > ``` > q->setBusy(false); > ``` > to > ``` > QObject *graphicObject = > q->property("_plasma_graphicObject").value<QObject *>(); > if (graphicObject) { > graphicObject->setProperty("busy", false); > } > ``` > > Q: `Plasma::Applet::showMessage()` can be ported to what? > > Q: `DataEngine::query()` should be ported to? Similar code is unported also > in the weather/ions/ion_noaa dataengine in plasma-workspace. > > ``` > Plasma::DataEngine::Data data = timeEngine->query( > QString(QLatin1String( > "Local|Solar|Latitude=%1|Longitude=%2" )).arg(latitude).arg(longitude)); > bool day = data[QLatin1String( "Corrected Elevation" )].toDouble() > > 0.0); > ``` > > > Diffs > ----- > > CMakeLists.txt 5568251 > applets/CMakeLists.txt bd3ea48 > applets/weather/CMakeLists.txt 354b6ee > applets/weather/package/contents/config/config.qml PRE-CREATION > applets/weather/package/contents/ui/DetailsView.qml bbcdd15 > applets/weather/package/contents/ui/FiveDaysView.qml a39f334 > applets/weather/package/contents/ui/Notice.qml d38c108 > applets/weather/package/contents/ui/NoticesView.qml 16923c4 > applets/weather/package/contents/ui/TopPanel.qml af57f45 > applets/weather/package/contents/ui/Utils.js f2d5b27 > applets/weather/package/contents/ui/WeatherListView.qml b29099f > applets/weather/package/contents/ui/configGeneral.qml PRE-CREATION > applets/weather/package/contents/ui/main.qml c501ab3 > applets/weather/package/metadata.desktop 79646c2 > applets/weather/plasma-applet-weather.desktop aca6da2 > applets/weather/plugin/abstractunitlistmodel.h PRE-CREATION > applets/weather/plugin/abstractunitlistmodel.cpp PRE-CREATION > applets/weather/plugin/locationlistmodel.h PRE-CREATION > applets/weather/plugin/locationlistmodel.cpp PRE-CREATION > applets/weather/plugin/plugin.h PRE-CREATION > applets/weather/plugin/plugin.cpp PRE-CREATION > applets/weather/plugin/qmldir PRE-CREATION > applets/weather/weatherapplet.h c4b376b > applets/weather/weatherapplet.cpp 60f882a > libs/CMakeLists.txt bd71119 > libs/plasmaweather/CMakeLists.txt a9faa7b > libs/plasmaweather/plasmaweather.knsrc 8525f20 > libs/plasmaweather/plasmaweather_export.h 691db23 > libs/plasmaweather/weatherconfig.h 9b3c2d7 > libs/plasmaweather/weatherconfig.cpp 1ec0b42 > libs/plasmaweather/weatherconfig.ui f285fff > libs/plasmaweather/weatheri18ncatalog.h 0122378 > libs/plasmaweather/weatheri18ncatalog.cpp 1868352 > libs/plasmaweather/weatherlocation.h 004d788 > libs/plasmaweather/weatherlocation.cpp 1d275ea > libs/plasmaweather/weatherpopupapplet.h ce95a5a > libs/plasmaweather/weatherpopupapplet.cpp 4533619 > libs/plasmaweather/weathervalidator.h eb33558 > libs/plasmaweather/weathervalidator.cpp 4d016e2 > > Diff: https://git.reviewboard.kde.org/r/127193/diff/ > > > Testing > ------- > > > Thanks, > > Friedrich W. H. Kossebau > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel