D23232: [dolphin] Add an action to toggle the searchbar

[Elvis Angelaccio]

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


  Patch looks good, but please note that bug #344617 also asks to make the filter button a toggle.
  So either we rename the bug report to only refer to the search button, or this commit should have `CCBUG:` rather than `BUG:`.
  
  (I think we should rename the bug report and leave the filter button as-is).

INLINE COMMENTS

> dolphinmainwindow.cpp:620-626
> +void DolphinMainWindow::toggleSearch()
> +{
> +    QAction* searchAction = actionCollection()->action(QStringLiteral("toggle_search"));
> +    m_activeViewContainer->setSearchModeEnabled(
> +        searchAction->isChecked()
> +    );
> +}

Please make `DolphinViewContainer::setSearchModeEnabled()` a `public slot` instead, so that we can just connect it to the `triggered` signal of the searchAction.

> dolphinmainwindow.cpp:1246
> +    QAction *toggleSearchAction = actionCollection()->addAction(QStringLiteral("toggle_search"));
> +    toggleSearchAction->setText(i18nc("@action:inmenu", "Toggle search bar"));
> +    toggleSearchAction->setIconText(i18nc("@action:intoolbar", "Search"));

Please use Title Capitalization: https://hig.kde.org/style/writing/capitalization.html#title-capitalization

REPOSITORY
  R318 Dolphin

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

To: iasensio, #dolphin, ngraham, #vdg, elvisangelaccio
Cc: ndavis, felixernst, elvisangelaccio, kfm-devel, aprcela, vmarinescu, fprice, MrPepe, fbampaloukas, alexde, Codezela, 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/20190825/7837baca/attachment.htm>