> On Feb. 27, 2016, 10:27 a.m., Kai Uwe Broulik wrote: > > Nice! > > > > Regarding the ComboBox issues, we know, it's horrible, sizing is broken and > > the popup is always white. Not much we can do here :( > > > > I gave the applet a try and there's a couple of issues: > > > > - It seems Plasma doesn't properly setup the widget. If I drag it to the > > desktop it works but if I double click to add it it says "Error loading > > widget: Package does not exist", same when I add it to system tray. > > - It doesn't properly load a new weather station, the busy indicator just > > spins forever; I need to restart Plasma for the weather to show up. > > - The popup size when placed in a panel and the default size on the desktop > > is way too small > > - The plasmoid is just empty when it is not configured; Plasmoid has a > > configurationRequired property which shows a "You need to configure this > > thing first [Configure]" > > - It sometimes seems to forget the weather station when I open settings and > > when I then save settings the applet is blank again > > - Sometimes it doesn't find any weather stations in the search > > - The tooltip subtext is blank (but not empty, ie. I get an additional > > empty line) > > - The strip with the temperatures needs to be taller, text barely fits in > > there. The weather icons also leak outside the strip but this looks like it > > might be intended. > > > > It also looks like we need some Breeze weather icons :) > > Friedrich W. H. Kossebau wrote: > Thanks a lot for your detailed review and testing, very appreciated! > > > if I double click to add it it says "Error loading widget: Package does > not exist", same when I add it to system tray. > > Confirmed. Same issue here also with the comic applet. Perhaps something > related to being cpp-backed applets? Shall I file a bug so it can be tracked? > At least I assume nothing I can fix in the applet itself. > > > It doesn't properly load a new weather station, the busy indicator just > spins forever; I need to restart Plasma for the weather to show up. > > Can reproduce, at least the busy indicator on the compact applet, still > investigating how the busy flag is set to wrong value. But here the config > itself works, the new weather station is used and data shown as expected. > Just the busy indicator stays. > So still ISSUE. > > > The popup size when placed in a panel and the default size on the > desktop is way too small > > Yes. Seems there are binding issues with the width & height values, I am > still lost what the Plasma5 approach is and need to investigate more. > So still ISSUE. > > > The plasmoid is just empty when it is not configured; Plasmoid has a > configurationRequired property which shows a "You need to configure this > thing first [Configure]" > > The C++-code (for the base class) uses > `Applet::setConfigurationRequired(false)`, is that no longer working or not > enough? > > > It sometimes seems to forget the weather station when I open settings > and when I then save settings the applet is blank again > > I have not seen that, any chance you can tell how to reproduce? > > > Sometimes it doesn't find any weather stations in the search > > Wetter.com might sometimes tack a few secs, though usually replies > instantly. The service behind "noaa" data engine though seems to > Needs perhaps inspection and fixing in the weather dataengines (in > plasma-workspace/dataengines/weather/. The code/logic in the weather applet I > mainly moved around (from old WeatherConfig class to LocationListModel), > still possible I messed something up. > > > The tooltip subtext is blank (but not empty, ie. I get an additional > empty line) > > Fixed, catching the case where "%1 %2" is filled with empty strings due > to no data. > > > The strip with the temperatures needs to be taller, text barely fits in > there. The weather icons also leak outside the strip but this looks like it > might be intended. > > I think it is by design. The Plasma4 version looked the same. And I do > not want to mess around with the design, that I happily leave to VDG once the > applet itself is ported and works in a first version :) > > Kai Uwe Broulik wrote: > > Shall I file a bug so it can be tracked? > > Yes, please, indeed looks like a bug with native applets, perhaps the > setup applet scripts or so aren't run properly. Marco? > > > is that no longer working or not enough? > > Can you try from QML? Plasmoid.configurationRequired: true; perhaps it > was broken/never ported to the new QML containment as we haven't had a > plasmoid so far in Plasma 5 that actually used this; if it doesn't, file a > bug, please. > > > Seems there are binding issues with the width & height values > > Try with Layout.minimumWidth and/or preferredWidth. > > > I think it is by design. > > For the weather icons that's fine but I have a high dpi screen and thus > large fonts and so the temperature barely fits into the strip whereas on your > screenshot it looks fine. Try setting the height to > theme.mSize(yourlabel.font) (or so) > > Friedrich W. H. Kossebau wrote: > >> Shall I file a bug so it can be tracked? > > > Yes, please > > Done, https://bugs.kde.org/show_bug.cgi?id=359902 > > > Can you try from QML? Plasmoid.configurationRequired: true; > > Results in applet not being loaded due to error on loading QML file: > Cannot assign to non-existent property "configurationRequired" > > Also no real hits on > https://lxr.kde.org/search?_filestring=&_string=configurationRequired > > Filed as minor bug in https://bugs.kde.org/show_bug.cgi?id=359906 > > > Try with Layout.minimumWidth and/or preferredWidth. > > Tried with that, seems things are working slightly better now. > > > Try setting the height to theme.mSize(yourlabel.font) (or so) > > Tried something with that, please test. > > Marco Martin wrote: > >>if I double click to add it it says "Error loading widget: Package does > not exist", same when I add it to system tray. > >Confirmed. Same issue here also with the comic applet. Perhaps something > related to being cpp-backed applets? Shall I file a bug so it can be tracked? > At least I assume nothing I can fix in the applet itself. > > I actually can't reproduce, the comic applet loads just fine here with > double click from widget explorer > > Marco Martin wrote: > updated plasma workspace and now i can reproduce the problem. > so it's a problem on the widget explorer side, introduced recently > > Marco Martin wrote: > caused by commit a4c6151e41d0c282495a34785c3aca1893a91828 in > plasma-workspace > > Marco Martin wrote: > bug fixed. > > Friedrich W. H. Kossebau wrote: > Jay, thanks for the quick fix :) Any chance you can port this to 5.5 > today, so it's part of 5.5.5 as well?
no, it's a commit to the framework, doesn't have a 5.5 branch, but updating frameworks on a 5.5 installation should go - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127193/#review92824 ----------------------------------------------------------- On Feb. 29, 2016, 2:23 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. 29, 2016, 2:23 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/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
