D8450: User can now hide an entire places group from KFilePlacesView

[Milian Wolff]

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


  please use arc, or add more context to your patches in the future

INLINE COMMENTS

> kfileplacesview.cpp:298
> +
> +    m_disappearingItems.reserve(m_disappearingItems.count() + indexesGroup.count());
> +    for (const QModelIndex &idx : indexesGroup) {

the reserve + loop should be the same as doing

  m_disappearingItems += indexesGroup;

> kfileplacesview.cpp:431
> +    const KFilePlacesModel *placesModel = static_cast<const KFilePlacesModel *>(index.model());
> +    const QString category = index.data(KFilePlacesModel::GroupRole).toString() + (placesModel->isPlaceGroupHidden(index) ? i18n(" (hidden)") : QString());
> +

i18n puzzle, do something like this:

  const QString groupLabel = index.data(KFilePlacesModel::GroupRole).toString();
  const bool groupHidden = placesModel->isPlaceGroupHidden(index);
  const QString category = groupHidden ? i18n("%1 (hidden)", groupLabel) : groupLabel;

> kfileplacesview.cpp:861
> +
> +        if (!d->showAll && hideSection->isChecked()) {
> +            delegate->addDisappearingItemGroup(index);

shouldn't we disable the `hideSection` when `d->showAll` is in effect? makes no sense to hide something when it has no visual effect, no?

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

To: mlaurent, ngraham, renatoo, franckarrecot, ervin, mwolff
Cc: mwolff, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171115/215c02a0/attachment.html>