Re: Review Request: Device notifier popup did not hide after the intended timeout of 7.5 s.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102824/#review7598 --- removing the emit activate() line is fine, but the rest should be left as-is. more comments below. plasma/generic/applets/devicenotifier/devicenotifier.cpp http://git.reviewboard.kde.org/r/102824/#comment6589 this was there to prevent the following case: * dialog autopops up * user clicks on an entry in there * the dialog goes away before they are done please test that this still works before removing this connection. plasma/generic/applets/devicenotifier/devicenotifier.cpp http://git.reviewboard.kde.org/r/102824/#comment6590 activate results in focus being given and the popup being shown. however, showPopup does the same thing in this case. removing this emit should be safe enough, however i do wonder why it was interfering with the LONG_NOTIFICATION_TIMEOUT period. one thing that this code would result in is activate() being emitted twice (once directly here and once due to the showPopup code). plasma/generic/applets/devicenotifier/notifierdialog.h http://git.reviewboard.kde.org/r/102824/#comment6591 should remain plasma/generic/applets/devicenotifier/notifierdialog.cpp http://git.reviewboard.kde.org/r/102824/#comment6592 should remain - Aaron J. Seigo On Oct. 11, 2011, 10:18 a.m., Simon Persson wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102824/ --- (Updated Oct. 11, 2011, 10:18 a.m.) Review request for Plasma, Giulio Camuffo and Jacopo De Simoi. Description --- This is my first fix for plasma, maybe there's a reason to emit plasma::applet::activated() that I don't know. Removing it fixes the problem and I can't see any regression. I also needed to remove activated() signal from NotifierDialog, I consider this one to be unnecessary since there is an eventfilter that on QEvent::GraphicsSceneHoverMove triggers the popup to be open for 7.5 s more. In other words, the popup will now close after 7.5 s of inactive mouse if it was shown as a result of plugging in a device... which I guess was the intended behavior. Couldn't actually find any open bug report for this. (!) Diffs - plasma/generic/applets/devicenotifier/notifierdialog.cpp dff38d9 plasma/generic/applets/devicenotifier/notifierdialog.h 8a03fc5 plasma/generic/applets/devicenotifier/devicenotifier.cpp b9dfce5 Diff: http://git.reviewboard.kde.org/r/102824/diff/diff Testing --- Thanks, Simon Persson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Device notifier popup did not hide after the intended timeout of 7.5 s.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102824/#review7463 --- Sorry for the late reply. First of all notice that there is a complete qml rewrite of the device notifier which could make it for 4.8. I still don't know if it will be the case, so, I'll review your patch in any case. I kind of remember that the call to activate() was indeed useful, but I have to recollect my memories, which unfortunately doesn't happen right now. I'll have a more in-depth look. Please bear with me a bit more; spare time is very scarce in these days. - Jacopo De Simoi On Oct. 11, 2011, 10:18 a.m., Simon Persson wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102824/ --- (Updated Oct. 11, 2011, 10:18 a.m.) Review request for Plasma, Giulio Camuffo and Jacopo De Simoi. Description --- This is my first fix for plasma, maybe there's a reason to emit plasma::applet::activated() that I don't know. Removing it fixes the problem and I can't see any regression. I also needed to remove activated() signal from NotifierDialog, I consider this one to be unnecessary since there is an eventfilter that on QEvent::GraphicsSceneHoverMove triggers the popup to be open for 7.5 s more. In other words, the popup will now close after 7.5 s of inactive mouse if it was shown as a result of plugging in a device... which I guess was the intended behavior. Couldn't actually find any open bug report for this. (!) Diffs - plasma/generic/applets/devicenotifier/notifierdialog.cpp dff38d9 plasma/generic/applets/devicenotifier/notifierdialog.h 8a03fc5 plasma/generic/applets/devicenotifier/devicenotifier.cpp b9dfce5 Diff: http://git.reviewboard.kde.org/r/102824/diff/diff Testing --- Thanks, Simon Persson ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
