Get icon size from QStyle instead of KIconLoader
ClosedPublic

Authored by nicolasfella on Nov 18 2019, 8:24 PM.

Details

Summary

The sizes from KIconLoader are considered to go away in KF6

Test Plan

Open archive. Icons in file view are properly sized

Diff Detail

Repository
R36 Ark
Branch
nokicon
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18918
Build 18936: arc lint + arc unit
nicolasfella created this revision.Nov 18 2019, 8:24 PM
Restricted Application added a project: Ark. · View Herald TranscriptNov 18 2019, 8:24 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald Transcript
nicolasfella requested review of this revision.Nov 18 2019, 8:24 PM

+1 but why only archivemodel.cpp ?

$ git grep KIconLoader
kerfuffle/extractiondialog.cpp:#include <KIconLoader>
kerfuffle/extractiondialog.cpp:    m_ui->iconLabel->setPixmap(QIcon::fromTheme(QStringLiteral("archive-extract")).pixmap(IconSize(KIconLoader::Desktop), IconSize(KIconLoader::Desktop)));
kerfuffle/propertiesdialog.cpp:#include <KIconLoader>
kerfuffle/propertiesdialog.cpp:    m_ui->lblIcon->setPixmap(icon.pixmap(IconSize(KIconLoader::Desktop), IconSize(KIconLoader::Desktop)));
part/archivemodel.cpp:#include <KIconLoader>
part/archivemodel.cpp:                return m_entryIcons.value(e->fullPath(NoTrailingSlash)).pixmap(IconSize(KIconLoader::Small), IconSize(KIconLoader::Small), mode);
part/arkviewer.cpp:#include <KIconLoader>
part/arkviewer.cpp:    m_iconLabel->setPixmap(QIcon::fromTheme(mimeType.iconName()).pixmap(IconSize(KIconLoader::Small), IconSize(KIconLoader::Small)));
part/infopanel.cpp:#include <KIconLoader>
part/infopanel.cpp:    return QIcon::fromTheme(name).pixmap(IconSize(KIconLoader::Desktop), IconSize(KIconLoader::Desktop));

If we also remove those, we could get rid of the KIconLoader dependency altogheter.

Can't you just return a QIcon and let the view figure it out? I don't like the model making assumptions on the visual representation

Can't you just return a QIcon and let the view figure it out? I don't like the model making assumptions on the visual representation

Good point...

Can't you just return a QIcon and let the view figure it out? I don't like the model making assumptions on the visual representation

This seems not trivially possible because of the mode argument. That can only be set in pixmap() or paint()

+1 but why only archivemodel.cpp

I thought one step at a time

elvisangelaccio accepted this revision.Dec 15 2019, 12:25 PM
This revision is now accepted and ready to land.Dec 15 2019, 12:25 PM
This revision was automatically updated to reflect the committed changes.