Review Request: Patch to add a 7-day (up-to) forecast to the NOAA weather Ion
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1584/ --- Review request for Plasma and Shawn Starr. Summary --- The patch uses the REST API provided by NOAA at http://www.weather.gov/forecasts/xml/rest.php to obtain the forecast for the location coordinates returned by the observation data. It requests the 7-day forecast but only displays the days for which data is available (usually 5-7 days). The weather icon descriptions unfortunately don't match those used by the normal observation feed, and are defined at http://www.weather.gov/forecasts/xml/DWMLgen/schema/parameters.xsd ; we however use a similar heuristic algorithm (keyword search)) to find the best match for the returned weather conditions. This patch also enables the normal condition icon which was being mistakenly being unset. Diffs - trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_noaa.cpp 1021754 trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_noaa.h 1016575 Diff: http://reviewboard.kde.org/r/1584/diff Testing --- Verified the NOAA forecast was displayed in the Weather Forecast applet for a few locations. Thanks, Amos ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: big revamp of Device Notifier
On 2009-09-11 19:44:22, Aaron Seigo wrote: is the latest version the most recent available version of the patch? because it doesn't apply cleanly to trunk. anyways ... i really don't like the how long to show the popup configuration item and the other 2 config items do need seem linguistic and layout adjustments, but that's easy to do once this is in. how long could remain configurable in the config file, but i don't think it belongs in the UI; that's just an excuse not to make a good default. having to click twice for items with one action is not great. instead, when there is just one item how about just putting the action description as the sub-title text for the main item and launch it on click/select? so it would be: ICON Camera ICON Open with Digikam... in the case of multiple items, it could look like: ICON USB Storage Device ICON 2 available actions ... and when it's clicked then the options show up. it could happen on hover, but i'm not sure that makes sense in this case. if the options could also be made smaller that would also help show that these are detail items that belong to the item above better IMO; the icon should probably only need to be roughly as tall as the line of text? The problem is that the subtitle could be covered by the KCapacityBar. So we need to find a solution, as to show the actions on hover. On 2009-09-11 19:44:22, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 366 http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366 this is only needed if m_showOnlyRemovable changes, correct? it is used also to hide or show the devices when you (un)check the option show all the items in the context menu. - Giulio --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298 --- On 2009-09-11 21:32:17, Giulio Camuffo wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/ --- (Updated 2009-09-11 21:32:17) Review request for Plasma. Summary --- This is a patch that modifies quite heavily the behaviour of the Device Notifier. It comes from here: http://kde-look.org/content/show.php/Device+Manager?content=106051 It can show the not removable devices too, it can mount them automatically or with a click, since the eject button is a mount button when the volume is umounted. So that guy on the dot will be ok. It can hide some items in the same way as Dolphin's places (hide item/ show all). Finally, it shows the various opening actions under the device instead of calling that xp-ish window. Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui PRE-CREATION /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp 1022457 Diff: http://reviewboard.kde.org/r/1370/diff Testing --- I'm using it every day since I released 0.1 on Kde-look. I tried all the options on my pc and they work. Some people on kde-look posted some comments about some problems, but it seems to me they are very particular cases, so in my opinion it is quite stable to go in trunk, but anyway review it! :) Screenshots --- screen http://reviewboard.kde.org/r/1370/s/183/ Thanks, Giulio ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: big revamp of Device Notifier
On 2009-09-11 19:44:22, Aaron Seigo wrote: is the latest version the most recent available version of the patch? because it doesn't apply cleanly to trunk. anyways ... i really don't like the how long to show the popup configuration item and the other 2 config items do need seem linguistic and layout adjustments, but that's easy to do once this is in. how long could remain configurable in the config file, but i don't think it belongs in the UI; that's just an excuse not to make a good default. having to click twice for items with one action is not great. instead, when there is just one item how about just putting the action description as the sub-title text for the main item and launch it on click/select? so it would be: ICON Camera ICON Open with Digikam... in the case of multiple items, it could look like: ICON USB Storage Device ICON 2 available actions ... and when it's clicked then the options show up. it could happen on hover, but i'm not sure that makes sense in this case. if the options could also be made smaller that would also help show that these are detail items that belong to the item above better IMO; the icon should probably only need to be roughly as tall as the line of text? Giulio Camuffo wrote: The problem is that the subtitle could be covered by the KCapacityBar. So we need to find a solution, as to show the actions on hover. the capacity bar should be painted below any text. On 2009-09-11 19:44:22, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 366 http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366 this is only needed if m_showOnlyRemovable changes, correct? wrote: it is used also to hide or show the devices when you (un)check the option show all the items in the context menu. yes, that's what i said :) - Aaron --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298 --- On 2009-09-11 21:32:17, Giulio Camuffo wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/ --- (Updated 2009-09-11 21:32:17) Review request for Plasma. Summary --- This is a patch that modifies quite heavily the behaviour of the Device Notifier. It comes from here: http://kde-look.org/content/show.php/Device+Manager?content=106051 It can show the not removable devices too, it can mount them automatically or with a click, since the eject button is a mount button when the volume is umounted. So that guy on the dot will be ok. It can hide some items in the same way as Dolphin's places (hide item/ show all). Finally, it shows the various opening actions under the device instead of calling that xp-ish window. Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui PRE-CREATION /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp 1022457 Diff: http://reviewboard.kde.org/r/1370/diff Testing --- I'm using it every day since I released 0.1 on Kde-look. I tried all the options on my pc and they work. Some people on kde-look posted some comments about some problems, but it seems to me they are very particular cases, so in my opinion it is quite stable to go in trunk, but anyway review it! :) Screenshots --- screen http://reviewboard.kde.org/r/1370/s/183/ Thanks, Giulio ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: big revamp of Device Notifier
On 2009-09-11 19:44:22, Aaron Seigo wrote: is the latest version the most recent available version of the patch? because it doesn't apply cleanly to trunk. anyways ... i really don't like the how long to show the popup configuration item and the other 2 config items do need seem linguistic and layout adjustments, but that's easy to do once this is in. how long could remain configurable in the config file, but i don't think it belongs in the UI; that's just an excuse not to make a good default. having to click twice for items with one action is not great. instead, when there is just one item how about just putting the action description as the sub-title text for the main item and launch it on click/select? so it would be: ICON Camera ICON Open with Digikam... in the case of multiple items, it could look like: ICON USB Storage Device ICON 2 available actions ... and when it's clicked then the options show up. it could happen on hover, but i'm not sure that makes sense in this case. if the options could also be made smaller that would also help show that these are detail items that belong to the item above better IMO; the icon should probably only need to be roughly as tall as the line of text? Giulio Camuffo wrote: The problem is that the subtitle could be covered by the KCapacityBar. So we need to find a solution, as to show the actions on hover. Aaron Seigo wrote: the capacity bar should be painted below any text. that would solve all, but personally i dislike very much from an aesthetically pov the items as they are painted in the current device notifier, with the increased height. i find them fat. and if the volumes are umounted there would be no capacity bar, and that would result in a lot of wasted white space On 2009-09-11 19:44:22, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 366 http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366 this is only needed if m_showOnlyRemovable changes, correct? wrote: it is used also to hide or show the devices when you (un)check the option show all the items in the context menu. wrote: yes, that's what i said :) no, i mean when you right click the dialog, and you set the option to show or not even devices that normally would be hidden too. like when you right click on the Places panel in dolphin. In fact resetDevices() is called in configAccepted() (if m_showOnlyRemovable changes) and in setAllItemsShown(bool shown) (if m_showAll changes). - Giulio --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298 --- On 2009-09-11 21:32:17, Giulio Camuffo wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/ --- (Updated 2009-09-11 21:32:17) Review request for Plasma. Summary --- This is a patch that modifies quite heavily the behaviour of the Device Notifier. It comes from here: http://kde-look.org/content/show.php/Device+Manager?content=106051 It can show the not removable devices too, it can mount them automatically or with a click, since the eject button is a mount button when the volume is umounted. So that guy on the dot will be ok. It can hide some items in the same way as Dolphin's places (hide item/ show all). Finally, it shows the various opening actions under the device instead of calling that xp-ish window. Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui PRE-CREATION /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp 1022457 Diff: http://reviewboard.kde.org/r/1370/diff Testing --- I'm using it every day since I released 0.1 on Kde-look. I tried all the options on my pc and they work. Some people on kde-look posted some comments about
Re: Review Request: Add tooltips to the SystemTray show/icon hidden icons button
On 2009-05-05 15:57:15, Aaron Seigo wrote: this will have to wait for 4.4 now due to the string freeze, but that will also give us time to track down the tooltip issue? :) Beat Wolf wrote: did anybody commit this? so the bugreport can be closed OK, I just retested it and it works properly now. I can't reproduce the issue I described before. I'm going to commit when SVN is up again. - Darío --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/630/#review1069 --- On 2009-04-25 15:31:01, Darío Andrés wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/630/ --- (Updated 2009-04-25 15:31:01) Review request for Plasma. Summary --- This adds a tooltip to the SystemTray icon which hides or shows the hidden icons. It may need better texts. Addresses bug 183953 This addresses bug 183953. https://bugs.kde.org/show_bug.cgi?id=183953 Diffs - svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/taskarea.cpp 958965 Diff: http://reviewboard.kde.org/r/630/diff Testing --- Thanks, Darío ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: big revamp of Device Notifier
On 2009-09-11 19:44:22, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp, line 374 http://reviewboard.kde.org/r/1370/diff/5/?file=10966#file10966line374 KMessageBox blocks all of plasma when used; use Plasma::Applet::showMessage instead. wrote: What about using notifications? (I have a half patch ready) wouldn't it be better to have the notification associated with the widget itself than pop up somewhere disembodied from the widget? that why i suggested Plasma::Applet::showMessage - it keeps it with the applet. i improved it the other day to work nicely in panels too :) - Aaron --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298 --- On 2009-09-11 21:32:17, Giulio Camuffo wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/ --- (Updated 2009-09-11 21:32:17) Review request for Plasma. Summary --- This is a patch that modifies quite heavily the behaviour of the Device Notifier. It comes from here: http://kde-look.org/content/show.php/Device+Manager?content=106051 It can show the not removable devices too, it can mount them automatically or with a click, since the eject button is a mount button when the volume is umounted. So that guy on the dot will be ok. It can hide some items in the same way as Dolphin's places (hide item/ show all). Finally, it shows the various opening actions under the device instead of calling that xp-ish window. Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui PRE-CREATION /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h 1022457 /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp 1022457 Diff: http://reviewboard.kde.org/r/1370/diff Testing --- I'm using it every day since I released 0.1 on Kde-look. I tried all the options on my pc and they work. Some people on kde-look posted some comments about some problems, but it seems to me they are very particular cases, so in my opinion it is quite stable to go in trunk, but anyway review it! :) Screenshots --- screen http://reviewboard.kde.org/r/1370/s/183/ Thanks, Giulio ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
workspace/plasma filesystem reorganization
Hi all, at tokamak we talked about moving stuff around in workspace/plsma, to better distinguish between the desktop shell, the netbook shell and possibly other future things. as i mentioned in an old message, i think a hyerarchy like that would make sense: |-desktop | |-applets | |-containments | `-shells |-netbook | `-the whole hierarchy again there |-common |-wallpapers |-scriptengines |-applets, the ones not shell-specific `-libplasma-workspace libplasma-workspace would be a dynamic lib, private without headers installed and would contain what shells/common has today comments? improvements? as soon as we agree on one i can start making damages :) Cheers Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: workspace/plasma filesystem reorganization
On September 13, 2009, Marco Martin wrote: |-desktop | | |-applets | |-containments | | `-shells should this be shells/ with plasma-desktop/ under it, or should it just be shell/ with the one shell under it? |-netbook | `-the whole hierarchy again there |-common generic? hmm ... not sure whether i like common or generic better.. |-wallpapers |-scriptengines |-applets, the ones not shell-specific shells/ under here as well? for e.g. plasmoidviewer? `-libplasma-workspace libplasma-workspace would be a dynamic lib, private without headers installed and would contain what shells/common has today this should go into workspace/libs/ -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: workspace/plasma filesystem reorganization
On Sunday 13 September 2009, Aaron J. Seigo wrote: On September 13, 2009, Marco Martin wrote: |-desktop | | |-applets | |-containments | | `-shells should this be shells/ with plasma-desktop/ under it, or should it just be shell/ with the one shell under it? woud the screensaver shell go here as well or in common? |-netbook | `-the whole hierarchy again there |-common generic? hmm ... not sure whether i like common or generic better.. don't think tere is much difference, really :) |-wallpapers |-scriptengines |-applets, the ones not shell-specific shells/ under here as well? for e.g. plasmoidviewer? `-libplasma-workspace libplasma-workspace would be a dynamic lib, private without headers installed and would contain what shells/common has today this should go into workspace/libs/ hmm, isn' t it a bit too specific of plasma shells? is really the right place? anyways i'll try to do the lib first and then start to move stuff Cheers, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: workspace/plasma filesystem reorganization
On September 13, 2009, Marco Martin wrote: On Sunday 13 September 2009, Aaron J. Seigo wrote: On September 13, 2009, Marco Martin wrote: |-desktop | | |-applets | |-containments | | `-shells should this be shells/ with plasma-desktop/ under it, or should it just be shell/ with the one shell under it? woud the screensaver shell go here as well or in common? screensaver has its own containment as well. perhaps it should be: plasma/ [common|generic]/ desktop/ netbook/ screensaver/ containments/ shells/ ? |-netbook | `-the whole hierarchy again there |-common generic? hmm ... not sure whether i like common or generic better.. don't think tere is much difference, really :) not much difference, but in this context generic means not specific and common means used or shared by all. these components are really more generic than common i think. libplasma-workspace would be a dynamic lib, private without headers installed and would contain what shells/common has today this should go into workspace/libs/ hmm, isn' t it a bit too specific of plasma shells? is really the right place? like libplasmaclock? ;) i'd prefer to keep all libraries in one place. they have a habit of hiding and getting forgotten otherwise. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: workspace/plasma filesystem reorganization
On Sunday 13 September 2009, Aaron J. Seigo wrote: On September 13, 2009, Marco Martin wrote: On Sunday 13 September 2009, Aaron J. Seigo wrote: On September 13, 2009, Marco Martin wrote: |-desktop | | |-applets | |-containments | | `-shells should this be shells/ with plasma-desktop/ under it, or should it just be shell/ with the one shell under it? woud the screensaver shell go here as well or in common? screensaver has its own containment as well. perhaps it should be: plasma/ [common|generic]/ desktop/ netbook/ screensaver/ containments/ shells/ ? yes, makes sense. i would leave the shells folder even where there is only one tough (for instance in the netbook there is only one now but i will probably add another one in the future) |-netbook | `-the whole hierarchy again there |-common generic? hmm ... not sure whether i like common or generic better.. don't think tere is much difference, really :) not much difference, but in this context generic means not specific and common means used or shared by all. these components are really more generic than common i think. ok, so let's go for generic libplasma-workspace would be a dynamic lib, private without headers installed and would contain what shells/common has today this should go into workspace/libs/ hmm, isn' t it a bit too specific of plasma shells? is really the right place? like libplasmaclock? ;) i'd prefer to keep all libraries in one place. they have a habit of hiding and getting forgotten otherwise. ok. Cheers, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: big revamp of Device Notifier
On 2009-09-11 19:44:22, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 366 http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366 this is only needed if m_showOnlyRemovable changes, correct? wrote: it is used also to hide or show the devices when you (un)check the option show all the items in the context menu. wrote: yes, that's what i said :) wrote: no, i mean when you right click the dialog, and you set the option to show or not even devices that normally would be hidden too. like when you right click on the Places panel in dolphin. In fact resetDevices() is called in configAccepted() (if m_showOnlyRemovable changes) and in setAllItemsShown(bool shown) (if m_showAll changes). wrote: ok, let's back up a bit here. that call to resetDevices is in DeviceNotifier::configAccepted. so i'm talking about when the configuration is changed. that means that in configAccepted, which is what line 365 here is, resetDevices _does not need to be called_ unless m_showOnlyRemovable changes. make sense? now forward again :) looking a m_showAll in NotifierView, Show All devices is always in the context menu and always enabled, even when there is nothing to show. probably, if there are no items to show there doesn't need to be a context menu in the view at all. i'm still not really sure what the real world use cases for hiding/showing individual items in the notifier are other than it's neat that i can; i'm trying to imagine someone who has so many items listed there that it's unmanagable otherwise. or someone who keeps two items in one notifier widget and has a second notifier widget with some others? hm. on the other hand, i can see this resulting in confusion when when someone plugs a device in, marks it to not be shown, then later plugs it in (and hopefully it's impossible to have different devices with the same name?) and it doesn't show up. particularly is days or weeks have passed between those two events. and why would i NOT want my camera to show up when i plug it in? what's the use case for that? this is why i'm still really unsure that turning the device *notifier* into a device *listing manager* in this way is the best of ideas. there's really two similar ideas here it seems: managing devices that are plugged and unplugged (which may or may not be storage devices) and listing storage devices. we actually have a widget for both of those tasks, but the storage device lister doesn't provide access to actions related to the device, which the device notifier does. this really comes back to use cases. can you provide use cases to go with the hide/show individual items feature? oh, and the widget should emit configNeedsSaving() after calls to writeEntry :) ok, i misunderstood a bit what you wanted to say. so, yes. in configAccepted() the call to resetDevices() is needed only if m_showOnlyRemovable changes, so we could actually check if the variable changed value and only in the case call resetDevices() and better yet add or remove the not removable devices without reset all the devices. as regards the show/hide system: saying that Show all devices is always enabled you mean it's always checked? if it is it is definitely a bug in the patch. i don't think the fact that the context menu is shown even when there are no devices is a big deal, but anyway it would be simple to make a check. anyway it uses the uuid provided by solid and not the names to identify the devices, so if you have two devices with the same name and you hide one, the other will remain visible (unless the uuid are equals, but i think this is a really rare case, if it is possible). Actually i decided to develop this feature only because a user on kde-look said to me that the applet was of no use for him because he had lots of devices and, unless he was able to hide some of them, the plasmoid was unusable. (comment #7 on http://kde-look.org/content/show.php?content=106051forumpage=2). - Giulio --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298 --- On 2009-09-11 21:32:17, Giulio Camuffo wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/ --- (Updated 2009-09-11 21:32:17) Review request for Plasma. Summary --- This is a patch that modifies quite heavily the behaviour of the Device Notifier. It comes from here: http://kde-look.org/content/show.php/Device+Manager?content=106051 It can show the
Re: Review Request: Use sunrise and sunset times from the Environment Canada weather service to set weather icon to day/night, accordingly.
On 2009-05-03 16:42:23, Shawn Starr wrote: It seems to be working so far, it's already committed. i can't find this code in svn, is it really commited? can somebody mark this as submitted? - Beat --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/665/#review1045 --- On 2009-05-03 16:05:40, Andrew Coles wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/665/ --- (Updated 2009-05-03 16:05:40) Review request for Plasma. Summary --- Previously, day began at 6am, and night began at 6pm. However, as summer approaches, this is increasingly far from the truth. Hence, the real sunrise/sunset times are now used to determine whether it is day or night, and the weather icon set appropriately. This also fixes issues with the weather wallpaper picking night-time images during the day. Diffs - /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_envcan.cpp 963098 Diff: http://reviewboard.kde.org/r/665/diff Testing --- I need someone who uses this weather service to test it for me. Thanks, Andrew ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Makes the configuration of the comic plasmoid easier to use
On 2009-03-29 05:27:41, Marco Martin wrote: looks good. i would skip the select all button for now this is submitted, can somebody mark it as that? - Beat --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/461/#review685 --- On 2009-04-07 06:09:38, Matthias Fuchs wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/461/ --- (Updated 2009-04-07 06:09:38) Review request for Plasma and Davide Bettio. Summary --- Whenever the user ticks Use Tabs the view changes and the available comics are displayed in a listview, clicking at them marks them and adds them as tabs if ok is pressed. Otherwise it is similar to the current situation. I'm not sure if I should add Select All-, Deselect All-Buttons. What do you think? Diffs - /trunk/KDE/kdeplasma-addons/applets/comic/appearanceSettings.ui 950532 /trunk/KDE/kdeplasma-addons/applets/comic/comic.h 950532 /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp 950532 /trunk/KDE/kdeplasma-addons/applets/comic/comicSettings.ui 950532 /trunk/KDE/kdeplasma-addons/applets/comic/configwidget.h 950532 /trunk/KDE/kdeplasma-addons/applets/comic/configwidget.cpp 950532 Diff: http://reviewboard.kde.org/r/461/diff Testing --- Screenshots --- http://reviewboard.kde.org/r/461/s/99/ http://reviewboard.kde.org/r/461/s/100/ http://reviewboard.kde.org/r/461/s/102/ Thanks, Matthias ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Fix the 0600GMT=sunrise, 1800GMT=sunset bug in the BBC Weather Ion
On 2009-05-15 16:49:12, Shawn Starr wrote: For the BBC ion, It looks fine. But I think maybe we should create a general purpose library for all of these things instead of using the dataengines? Seems we should have a libplasma-utils.so library that contains all sorts of reusable functions? Otherwise SHIP IT! i think this patch was never commited, anyboddy knows what happend? - Beat --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/684/#review1140 --- On 2009-05-15 15:59:05, Andrew Coles wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/684/ --- (Updated 2009-05-15 15:59:05) Review request for Plasma. Summary --- To give an icon depicting the current weather, the BBC UK Met. Office backend decides whether or not it is day or night using a simple rule: if it's after 0600GMT but before 1800GMT, it is daytime; otherwise, it's night time. This causes two known issues: i) For people in the UK, the weather icons indicate night even though sunset is not until well after 2000GMT at this time of year ii) For people outside the UK, day and night are set to an approximation of what they are in the UK. So, for instance, for people in New York, the weather icon goes to night at lunchtime. The attached diff fixes this bug by calculating the actual sunrise/sunset times for the weather location, and using /these/ to decide whether it's day or night. How is this done? Whilst the BBC don't provide sunrise/sunset time in the current observations, they do provide latitude and longitude information. Given we know: i) The date ii) The latitude/longitude ...we can then calculate sunrise and sunset times using a bit of maths. If you look, the diffs for the ion itself are very small: just enough to handle the lat/long information, and then calls to sunrise/sunset calculation to decide whether it's day or night. This code has been reused from the time DataEngine - so we're not importing lots of new code at this point. Diffs - /trunk/KDE/kdebase/workspace/plasma/dataengines/time/CMakeLists.txt 968518 /trunk/KDE/kdebase/workspace/plasma/dataengines/time/solarposition.h 968518 /trunk/KDE/kdebase/workspace/plasma/dataengines/time/time_solar_export.h PRE-CREATION /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/CMakeLists.txt 968518 /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_bbcukmet.h 968518 /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_bbcukmet.cpp 968518 Diff: http://reviewboard.kde.org/r/684/diff Testing --- I use the BBC weather ion for my weather data, so I tested it for my current location (fine) and a few others from around the world (also fine). Has been fine for well over a week, becoming increasingly more useful as sunset pushes towards 10pm. Thanks, Andrew ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Patch to support Plasma::PopupApplets in scriptengines
On 2009-07-14 20:51:10, Marco Martin wrote: this actually changes the behaviour of popupapplet, so now is possible to actually set a widget besides reimplementing graphicsWidget() hmm... i think it should have been this way since the beginning (the api of popupapplet is a ctually a bit... sad, but well..) and i like this change, even for c++ applets too.. however, since is still possible to reimplement graphicswidget, this makes setGraphicsWidget useless in those cases, but i guess it depends from the brokeness of popupapplet api in first place, so let's go for it i would say.. was this patch ever shipped? please mark as submitted if yes - Beat --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/964/#review1598 --- On 2009-07-14 19:23:26, Richard Dale wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/964/ --- (Updated 2009-07-14 19:23:26) Review request for Plasma. Summary --- Add getters and setters for PopupApplet::widget() and PopupApplet::graphicsWidget() for use in scripting languages. Add a initScriptingExtenderItem() signal for script engines to connect to and call their versions of Applet::initExtenderItem() Diffs - trunk/KDE/kdelibs/plasma/applet.h 996721 trunk/KDE/kdelibs/plasma/applet.cpp 996702 trunk/KDE/kdelibs/plasma/popupapplet.h 996702 trunk/KDE/kdelibs/plasma/popupapplet.cpp 996702 trunk/KDE/kdelibs/plasma/private/popupapplet_p.h 996702 Diff: http://reviewboard.kde.org/r/964/diff Testing --- Thanks, Richard ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Quicklaunch: Fix bugs related to having unlimited visible icons
On 2009-08-02 23:51:37, Aaron Seigo wrote: looks fine; perhaps all the instances of -1 should be replaced with a `static const int UNLIMITED_ICONS = -1` in the header file? :) Shafqat Bhuiyan wrote: Ok I added the UNLIMITED_ICONS = -1 :) PS. Could you please commit this because my SVN account still isn't working :S Aaron Seigo wrote: hm.. i thought i saw the update to your account the other day? there's no rush on these fixes at this point (4.3.0 is already tagged, so this will end up in 4.3.1 and 4.4 only), maybe you could find out what's up with your svn account? find one of us on irc maybe and we can try and figure out what's up... is there any news on the svn account or the patch? - Beat --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1220/#review1885 --- On 2009-08-03 00:17:24, Shafqat Bhuiyan wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1220/ --- (Updated 2009-08-03 00:17:24) Review request for Plasma. Summary --- This patch fixes a couple of bugs when having unlimited visible icons in the quicklaunch plasmoid. They were: - Having a vertical panel with unlimited number of icons resulted in just one row of squashed icons - Having unlimited icons gives a bigger width for no reason This should be backported to 4.3 Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/quicklaunch/quicklaunchApplet.cpp 1004927 Diff: http://reviewboard.kde.org/r/1220/diff Testing --- tested and compiled on trunk Thanks, Shafqat ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Use sunrise and sunset times from the Environment Canada weather service to set weather icon to day/night, accordingly.
On 2009-05-03 16:42:23, Shawn Starr wrote: It seems to be working so far, it's already committed. Beat Wolf wrote: i can't find this code in svn, is it really commited? can somebody mark this as submitted? Its been redone. But we still dont have proper sunrise/sunset. - Shawn --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/665/#review1045 --- On 2009-05-03 16:05:40, Andrew Coles wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/665/ --- (Updated 2009-05-03 16:05:40) Review request for Plasma. Summary --- Previously, day began at 6am, and night began at 6pm. However, as summer approaches, this is increasingly far from the truth. Hence, the real sunrise/sunset times are now used to determine whether it is day or night, and the weather icon set appropriately. This also fixes issues with the weather wallpaper picking night-time images during the day. Diffs - /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_envcan.cpp 963098 Diff: http://reviewboard.kde.org/r/665/diff Testing --- I need someone who uses this weather service to test it for me. Thanks, Andrew ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Patch to add a 7-day (up-to) forecast to the NOAA weather Ion
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1584/#review2338 --- It looks fine, but please make sure any new strings are wrapped in proper i18n(). I cannot approve this commit until that's please. - Shawn On 2009-09-13 14:04:24, Amos Kariuki wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1584/ --- (Updated 2009-09-13 14:04:24) Review request for Plasma and Shawn Starr. Summary --- The patch uses the REST API provided by NOAA at http://www.weather.gov/forecasts/xml/rest.php to obtain the forecast for the location coordinates returned by the observation data. It requests the 7-day forecast but only displays the days for which data is available (usually 5-7 days). The weather icon descriptions unfortunately don't match those used by the normal observation feed, and are defined at http://www.weather.gov/forecasts/xml/DWMLgen/schema/parameters.xsd ; we however use a similar heuristic algorithm (keyword search)) to find the best match for the returned weather conditions. This patch also enables the normal condition icon which was being mistakenly being unset. Diffs - trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_noaa.cpp 1021754 trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_noaa.h 1016575 Diff: http://reviewboard.kde.org/r/1584/diff Testing --- Verified the NOAA forecast was displayed in the Weather Forecast applet for a few locations. Thanks, Amos ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: big revamp of Device Notifier
On 2009-09-11 19:44:22, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 366 http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366 this is only needed if m_showOnlyRemovable changes, correct? wrote: it is used also to hide or show the devices when you (un)check the option show all the items in the context menu. wrote: yes, that's what i said :) wrote: no, i mean when you right click the dialog, and you set the option to show or not even devices that normally would be hidden too. like when you right click on the Places panel in dolphin. In fact resetDevices() is called in configAccepted() (if m_showOnlyRemovable changes) and in setAllItemsShown(bool shown) (if m_showAll changes). wrote: ok, let's back up a bit here. that call to resetDevices is in DeviceNotifier::configAccepted. so i'm talking about when the configuration is changed. that means that in configAccepted, which is what line 365 here is, resetDevices _does not need to be called_ unless m_showOnlyRemovable changes. make sense? now forward again :) looking a m_showAll in NotifierView, Show All devices is always in the context menu and always enabled, even when there is nothing to show. probably, if there are no items to show there doesn't need to be a context menu in the view at all. i'm still not really sure what the real world use cases for hiding/showing individual items in the notifier are other than it's neat that i can; i'm trying to imagine someone who has so many items listed there that it's unmanagable otherwise. or someone who keeps two items in one notifier widget and has a second notifier widget with some others? hm. on the other hand, i can see this resulting in confusion when when someone plugs a device in, marks it to not be shown, then later plugs it in (and hopefully it's impossible to have different devices with the same name?) and it doesn't show up. particularly is days or weeks have passed between those two events. and why would i NOT want my camera to show up when i plug it in? what's the use case for that? this is why i'm still really unsure that turning the device *notifier* into a device *listing manager* in this way is the best of ideas. there's really two similar ideas here it seems: managing devices that are plugged and unplugged (which may or may not be storage devices) and listing storage devices. we actually have a widget for both of those tasks, but the storage device lister doesn't provide access to actions related to the device, which the device notifier does. this really comes back to use cases. can you provide use cases to go with the hide/show individual items feature? oh, and the widget should emit configNeedsSaving() after calls to writeEntry :) wrote: ok, i misunderstood a bit what you wanted to say. so, yes. in configAccepted() the call to resetDevices() is needed only if m_showOnlyRemovable changes, so we could actually check if the variable changed value and only in the case call resetDevices() and better yet add or remove the not removable devices without reset all the devices. as regards the show/hide system: saying that Show all devices is always enabled you mean it's always checked? if it is it is definitely a bug in the patch. i don't think the fact that the context menu is shown even when there are no devices is a big deal, but anyway it would be simple to make a check. anyway it uses the uuid provided by solid and not the names to identify the devices, so if you have two devices with the same name and you hide one, the other will remain visible (unless the uuid are equals, but i think this is a really rare case, if it is possible). Actually i decided to develop this feature only because a user on kde-look said to me that the applet was of no use for him because he had lots of devices and, unless he was able to hide some of them, the plasmoid was unusable. (comment #7 on http://kde-look.org/content/show.php?content=106051forumpage=2). i still think a better answer is to extend the disk monitor. however, as the places view does indeed have this behaviour, for consistency i suppose the device notifier can/should too. please make it consistent with the places view (e.g. in dolphin), however, so that the show all entry isn't shown unless there are hidden items to show. after that, this patch can go in afaic. do you have an svn account? if not, would you like to continue working on this (and/or other items) in KDE's svn? - Aaron --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298
Re: Review Request: Quicklaunch: Fix bugs related to having unlimited visible icons
On 2009-08-02 23:51:37, Aaron Seigo wrote: looks fine; perhaps all the instances of -1 should be replaced with a `static const int UNLIMITED_ICONS = -1` in the header file? :) Shafqat Bhuiyan wrote: Ok I added the UNLIMITED_ICONS = -1 :) PS. Could you please commit this because my SVN account still isn't working :S Aaron Seigo wrote: hm.. i thought i saw the update to your account the other day? there's no rush on these fixes at this point (4.3.0 is already tagged, so this will end up in 4.3.1 and 4.4 only), maybe you could find out what's up with your svn account? find one of us on irc maybe and we can try and figure out what's up... Beat Wolf wrote: is there any news on the svn account or the patch? I got my svn account sorted and it seems to work fine now :) I still have the patch on my machine and I'll commit it once I figure it how to backport patches. Any hints are welcome... ;) - Shafqat --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1220/#review1885 --- On 2009-08-03 00:17:24, Shafqat Bhuiyan wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1220/ --- (Updated 2009-08-03 00:17:24) Review request for Plasma. Summary --- This patch fixes a couple of bugs when having unlimited visible icons in the quicklaunch plasmoid. They were: - Having a vertical panel with unlimited number of icons resulted in just one row of squashed icons - Having unlimited icons gives a bigger width for no reason This should be backported to 4.3 Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/quicklaunch/quicklaunchApplet.cpp 1004927 Diff: http://reviewboard.kde.org/r/1220/diff Testing --- tested and compiled on trunk Thanks, Shafqat ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Length of Remote Widget Policies KCM string causing sizing issues in System Settings
Hi everyone, I recently noticed that the new Remote Widgets Policies KCM has a name that causes it to be wrapped to a length of 3 lines. This causes all other lines to increase to 3 lines as well, causing System Settings to use excessive amounts of vertical space. Would it be possible to rename it to something shorter such as Remote Widgets, or Widget Policies? This would allow the previous two line format to be restored. Regards, Ben Cooksley ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
