Review Request: Fix scrolling with mouse (without wheel) in krunner

Jacopo De Simoi wilderkde at gmail.com
Thu Jul 16 20:57:49 CEST 2009



> On 2009-07-16 17:47:03, Aaron Seigo wrote:
> > branches/KDE/4.3/kdebase/workspace/krunner/interfaces/default/resultsview.cpp, line 176
> > <http://reviewboard.kde.org/r/948/diff/2/?file=8540#file8540line176>
> >
> >     blitting the whole pixmap on every paint event seems a bit wasteful; it should probably just paint the event->rect()
> 
>  wrote:
>     We would need to paint the event->rect() + the rects of the gradients since they could be shown/hidden without notice... 
>     I thought we would not get a performance gain in calling three times drawPixmap w.r.t. just once with a big pixmap. 
>     I'll look into that.
>     
>     Thanks for your prompt answer.

It works just with event->rect() as you said; 
this is probably because whenever an event would trigger the buttons (and therefore the gradients), there is a paint event on the region of the gradient.  This is obvious while scrolling, less obvious when adding items, but it seems still ok.
I'll see if I encounter any glitches, otherwise I'll commit.


- Jacopo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/948/#review1630
-----------------------------------------------------------


On 2009-07-15 09:49:09, Jacopo De Simoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/948/
> -----------------------------------------------------------
> 
> (Updated 2009-07-15 09:49:09)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Krunner does not have a vertical scrollbar in the result view for design reasons. This patch fixes this by adding two page buttons.
> The buttons are painted inside the view and are shown only if they are needed. Also, the view fades if other items are available but not visible (please see attached screenshot).
> This was done for a couple of design reasons:
> - putting the buttons outside the view was wasting too much space if they were always visible;
> - making them appear/disappear would necessarily displace up or down the results view, possibly during user interaction (in case new results are added late) and possibly resulting in misclicks;
> As for the technical side please note:
> - the scrolling is actually implemented by selecting an item which is outside the current view; this smoothly scrolls the view for free with a nice animation, as a bonus the selected item is always visible. A double fallback mechanism makes (hopefully) sure that some scrolling is always performed.
> Some questions:
> - to fade the view I could not find a better way than calling QGV::render(); _please_ have a look at paintEvent and let me know if there is a more efficient way to do what I do.
> - to check if there are items outside the view I use some very naïve approach. Is there a better way to do that ?
> - apparently the strings ``Previous Page'' and ``Next Page'' have already been i18ned; can I add them as tooltips? does this break the string freeze?
> - now that we subclass QGV, should I move all QGV->initialization stuff which is now in the ctor of interface to the ctor of resultsview?
> 
> Finally, I would consider this a bugfix rather than a feature (we have already a bug opened for that and we already established we do not want a real scrollbar) and throw it in 4.3 branch before release. However, of course the final decision is not mine, so let me know what I should do.
> Thanks
> 
> 
> This addresses bug 198501.
>     https://bugs.kde.org/show_bug.cgi?id=198501
> 
> 
> Diffs
> -----
> 
>   branches/KDE/4.3/kdebase/workspace/krunner/CMakeLists.txt 995691 
>   branches/KDE/4.3/kdebase/workspace/krunner/interfaces/default/interface.cpp 995765 
>   branches/KDE/4.3/kdebase/workspace/krunner/interfaces/default/resultsview.h PRE-CREATION 
>   branches/KDE/4.3/kdebase/workspace/krunner/interfaces/default/resultsview.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/948/diff
> 
> 
> Testing
> -------
> 
> Tested for a few days, did not notice any glitch so far
> 
> 
> Screenshots
> -----------
> 
> Mouse hovering the previous page button
>   http://reviewboard.kde.org/r/948/s/135/
> 
> 
> Thanks,
> 
> Jacopo
> 
>



More information about the Plasma-devel mailing list