----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129373/#review100763 -----------------------------------------------------------
Thanks for your patch! I'm not fully sure about this, but let's wait for other Plasma developer feedback :) Especially, since I couldn't yet look at your patch that actually makes use of this API in icontasks. src/declarativeimports/core/tooltip.h (line 106) <https://git.reviewboard.kde.org/r/129373/#comment67630> This won't work when tooltip is "interactive: false" (the default) because then mouse events pass right through it src/declarativeimports/core/tooltip.h (line 128) <https://git.reviewboard.kde.org/r/129373/#comment67631> I know it's been wrong in the other property already but "if a tooltip will show". Also, clarify when, "automatically show when the mouse hovers" or sth like that src/declarativeimports/core/tooltip.h (line 147) <https://git.reviewboard.kde.org/r/129373/#comment67632> Regardless of the rest of this patch, I like this. For example, in Application Dashboard we could use a single ToolTip instance and set the title and manually show it, just like we only have one global MouseArea in there as well. src/declarativeimports/core/tooltip.cpp (line 162) <https://git.reviewboard.kde.org/r/129373/#comment67633> You're connecting to this signal every time you do showToolTip(), use Qt::UniqueConnection src/declarativeimports/core/tooltip.cpp (line 332) <https://git.reviewboard.kde.org/r/129373/#comment67634> Coding style, brace on next line for functions: void Foo() { ... } src/declarativeimports/core/tooltip.cpp (line 350) <https://git.reviewboard.kde.org/r/129373/#comment67635> Coding style: Space after if: if (foo) { src/declarativeimports/core/tooltip.cpp (line 358) <https://git.reviewboard.kde.org/r/129373/#comment67639> I just wanted to complain about it not being keepAlive but that's been already there before :) src/declarativeimports/core/tooltipdialog.cpp (line 153) <https://git.reviewboard.kde.org/r/129373/#comment67636> can probably be const QObject * src/declarativeimports/core/tooltipdialog.cpp (line 155) <https://git.reviewboard.kde.org/r/129373/#comment67637> Coding style src/declarativeimports/core/tooltipdialog.cpp (line 162) <https://git.reviewboard.kde.org/r/129373/#comment67638> Coding style - Kai Uwe Broulik On Nov. 10, 2016, 8:47 nachm., René Fürst wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129373/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2016, 8:47 nachm.) > > > Review request for Plasma and Eike Hein. > > > Repository: plasma-framework > > > Description > ------- > > Plasma Framework changes for #129372 > > > Diffs > ----- > > src/declarativeimports/core/tooltip.h d38c49b > src/declarativeimports/core/tooltip.cpp ffe1064 > src/declarativeimports/core/tooltipdialog.h d4e0ff0 > src/declarativeimports/core/tooltipdialog.cpp 28ba9be > > Diff: https://git.reviewboard.kde.org/r/129373/diff/ > > > Testing > ------- > > > Thanks, > > René Fürst > >
