D8454: Make Saved Search feature discoverable

[Elvis Angelaccio]

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


  > I have one question: right now I seem to be able to compile and run this code just fine without having #include-ed QAction anywhere, even though I'm using it. Is this... expected?
  
  Yes, the QAction header might be included indirectly by something else.

INLINE COMMENTS

> dolphinsearchbox.cpp:32
>  #include <KNS3/KMoreToolsMenuFactory>
> +#include <KIO/JobUiDelegate>
>  

seems unused

> dolphinsearchbox.cpp:265
>      m_startedSearching = false;
> +    m_saveSearchAction->setEnabled(false);
>      emit closeRequest();

Do we really need to disable the action? If the searchbox is not visible we don't really care, no?

> dolphinsearchbox.cpp:310
> +{
> +    QUrl searchURL = urlForSearching();
> +    if (searchURL.isValid()) {

const

> dolphinsearchbox.cpp:383
> +    // Add "Save search" button inside search box
> +    m_saveSearchAction = new QAction();
> +    m_saveSearchAction->setIcon (QIcon::fromTheme(QStringLiteral("document-save-symbolic")));

Missing `this` as parent (it will leak otherwise)

REPOSITORY
  R318 Dolphin

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

To: ngraham, #dolphin, #kde_applications, broulik, dfaure, markg, emateli, elvisangelaccio
Cc: markg, emateli, elvisangelaccio, cfeck, #dolphin, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20171026/12238d06/attachment.htm>