Re: Review Request 129648: New widget: tooltip that contains another widget

2017-01-01 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Jan. 1, 2017, 4:56 p.m.) Status -- This change has been mar

Re: Review Request 129648: New widget: tooltip that contains another widget

2017-01-01 Thread Elvis Angelaccio
> On Jan. 1, 2017, 3:25 p.m., Martin Gräßlin wrote: > > Looks good to me. Using the transientParent's qscreen was what I meant. Thanks, I'm going to commit then. :) - Elvis --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 129648: New widget: tooltip that contains another widget

2017-01-01 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101699 --- Looks good to me. Using the transientParent's qscreen was wh

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-30 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 30, 2016, 10:27 a.m.) Review request for KDE Frameworks, Be

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-30 Thread Elvis Angelaccio
> On Dec. 29, 2016, 4:01 p.m., Martin Gräßlin wrote: > > src/ktooltipwidget.cpp, line 101 > > > > > > this won't work on Wayland, there is no global cursor pos. > > Elvis Angelaccio wrote: > hmm, not sure w

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-29 Thread Martin Gräßlin
> On Dec. 29, 2016, 5:01 p.m., Martin Gräßlin wrote: > > src/ktooltipwidget.cpp, line 101 > > > > > > this won't work on Wayland, there is no global cursor pos. > > Elvis Angelaccio wrote: > hmm, not sure w

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-29 Thread Elvis Angelaccio
> On Dec. 29, 2016, 4:01 p.m., Martin Gräßlin wrote: > > src/ktooltipwidget.cpp, line 101 > > > > > > this won't work on Wayland, there is no global cursor pos. hmm, not sure what to use instead. `rect`? Or may

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-29 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101652 --- src/ktooltipwidget.cpp (line 101)

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-29 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 29, 2016, 10:11 a.m.) Review request for KDE Frameworks, Be

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-28 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101637 --- src/ktooltipwidget.cpp (line 98)

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-28 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101627 --- src/ktooltipwidget.cpp (lines 139 - 141)

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 21, 2016, 6:43 p.m.) Review request for KDE Frameworks, Ben

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Christoph Feck
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101542 --- Fix it, then Ship it! That's it from my side. src/ktool

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 21, 2016, 5:57 p.m.) Review request for KDE Frameworks, Ben

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Christoph Feck
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101540 --- src/ktooltipwidget.cpp (line 48)

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
> On Dec. 21, 2016, 5:23 p.m., Christoph Feck wrote: > > src/ktooltipwidget.cpp, line 94 > > > > > > 'auto' is C++11, too (if used as a type). Please use explicit types > > here. > > > > Especially with

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 21, 2016, 5:29 p.m.) Review request for KDE Frameworks and

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Christoph Feck
> On Dec. 21, 2016, 5 p.m., Christoph Feck wrote: > > src/ktooltipwidget.h, line 72 > > > > > > I am not a native english speaker, but I think 'showBelow' sounds > > better. > > > > Needs a comment from

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Christoph Feck
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101538 --- src/ktooltipwidget.cpp (line 94)

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 21, 2016, 5:19 p.m.) Review request for KDE Frameworks, Ben

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
> On Dec. 21, 2016, 4 p.m., Christoph Feck wrote: > > src/ktooltipwidget.h, line 72 > > > > > > I am not a native english speaker, but I think 'showBelow' sounds > > better. > > > > Needs a comment from

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 21, 2016, 5:01 p.m.) Review request for KDE Frameworks, Ben

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Elvis Angelaccio
> On Dec. 21, 2016, 4 p.m., Christoph Feck wrote: > > Looks good so far, but see issues below. Please do not use C++11 features > > in library code yet (it's okey for tests). > > > > Regarding the 500 ms timeout, does it need to be configurable using a > > property? I am unsure how the interac

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-21 Thread Christoph Feck
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101534 --- Looks good so far, but see issues below. Please do not use C

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-20 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 20, 2016, 3:40 p.m.) Review request for KDE Frameworks, Ben

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-17 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 17, 2016, 4:24 p.m.) Review request for KDE Frameworks, Ben

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-17 Thread Christoph Feck
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101481 --- Minor nitpick: it's "ToolTip", not "Tooltip", see QToolTip c

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-17 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 17, 2016, 12:35 p.m.) Review request for KDE Frameworks, Be

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-17 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- (Updated Dec. 17, 2016, 11:25 a.m.) Review request for KDE Frameworks and

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-17 Thread Martin Gräßlin
> On Dec. 14, 2016, 8:21 p.m., Martin Gräßlin wrote: > > If I see correctly we are losing a feature here: blur behind. > > Elvis Angelaccio wrote: > Right, I had to drop that because we cannot use KWindowSystem in tier 1. > Is there a way to achieve the same feature with Qt only? > > Marti

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-15 Thread Elvis Angelaccio
> On Dec. 14, 2016, 7:21 p.m., Martin Gräßlin wrote: > > If I see correctly we are losing a feature here: blur behind. > > Elvis Angelaccio wrote: > Right, I had to drop that because we cannot use KWindowSystem in tier 1. > Is there a way to achieve the same feature with Qt only? > > Marti

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-15 Thread Martin Gräßlin
> On Dec. 14, 2016, 8:21 p.m., Martin Gräßlin wrote: > > If I see correctly we are losing a feature here: blur behind. > > Elvis Angelaccio wrote: > Right, I had to drop that because we cannot use KWindowSystem in tier 1. > Is there a way to achieve the same feature with Qt only? Well ther

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-14 Thread Elvis Angelaccio
> On Dec. 14, 2016, 7:21 p.m., Martin Gräßlin wrote: > > If I see correctly we are losing a feature here: blur behind. Right, I had to drop that because we cannot use KWindowSystem in tier 1. Is there a way to achieve the same feature with Qt only? - Elvis --

Re: Review Request 129648: New widget: tooltip that contains another widget

2016-12-14 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/#review101445 --- If I see correctly we are losing a feature here: blur behind

Review Request 129648: New widget: tooltip that contains another widget

2016-12-13 Thread Elvis Angelaccio
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129648/ --- Review request for KDE Frameworks, Ben Cooksley, Christoph Feck, and Martin