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
