----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/948/#review1485 -----------------------------------------------------------
i quite like how it feels in use. nice :) and yes, i think the initialization code should go into the subclass itself to keep it tidy. as for the fade effect, however, that should be using QGraphicsView::drawForeground :) other than that, i think this is ready to go, and yes, should be backported for 4.3 - Aaron On 2009-07-07 10:12:01, Jacopo De Simoi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/948/ > ----------------------------------------------------------- > > (Updated 2009-07-07 10:12:01) > > > 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 991523 > branches/KDE/4.3/kdebase/workspace/krunner/interfaces/default/interface.cpp > 991523 > > branches/KDE/4.3/kdebase/workspace/krunner/interfaces/default/resultscene.cpp > 991523 > 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 > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel