[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: QToolTip of Ksnapshot disables the plasma-effects.
From: jignesh kakadiya <jigneshhk1992 () gmail ! com>
Date: 2011-09-26 17:17:29
Message-ID: CANPWanz92ES1h_ZjX4e=rrrvR4dVhYP6-qZ7CVC2_XerMebCiQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
I submitted my patch here ( https://bugs.kde.org/show_bug.cgi?id=235545 ).
Let me know If it requires more changes or not.
-Jignesh
On Mon, Sep 26, 2011 at 3:32 PM, Aaron J. Seigo <aseigo@kde.org> wrote:
> On Sunday, September 25, 2011 18:39:10 jignesh kakadiya wrote:
> > http://paste.kde.org/126985/ here is the Ksnapshottimer.cpp
>
> please post patches, not full copies of the source files, so that we can
> see
> the changes. `git diff` makes this easy.
>
> some comments:
>
> * please keep indentation and whitespace identical in the file to other
> uses.
> e.g. it should be "if (" not "if(" and the opening curly brace on line 84
> looks like it is misalinged
>
> * an implementation of enterEvent would probably have been enough;
> mouseMoveEvent is only really better if one assumes that after moving the
> lable, it might still be under the mouse ... but in that case moving it
> back
> will have the same problem. enterEvent allows you to avoid the check for
> "is
> in the mouse inside the widget" and so keeps the code simpler
>
> * you don't need two booleans for "onLeftCorner" and "onRightCorner". just
> one
> is enough. even then, a simple check for "if (x() == screenGeom.left())"
> would
> probably be enough to know if it is in the left corner.
>
> * the move call to (0, 0) on line 85 will break for screens whose geometry
> does not start at (0, 0). it should instead be move(screenGeom.topLeft()).
> same for the move call on line 78 which uses 0 for the y coordinate.
>
> > > /home/jiggy/kde/src/ksnapshot/snapshottimer.cpp:76:49: error: invalid
> use
> > > of incomplete type ‘struct QDesktopWidget'
>
> the file is missing: #include <QDesktopWidget>
>
> --
> 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
>
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>
>
[Attachment #5 (text/html)]
I submitted my patch here ( <a \
href="https://bugs.kde.org/show_bug.cgi?id=235545">https://bugs.kde.org/show_bug.cgi?id=235545</a> \
). Let me know If it requires more changes or not.<br><br>-Jignesh<br><br><div \
class="gmail_quote"> On Mon, Sep 26, 2011 at 3:32 PM, Aaron J. Seigo <span \
dir="ltr"><<a href="mailto:aseigo@kde.org">aseigo@kde.org</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex;"> <div class="im">On Sunday, September 25, 2011 18:39:10 \
jignesh kakadiya wrote:<br> > <a href="http://paste.kde.org/126985/" \
target="_blank">http://paste.kde.org/126985/</a> here is the Ksnapshottimer.cpp<br> \
<br> </div>please post patches, not full copies of the source files, so that we can \
see<br> the changes. `git diff` makes this easy.<br>
<br>
some comments:<br>
<br>
* please keep indentation and whitespace identical in the file to other uses.<br>
e.g. it should be "if (" not "if(" and the opening curly brace on \
line 84<br> looks like it is misalinged<br>
<br>
* an implementation of enterEvent would probably have been enough;<br>
mouseMoveEvent is only really better if one assumes that after moving the<br>
lable, it might still be under the mouse ... but in that case moving it back<br>
will have the same problem. enterEvent allows you to avoid the check for "is<br>
in the mouse inside the widget" and so keeps the code simpler<br>
<br>
* you don't need two booleans for "onLeftCorner" and \
"onRightCorner". just one<br> is enough. even then, a simple check for \
"if (x() == screenGeom.left())" would<br> probably be enough to know if it \
is in the left corner.<br> <br>
* the move call to (0, 0) on line 85 will break for screens whose geometry<br>
does not start at (0, 0). it should instead be move(screenGeom.topLeft()).<br>
same for the move call on line 78 which uses 0 for the y coordinate.<br>
<div class="im"><br>
> > /home/jiggy/kde/src/ksnapshot/snapshottimer.cpp:76:49: error: invalid \
use<br> </div>> > of incomplete type ‘struct QDesktopWidget'<br>
<br>
the file is missing: #include <QDesktopWidget><br>
<font color="#888888"><br>
--<br>
</font><div><div></div><div class="h5">Aaron J. Seigo<br>
humru othro a kohnu se<br>
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43<br>
<br>
KDE core developer sponsored by Qt Development Frameworks<br>
</div></div><br>_______________________________________________<br>
Plasma-devel mailing list<br>
<a href="mailto:Plasma-devel@kde.org">Plasma-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/plasma-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br> \
<br></blockquote></div><br>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic