D22512: [Dolphin] Hide tooltip instantly on key press

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  Thanks for the patch! The fix looks correct, but I have suggestions for the coding style.

INLINE COMMENTS

> dolphinview.cpp:1427-1435
> +{
> +#ifdef HAVE_BALOO
> +    if (GeneralSettings::showToolTips()) {
> +        m_toolTipManager->hideToolTip(true);
> +    }
> +#endif
> +}

I don't like this duplicate function. We could just add the bool parameter to `DolphinView::hideToolTip()`.

> tooltipmanager.h:59
>       */
> -    void hideToolTip();
> +    void hideToolTip(bool instantly = false);
>  signals:

Please call this parameter `hideIstantly`. Bonus point if you use an enum instead of a bool ;) (see http://www.informit.com/articles/article.aspx?p=1392524)

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22512

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, pdabrowski, aprcela, fprice, fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190717/5f61860c/attachment.htm>