D9973: ktooltipwidget: Fix tooltip positioning

[Michael Heidelbach]

michaelh marked 2 inline comments as done.
michaelh added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in ktooltippositiontest.cpp:65
> Weird comma position :p

Old habits, sorry.

> elvisangelaccio wrote in ktooltippositiontest.cpp:136
> This should be a QCOMPARE

We're letting go of the description with QCOMPARE

> elvisangelaccio wrote in ktooltippositiontest.cpp:148
> What do you mean with "test is wrong"? Can this branch ever happen?

A better message would have been "michaelh was not thinking correctly" ;-)

> Can this branch ever happen?

Not unless we add conditions which are expected to fail. In that case testing the margin makes no sense.

> elvisangelaccio wrote in ktooltipwidget.cpp:122
> `centerBelow()` is `const`, but this actually changes the tooltip, right?
> Maybe we can move this call to `addWidget()` ?

Not needed. Dolphin calls `adjustSize() ` on the widget before calling `tooltip->showBelow()`.

> elvisangelaccio wrote in ktooltipwidget.cpp:142
> This looks unrelated to this patch. I don't see a testcase for it and if I revert this change, the new tests still pass.
> 
> Ideally we need a failing test case that is fixed by this line of code. Btw, don't we have a similar "negative y" problem?

In addition I cannot reproduce the behaviour depicted here <https://phabricator.kde.org/file/data/2pnyr5uk2oqkhugvg3hb/PHID-FILE-ewztr5h5iwmbc6rk2n64/tt-placement-outside.png> anymore.

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D9973

To: michaelh, elvisangelaccio, #frameworks, #dolphin, ngraham
Cc: dfaure, cfeck, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180305/2b97d63c/attachment.htm>