[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Junior job - zooming tool doesn't zoom out
From:       Thomas Zander <zander () kde ! org>
Date:       2009-09-05 19:41:31
Message-ID: 200909052141.32584.zander () kde ! org
[Download RAW message or body]

On Saturday 5. September 2009 12.04.42 Hans Bakker wrote:
> Hi all,
> I made a beginning of the closing of bug 171976
> <http://bugs.kde.org/171976>. 

nice!

> The patch is attached, but there are still
> some things left to do that I'd like some help with:
>
>    - when pressing Ctrl, sometimes the cursor is only updated when moving
>    the mouse. Right now at the key*Event methods, event->ignore(); is
> called.

I notice that on startup neither of the 2 radio buttons are selected and 
then the ctrl works exactly as expected.
Only after selecting one it starts to fail.
So I think that the ignore() has nothing to do with it, rather there is a 
bug in the logic somewhere.
I suggest adding some kDebug() statements to figure out what is happening.

> - the birdEyeLabel is not used / updated yet.

I suggest to hide the widget (birdEyeLabel->hide()) so it doesn't cause the 
painting issues we see now.

Other comments on the patch;
please make sure you follow the coding style; 
http://techbase.kde.org/Policies/Kdelibs_Coding_Style

In finishInteraction you duplicated the code; I suggest to instead introduce 
a local boolean that initially gets its value from m_forceZoomOut, but that 
the ctrl key can change. Then you can avoid the duplication.

I noticed you added the same 4 lines of 'setCursor' code various times, I'm 
thinking this is a good candidate for a private method. Something like;
  updateCursor(bool invertZoom);
so you can call;
  updateCursor(event->modifiers() & Qt::ControlModifier);
for instance.

Please re-send the patch after looking into these issues.
-- 
Thomas Zander
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]