----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/251/#review414 -----------------------------------------------------------
I haven't tried this patch.. just a quick review before I go do other stuff /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp <http://reviewboard.kde.org/r/251/#comment250> so... your config dialog only allows you to configure the comic for the current tab? awww. well... I can't think of a better way anyways... maybe that's not such a bad idea... /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp <http://reviewboard.kde.org/r/251/#comment248> why do you need this check? are there other parts of the code messing with your tab lists? /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp <http://reviewboard.kde.org/r/251/#comment249> you put the text in the list, and then you get it out? faster to just use mComicTitle directly here /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp <http://reviewboard.kde.org/r/251/#comment246> aren't you supposed to accept/ignore the event or something? /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp <http://reviewboard.kde.org/r/251/#comment242> o.0 I actually had to go read up on static to understand this. I have bad feelings about this... /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp <http://reviewboard.kde.org/r/251/#comment243> what's entfernen? :) /trunk/KDE/kdeplasma-addons/applets/comic/configwidget.h <http://reviewboard.kde.org/r/251/#comment244> if it's a hack, you probably shouldn't do it. :) /trunk/KDE/kdeplasma-addons/applets/comic/imagewidget.cpp <http://reviewboard.kde.org/r/251/#comment247> again, I think you're supposed to accept/ignore events... and I'm concerned as to how hte scrollbar scrolling interacts with the tab-change scrolling. it feels fragile right now - you're doing lots of checks to make sure nothing conflicts, but I think there's a better way. - Chani On 2009-03-07 12:00:29, Matthias Fuchs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/251/ > ----------------------------------------------------------- > > (Updated 2009-03-07 12:00:29) > > > Review request for Plasma. > > > Summary > ------- > > Adds tabs to the comic applet, on default the tab bar is not shown. > > Ctrl + Scrolling on the image also changes the tabs, that way you can hide > the tab bar but still access the tabs. > > > Diffs > ----- > > /trunk/KDE/kdeplasma-addons/applets/comic/appearanceSettings.ui 936459 > /trunk/KDE/kdeplasma-addons/applets/comic/comic.h 936459 > /trunk/KDE/kdeplasma-addons/applets/comic/comic.cpp 936459 > /trunk/KDE/kdeplasma-addons/applets/comic/configwidget.h 936459 > /trunk/KDE/kdeplasma-addons/applets/comic/configwidget.cpp 936459 > /trunk/KDE/kdeplasma-addons/applets/comic/imagewidget.cpp 936459 > > Diff: http://reviewboard.kde.org/r/251/diff > > > Testing > ------- > > Works, though sometimes the tab bar disappears, especially when scrolling > back. notmart could not reproduce it though, so maybe a local problem. > > > Thanks, > > Matthias > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
