----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100230/#review889 -----------------------------------------------------------
src/tabbar.h <http://git.reviewboard.kde.org/r/100230/#comment719> Space at the end of the line... src/tabbar.cpp <http://git.reviewboard.kde.org/r/100230/#comment726> Qt::black instead of QColor(0, 0, 0)? src/tabbar.cpp <http://git.reviewboard.kde.org/r/100230/#comment727> Shouldn't you set the deletion policy to QAbstractAnimation::DeleteWhenStopped to avoid leaking it? src/tabhighlighteffect.h <http://git.reviewboard.kde.org/r/100230/#comment724> Superfluous spaces.... src/tabhighlighteffect.h <http://git.reviewboard.kde.org/r/100230/#comment720> Does it make sense to have the object without parent or tab bar? I would rather have an explicit construtor taking the tab bar it applies on: explicit TabHighlightEffect(QTabBar *tabBar); src/tabhighlighteffect.h <http://git.reviewboard.kde.org/r/100230/#comment725> That could be a const pointer. src/tabhighlighteffect.cpp <http://git.reviewboard.kde.org/r/100230/#comment722> If you follow the previous advice about the constructor, you could add an assert here: Q_ASSERT(m_tabBar); src/tabhighlighteffect.cpp <http://git.reviewboard.kde.org/r/100230/#comment723> This is properly unreadable :) You should split the line over temporary variable. Especially the evName.remove(0,5) is mysterious. - Benjamin On Jan. 6, 2011, 8:06 p.m., Johannes Tröscher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100230/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2011, 8:06 p.m.) > > > Review request for rekonq. > > > Summary > ------- > > This patch highlights inactive tabs if their title has changed, so people get > aware of changes in the tabs (for instance a new message in a chat window). > > I don't have a kde git account, so please merge it if you like it. > > > Diffs > ----- > > src/CMakeLists.txt 6cf2651 > src/mainview.cpp 94dc168 > src/tabbar.h acd8e2f > src/tabbar.cpp 286d7d4 > src/tabhighlighteffect.h PRE-CREATION > src/tabhighlighteffect.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/100230/diff > > > Testing > ------- > > compiles and works > > > Thanks, > > Johannes > >
_______________________________________________ rekonq mailing list [email protected] https://mail.kde.org/mailman/listinfo/rekonq
