D6542: Fixes to GrepOutputView toolbar

[Kevin Funk]

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

INLINE COMMENTS

> grepdialog.cpp:426
>      connect(m_plugin, &GrepViewPlugin::grepJobFinished, this, &GrepDialog::nextHistory);
> -    QTimer::singleShot(0, this, &GrepDialog::nextHistory);
> +    QTimer::singleShot(0, this, [=]() {nextHistory();});
>  

This doesn't really change behavior,does it?

> grepdialog.h:56
>      ///Call the next element in m_jobs_history or close the dialog if all jobs are done
> -    void nextHistory();
> +    void nextHistory(bool next = true);
>  

`nextHistory(false)` is never called(?)

> grepoutputview.cpp:75
>      m_next = new QAction(QIcon::fromTheme(QStringLiteral("go-next")), i18n("&Next Item"), this);
> -    m_next->setEnabled(false);
> -    m_collapseAll = new QAction(QIcon::fromTheme(QStringLiteral("arrow-left-double")), i18n("C&ollapse All"), this); // TODO change icon
> -    m_collapseAll->setEnabled(false);
> -    m_expandAll = new QAction(QIcon::fromTheme(QStringLiteral("arrow-right-double")), i18n("&Expand All"), this); // TODO change icon
> -    m_expandAll->setEnabled(false);
> +    m_collapseAll = new QAction(QIcon::fromTheme(QStringLiteral("menu_new")), i18n("C&ollapse All"), this);
> +    m_expandAll = new QAction(QIcon::fromTheme(QStringLiteral("description")), i18n("&Expand All"), this);

To be honest I don't think these icons are an improvement. Now they're pretty much independent (i.e. you don't clearly see they're connected, cf. collapse/expand).

I'd rather keep the old ones.

> grepoutputview.cpp:357
>  
> +void GrepOutputView::disableNavigation()
> +{

Turn that into a `updateButtonStates()` and do something along

  const bool enabled = model()->hasResults();
  m_prev->setEnabled(enabled);
  ...

Then call it from `GrepOutputView::expandElements` and other places?

REPOSITORY
  R33 KDevPlatform

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

To: croick, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170707/f4796d58/attachment.html>