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
