KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls

Review Request #120160 - Created Sept. 12, 2014 and discarded

Information
Fabio D'Urso
kwidgetsaddons
master
Reviewers
kdeframeworks
cfeck, gateau

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.

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.

Issues

  • 10
  • 0
  • 0
  • 10
Description From Last Updated
Couldn't this actually check that no animation is running at all now? We triggered the hide directly after the show, ... Milian Wolff Milian Wolff
same as above Milian Wolff Milian Wolff
I believe this is missing after line 411: emit showAnimationFinished(); Dominik Haumann Dominik Haumann
I believe this is missing after line 441: emit hideAnimationFinished(); Dominik Haumann Dominik Haumann
btw, are there signals for these animations? then you could use the much faster approach to wait for the signal ... Milian Wolff Milian Wolff
instead, I'd add a QVERIFY(shownHeight) above when you get the value of shownHeight. Milian Wolff Milian Wolff
imo, this should be replaced by an explicit call to the event loop instead of waiting in a loop. If ... Milian Wolff Milian Wolff
considering that the show was instantly stopped by a hide, I would say these checks should be done before handling ... Milian Wolff Milian Wolff
similar to above Milian Wolff Milian Wolff
Would it make sense to emit hideAnimationFinished() before reversing the direction to "showing"? Likewise for the other case. Christoph Feck Christoph Feck
Martin Klapetek
Milian Wolff
Dominik Haumann
Fabio D'Urso
Milian Wolff
Christoph Feck
Christoph Feck
Albert Astals Cid
Fabio D'Urso
Review request changed

Status: Discarded

Loading...