Differential D28194 Diff 78197 plugin/kquickstyleitem.cpp

Changeset View

Standalone View

View Options

plugin/kquickstyleitem.cpp

Show First 20 Lines • Show All 204 Lines • ▼ Show 20 Line(s)185{
205 switch (m_itemType) { 205​ switch (m_itemType) {
206 case Button: { 206​ case Button: {
207 if (!m_styleoption) 207​ if (!m_styleoption)
208 m_styleoption = new QStyleOptionButton(); 208​ m_styleoption = new QStyleOptionButton();
209 209​
210 QStyleOptionButton *opt = qstyleoption_cast<QStyleOptionButton*>(m_styleoption); 210​ QStyleOptionButton *opt = qstyleoption_cast<QStyleOptionButton*>(m_styleoption);
211 opt->text = text(); 211​ opt->text = text();
212 212​
213 const QVariant icon = m_properties[QStringLiteral("icon")]; 213​ const QVariant icon = m_properties[QStringLiteral("icon")];
ervinUnsubmitted
Not Done

Note that this construct is duplicated three times in that file. It really calls for having a findIcon like that:

QIcon KQuickStyleItem::findIcon(const QVariant &iconProp) const
{
    if (iconProp.canConvert<QIcon>()) {
        return iconProp.value<QIcon>();
    } else if (iconProp.canConvert<QString>()) {
        const auto iconId = iconProp.toString();
        if (iconId.startsWith(QLatin1Char('/')) || iconId.startsWith(QStringLiteral(":/"))) {
            return QIcon(iconId);
        } else if (iconId.startsWith(QStringLiteral("file:"))) {
            return QIcon(QUrl(iconId).toLocalFile());
        } else if (iconId.startsWith(QStringLiteral("qrc:"))) {
            return QIcon(QLatin1Char(':') + QUrl(iconId).path());
        } else {
            return m_theme->iconFromTheme(iconId, m_properties[QStringLiteral("iconColor")].value<QColor>());
        }
    } else {
        return QIcon();
    }
}

And then using it for setting opt->icon and opt->currentIcon (combobox case). Yes, I did that locally and that definitely helped.

ervin: Note that this construct is duplicated three times in that file. It really calls for having a…
214 if (icon.canConvert<QIcon>()) { 214​ if (icon.canConvert<QIcon>()) {
215 opt->icon = icon.value<QIcon>(); 215​ opt->icon = icon.value<QIcon>();
216 } else if (icon.canConvert<QUrl>() && icon.value<QUrl>().isLocalFile()) { 216​ } else if (icon.canConvert<QUrl>() && icon.value<QUrl>().isLocalFile()) {
217 opt->icon = QIcon(icon.value<QUrl>().toLocalFile()); 217​ opt->icon = QIcon(icon.value<QUrl>().toLocalFile());
218​ } else if (icon.canConvert<QUrl>() && icon.value<QUrl>().scheme() == QLatin1String("qrc")) {
martUnsubmitted
Done

should it also catch the ":/foo" case?

mart: should it also catch the ":/foo" case?
219​ const QString iconUrl = icon.value<QString>().mid(3);
220​ opt->icon = QIcon(iconUrl);
218 } else if (icon.canConvert<QString>()) { 221​ } else if (icon.canConvert<QString>()) {
219 opt->icon = m_theme->iconFromTheme(icon.value<QString>(), m_properties[QStringLiteral("iconColor")].value<QColor>()); 222​ opt->icon = m_theme->iconFromTheme(icon.value<QString>(), m_properties[QStringLiteral("iconColor")].value<QColor>());
220 } 223​ }
221 auto iconSize = QSize(m_properties[QStringLiteral("iconWidth")].toInt(), m_properties[QStringLiteral("iconHeight")].toInt()); 224​ auto iconSize = QSize(m_properties[QStringLiteral("iconWidth")].toInt(), m_properties[QStringLiteral("iconHeight")].toInt());
222 if (iconSize.isEmpty()) { 225​ if (iconSize.isEmpty()) {
223 int e = KQuickStyleItem::style()->pixelMetric(QStyle::PM_ButtonIconSize, m_styleoption, nullptr); 226​ int e = KQuickStyleItem::style()->pixelMetric(QStyle::PM_ButtonIconSize, m_styleoption, nullptr);
224 if (iconSize.width() <= 0) { 227​ if (iconSize.width() <= 0) {
225 iconSize.setWidth(e); 228​ iconSize.setWidth(e);
▲ Show 20 Lines • Show All 1659 Lines • Show Last 20 Lines