Review Request 127193: Revival of Weather applet (for Plasma 5.6)

Marco Martin notmart at gmail.com
Sat Feb 27 17:19:12 UTC 2016


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127193/#review92836
-----------------------------------------------------------



>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);
>        }

right, i noted that in the past as well.
you'll have to do this was for now, i'll reintroduce again the busy property in Applet

> Q: Plasma::Applet::showMessage() can be ported to what?

for what you need it for? should perhaps send a notification instead?

> 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);

you can't do synchronous queries anymore,  should be found a way to make it anync (i could perhaps introduce a new query method that instead of being sync as it was the old one, uses a lambda as a callback, would that work?

- Marco Martin


On Feb. 27, 2016, 1:18 a.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127193/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2016, 1:18 a.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/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/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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160227/44980400/attachment-0001.html>


More information about the Plasma-devel mailing list