----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/#review66825 -----------------------------------------------------------
Ship it! some more nitpickery from my side, but otherwise this is good already. I'd like to see this in and not hold you up much longer. autotests/kmessagewidgetautotest.cpp <https://git.reviewboard.kde.org/r/120160/#comment46606> btw, are there signals for these animations? then you could use the much faster approach to wait for the signal via a QSignalSpy. autotests/kmessagewidgetautotest.cpp <https://git.reviewboard.kde.org/r/120160/#comment46605> instead, I'd add a QVERIFY(shownHeight) above when you get the value of shownHeight. autotests/kmessagewidgetautotest.cpp <https://git.reviewboard.kde.org/r/120160/#comment46607> imo, this should be replaced by an explicit call to the event loop instead of waiting in a loop. If I understand you correctly, the animation just needs to work through the pending events to know its not running anymore. there should not be any time delay involved otherwise. qApp->processEvents(); autotests/kmessagewidgetautotest.cpp <https://git.reviewboard.kde.org/r/120160/#comment46608> considering that the show was instantly stopped by a hide, I would say these checks should be done before handling any events and then afterwards as well. autotests/kmessagewidgetautotest.cpp <https://git.reviewboard.kde.org/r/120160/#comment46609> similar to above - Milian Wolff On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120160/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2014, 10:40 a.m.) > > > Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. > > > Repository: kwidgetsaddons > > > Description > ------- > > Hi, I've found that if you call animatedShow() immediatly after > animatedHide(), without waiting for the hide animation to finish first, the > hide animation goes on and, in the end, the message widget becomes hidden. > > The attached patch adds checks in animatedShow and animatedHide so that, if > an opposite animation is currently running, they just reverse it. > > > Diffs > ----- > > autotests/kmessagewidgetautotest.h 062e2b3 > autotests/kmessagewidgetautotest.cpp f46bab0 > src/kmessagewidget.cpp bc7ba32 > tests/kmessagewidgettest.cpp f621b5a > > Diff: https://git.reviewboard.kde.org/r/120160/diff/ > > > Testing > ------- > > I've added buttons to invoke animatedShow and animatedHide in > tests/kmessagewidget.cpp and used them to test the behavior. > > I've also added an autotest that checks the final height and isVisible() > value, both after single animatedShow/animatedHide calls and after > animatedShow+animatedHide and animatedHide+animatedShow. > > > Thanks, > > Fabio D'Urso > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
