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