ktooltipwidget: Subtract margins from available size
ClosedPublic
Actions

Authored by michaelh on Mar 27 2018, 8:28 AM.

Details

Reviewers

Commits

R236:309c92215e35: ktooltipwidget: Subtract margins from available size

Summary

Try to fix test failure in CI
https://build.kde.org/view/Frameworks/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20SUSEQt5.10/35/

Test Plan

tooltips position correctly in dolphin

Diff Detail

Repository

R236 KWidgetsAddons

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

michaelh created this revision.Mar 27 2018, 8:28 AM

Restricted Application added a project: Frameworks. · View Herald TranscriptMar 27 2018, 8:28 AM

michaelh requested review of this revision.Mar 27 2018, 8:28 AM

michaelh added inline comments.

src/ktooltipwidget.cpp
186	This is the result of a lot of trials and errors. I'm not ssure if this is the right thing to do.

cfeck added inline comments.Mar 27 2018, 10:20 AM

src/ktooltipwidget.cpp
142	Crashes if size.width > screenGeometry.width. What was wrong with the qMax?

michaelh added inline comments.Mar 27 2018, 10:36 AM

src/ktooltipwidget.cpp
142	Nothing was wrong with qmax. But with line 183-186 `size.width` should always be <= `screenGeometry.width.` In case I got that wrong it's better to crash than to hide my misconception. :-)

michaelh added inline comments.Mar 27 2018, 11:11 AM

src/ktooltipwidget.cpp
142	I forgot to mention `x = qMax(0, ...` did not fix failures in CI

Ping

elvisangelaccio requested changes to this revision.Mar 29 2018, 8:17 PM

elvisangelaccio added inline comments.

autotests/ktooltipwidgettest.cpp
33–37	This is not needed, it is possible to just use the standard style switch: `ktooltipwidgettest -style windows`
src/ktooltipwidget.cpp
183	`const`

This revision now requires changes to proceed.Mar 29 2018, 8:17 PM

Apply suggested changes

cfeck added inline comments.Mar 30 2018, 1:03 AM

src/ktooltipwidget.cpp
183	Call me old-school, but I prefer to see the type before the variable name, i.e. const QSize marginSize(w, h); instead of const auto marginSize = QSize(w, h); One word less to read.

Demonstrate respect to old school ;-)

elvisangelaccio accepted this revision as: elvisangelaccio.Mar 30 2018, 8:40 AM

elvisangelaccio removed a reviewer: Frameworks.

This revision is now accepted and ready to land.Mar 30 2018, 8:41 AM

Restricted Application added a subscriber: Frameworks. · View Herald TranscriptMar 30 2018, 8:41 AM

Closed by commit R236:309c92215e35: ktooltipwidget: Subtract margins from available size (authored by michaelh). · Explain WhyMar 30 2018, 9:36 AM

This revision was automatically updated to reflect the committed changes.

You made CI happy, merci :)

In D11740#236892, @cfeck wrote:

You made CI happy, merci :)

I've been watching it 'live'. I'm happy too.

Revision Contents
Changeset List

			Path	Packages
M			autotests/ktooltipwidgettest.cpp (50 lines)
M			src/ktooltipwidget.cpp (7 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	30703	6b88b7c		Mar 27 2018, 8:23 AM	★	★
Diff 2	30888	6b88b7c	- Apply suggested changes	Mar 29 2018, 10:02 PM	★	★
Diff 3	30903	6b88b7c	- Demonstrate respect to old school ;-)	Mar 30 2018, 7:29 AM	★	★
Diff 4	30911	7fe64f5	R236:309c92215e352deb8ac477484f8632db2b5b3d11	Mar 30 2018, 9:39 AM	★	★

ktooltipwidget: Subtract margins from available sizeClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 30911

autotests/ktooltipwidgettest.cpp

src/ktooltipwidget.cpp

ktooltipwidget: Subtract margins from available size
ClosedPublic
Actions

Revision Contents
Changeset List