D23196: have the app icon as fallback icon in Snore

Kai Uwe Broulik noreply at phabricator.kde.org
Fri Aug 16 07:47:01 BST 2019


broulik added inline comments.

INLINE COMMENTS

> notifybysnore.cpp:159
> +    } else {
> +        Qstring iconPath = QString(m_iconDir.path() + QLatin1Char('/')
> +                            + QString::number(notification->id()) + QStringLiteral(".png"));

Did you even compile this?
Also, this is the same in both branches, move it above the `if` and also make it `const`

> notifybysnore.cpp:161
> +                            + QString::number(notification->id()) + QStringLiteral(".png"));
> +        QIcon app_icon = qApp->windowIcon();
> +    // We limit the icon size to 1024x1024 as it is the highest supported by Windows

Check for `isNull()`

> notifybysnore.cpp:163
> +    // We limit the icon size to 1024x1024 as it is the highest supported by Windows
> +        QPixmap pixmap = app_icon.pixmap(app_icon.actualSize(QSize(1024, 1024)));
> +        pixmap.save(iconPath, "PNG");

The explicit call to `actualSize` shouldn't be neccessary

> notifybysnore.cpp:165
> +        pixmap.save(iconPath, "PNG");
> +        arguments   << QStringLiteral("-p") << iconPath;
>      }

Whitespace

REPOSITORY
  R289 KNotifications

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

To: brute4s99, #frameworks
Cc: broulik, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190816/9c43eb1a/attachment.html>


More information about the Kde-frameworks-devel mailing list