D22512: [Dolphin] Hide tooltip instantly on key press

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> pdabrowski wrote in dolphinview.cpp:1427-1435
> To be precise:
> QOverload plus still a separate overloaded DolphinView::hideToolTip() function with no arguments,
> which then calls the DolphinView::hideToolTip(...) with the default flag (in .cpp).

We can fix this issue by using a lamba ;)

  connect(m_container->horizontalScrollBar(), &QScrollBar::valueChanged, this, [=] {
      hideToolTip();
  });

> dolphinview.h:26
>  #include "dolphin_export.h"
> +#include "views/tooltips/tooltipmanager.h"
>  

Please drop this include from `dolphinview.cpp` then.

> tooltipmanager.cpp:120-123
> +            case HideToolTipFlag::Instantly:
> +                m_tooltipWidget->hide();
> +                break;
> +            case HideToolTipFlag::Later:

Coding style: `case` should not be more indented than `switch`

> tooltipmanager.h:45
>  public:
> +    enum class HideToolTipFlag {
> +        Instantly,

I'd call this enum `HideBehavior`. It's already implied from the `ToolTipManager` class that we are talking about tooltips.

> tooltipmanager.h:47
> +        Instantly,
> +        Later,
> +    };

Comma not needed

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

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, elvisangelaccio, kfm-devel, pdabrowski, aprcela, vmarinescu, 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/20190728/4cd89ccc/attachment.htm>