D12337: Give the file dialogs a "Sort by" menu button on the toolbar

[Henrik Fehlauer]

rkflx added inline comments.

INLINE COMMENTS

> kdiroperator.cpp:1888
>      d->actionCollection->addAction(QStringLiteral("sorting menu"),  sortMenu);
> +    sortMenu->setIcon(QIcon::fromTheme(QStringLiteral("itemize")));
>  

While we agreed upon wanting a better icon, until that's done I'd prefer `view-sort-ascending` instead. For me, `itemize` has no connection to sorting at all, sorry.

I'm aware my alternative shows a specific mode, but TBH I don't think users will be put off too much by this detail, in particular because it is the only sorting-related icon in the dialog.

Anyway, that's just my preference. Let me know if you think `itemize` is vastly better.

> kfilewidget.cpp:365
>      opsWidgetLayout->setSpacing(0);
> -    //d->toolbar = new KToolBar(this, true);
> -    d->toolbar = new KToolBar(d->opsWidget, true);
> +    d->toolbar = new KToolBar(this, true);
>      d->toolbar->setObjectName(QStringLiteral("KFileWidget::toolbar"));

?

> kfilewidget.cpp:365-369
> -    //d->toolbar = new KToolBar(this, true);
> -    d->toolbar = new KToolBar(d->opsWidget, true);
> +    d->toolbar = new KToolBar(this, true);
>      d->toolbar->setObjectName(QStringLiteral("KFileWidget::toolbar"));
>      d->toolbar->setMovable(false);
> -    opsWidgetLayout->addWidget(d->toolbar);

?

> kfilewidget.cpp:554-559
> +    // Tweak the look and feel of the sort menu button
> +    foreach(QToolButton* button, d->toolbar->findChildren<QToolButton*>()) {
> +        if (button->defaultAction() == coll->action(QStringLiteral("sorting menu"))) {
> +            button->setPopupMode(QToolButton::InstantPopup);
> +        }
> +    }

This also worked for me, and would avoid the `foreach`:

  KActionMenu *x = new KActionMenu(QIcon::fromTheme(QStringLiteral("configure")), i18n("Options"), this);
  x->setMenu(coll->action(QStringLiteral("sorting menu"))->menu());
  x->setDelayed(false);
  d->toolbar->addAction(x);

> kfilewidget.cpp:561
> +
> +
>      KUrlCompletion *pathCompletionObj = new KUrlCompletion(KUrlCompletion::DirCompletion);

Unintentional newline?

> kfilewidget.cpp:1410
>      boxLayout->setMargin(0); // no additional margin to the already existing
> +    boxLayout->addWidget(toolbar);
>  

?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #dolphin, #vdg, rkflx
Cc: andreaska, markg, broulik, anemeth, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180502/2b32737b/attachment-0001.html>