Review Request 129648: New widget: tooltip that contains another widget
Christoph Feck
cfeck at kde.org
Wed Dec 21 17:28:40 UTC 2016
> On Dec. 21, 2016, 5 p.m., Christoph Feck wrote:
> > src/ktooltipwidget.h, line 72
> > <https://git.reviewboard.kde.org/r/129648/diff/5/?file=487886#file487886line72>
> >
> > I am not a native english speaker, but I think 'showBelow' sounds better.
> >
> > Needs a comment from a native speaker maybe.
> >
> > Additionally, how about adding a 'showAbove' too?
>
> Elvis Angelaccio wrote:
> Pheraps we need a different naming approach here: `showBelow` is actually "show below if there is enough space on screen", otherwise it will be shown above the rectangle. That's why I think a `showAbove` wouldn't make much sense.
>
> What about something like `showNear`?
Well, I was thinking about making it possible for the application to choose the default position (top or bottom). But you are right, the default should really be below, because the content starts at the top, and I think the name should reflect that. So unless someone corrects me, go for 'showBelow'.
- Christoph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129648/#review101534
-----------------------------------------------------------
On Dec. 21, 2016, 6:19 p.m., Elvis Angelaccio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129648/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2016, 6:19 p.m.)
>
>
> Review request for KDE Frameworks, Ben Cooksley, Christoph Feck, and Martin Gräßlin.
>
>
> Repository: kwidgetsaddons
>
>
> Description
> -------
>
> This new widget is based on the KToolTip code that is duplicated across multiple products: at least Dolphin, systemsettings, kinfocenter, ktp-contact-list.
>
> Rationale: with a single class in frameworks, it will be possible to apply features/fixes only once. See for example the comments in https://phabricator.kde.org/D3112
>
> A new feature that the old code doesn't have is the delayed hide: this makes it possible to actually use the widget shown in the tooltip.
>
>
> Diffs
> -----
>
> autotests/CMakeLists.txt ffcce3a046ec98b07c4677578f6bc997de1ef16b
> autotests/ktooltipwidgettest.h PRE-CREATION
> autotests/ktooltipwidgettest.cpp PRE-CREATION
> src/CMakeLists.txt de0a8d965f1541d5ffeec93d1aa06600b0b9c138
> src/ktooltipwidget.h PRE-CREATION
> src/ktooltipwidget.cpp PRE-CREATION
> tests/CMakeLists.txt fcb348b1ae9d4270468c3f9003c5ba5f3903db84
> tests/ktooltipwidget_test.h PRE-CREATION
> tests/ktooltipwidget_test.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/129648/diff/
>
>
> Testing
> -------
>
> Manual test works both in X11 and Wayland. Unit tests pass.
>
> Ported Dolphin locally to this new class, everything seems to work (and this will fix bug [#352276](https://bugs.kde.org/show_bug.cgi?id=352276) and [#371223](https://bugs.kde.org/show_bug.cgi?id=371223)).
>
>
> Thanks,
>
> Elvis Angelaccio
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161221/70af3036/attachment.html>
More information about the Kde-frameworks-devel
mailing list