Review Request: Device notifier popup did not hide after the intended timeout of 7.5 s.
Aaron J. Seigo
aseigo at kde.org
Tue Oct 25 12:27:10 UTC 2011
-----------------------------------------------------------
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20111025/eb778e57/attachment.html>
More information about the Plasma-devel
mailing list