KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide
ClosedPublic

Authored by kossebau on Jan 29 2017, 2:35 AM.

Details

Summary

Calling animatedHide() on a not yet shown KMessageWidget instance was
ignored when the parent
And instant counter-direction updates (like it can happen due to complicated signal
handling on UI updates) resulted in the last calls being ignored,
so an instant hide after a show or an instant show after a hide were
ineffective, resulting in wrong messages or wrongly no messages shown.

Test Plan

Works now in WIP KDevelop code,
and new unit tests no longer fail.

Diff Detail

Repository
R236 KWidgetsAddons
Branch
fixKMessageWidgetInstantShowHide
Lint
No Linters Available
Unit
No Unit Test Coverage
kossebau updated this revision to Diff 10667.Jan 29 2017, 2:35 AM
kossebau retitled this revision from to KMessageWidget: fix behaviour on overlapping calls of animatedShow/animatedHide.
kossebau updated this object.
kossebau edited the test plan for this revision. (Show Details)
kossebau added reviewers: Frameworks, dhaumann.
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 29 2017, 2:35 AM
kossebau updated this revision to Diff 10669.Jan 29 2017, 2:50 AM

deduplicate unit tests

cfeck added a subscriber: cfeck.Feb 2 2017, 12:44 AM
cfeck added inline comments.
autotests/kmessagewidgetautotest.cpp
24

Isn't QSignalSpy part of QtTest? In other words, isn't it included by above #include line?

159

if (delay) {

QTest::bla;

}

192

s.a.

192

s.a.

216

s.a.

kossebau updated this revision to Diff 10837.Feb 2 2017, 1:40 AM
kossebau marked 5 inline comments as done.

updates to code style, avoid unneeded include

kossebau updated this revision to Diff 10838.Feb 2 2017, 1:44 AM

add a note to the "500" value in code to keep test in sync

dhaumann edited edge metadata.Feb 2 2017, 12:05 PM

First of all a general note: Unfortunately, we have had quite some issues with getting the geometry right in KMessageWidget over the past few years. Essentially, the problem is that KMessageWidgets animates its show and hide events, and therefore caches its initial and its final geometry, as well as the pixmap to render to avoid stretching the KMessageWidget's contents. That said, doing this caching too early results in a final geometry that is wrong (we had this issue in Kate a lot).

This may sound a bit off-topic, but the point is: it's easy to break things. If you in addition also run the unit test 'messagetest' of KTextEditor and if it passes, then I'm fine with this change. It looks reasonable after all :-)

messagetest for me always ran with this result:

Totals: 4 passed, 0 failed, 0 skipped, 0 blacklisted, 3603ms

Though I had to hack into KTextEditor to make it actually also call animatedShow & animatedHide(), to test if there is any effect ;)
true -> false here in KTextEditor::ViewPrivate::postMessage(...):

m_floatTopMessageWidget = new KateMessageWidget(m_viewInternal, false);
dhaumann accepted this revision.Feb 5 2017, 1:35 PM
dhaumann edited edge metadata.

KTextEditor uses KMessageWidget 4 times: floating inside on top right or bottom right. This is what you hacked.
And 2 times above and below. These two times use the animation, but given the unit test runs fine, I think this is ok.

This revision is now accepted and ready to land.Feb 5 2017, 1:35 PM
This revision was automatically updated to reflect the committed changes.