Differential D23196

have 1024x1024 app icon as fallback icon in Snore
ClosedPublic
Actions

Authored by brute4s99 on Aug 16 2019, 2:43 AM.

Details

Reviewers
broulik
sredman
Group Reviewers
Frameworks
Commits
R289:e96bf17d8cac: have 1024x1024 app icon as fallback icon in Snore
Summary

by this patch, apps with a set WindowIcon will have that icon as fallback icon for their notifications invoked through NotifyBySnore backend for KNotifications on Windows 10.

Diff Detail

Repository
R289 KNotifications
Branch
arcpatch-D23196
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15348
Build 15366: arc lint + arc unit
brute4s99 created this revision.Aug 16 2019, 2:43 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptAug 16 2019, 2:43 AM
brute4s99 requested review of this revision.Aug 16 2019, 2:43 AM
broulik added a subscriber: broulik.Aug 16 2019, 6:46 AM
broulik added inline comments.
src/notifybysnore.cpp
159

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

161

Check for isNull()

163

The explicit call to actualSize shouldn't be neccessary

165

Whitespace

brute4s99 marked 3 inline comments as done.Aug 16 2019, 10:03 PM
brute4s99 added inline comments.
src/notifybysnore.cpp
163

just to be sure :)

but if there's a check implemented already somewhere within QIcon, I'd happily remove it from here!

sredman accepted this revision.Aug 18 2019, 3:56 AM
sredman added a subscriber: sredman.

Since KNotifications on Windows is so far only used by you anyway, I say it's fine to merge. @broulik, any objections?

src/notifybysnore.cpp
364

Whitespace

367

Indentation

This revision is now accepted and ready to land.Aug 18 2019, 3:56 AM
brute4s99 updated this revision to Diff 63981.Aug 18 2019, 7:06 PM
brute4s99 marked 2 inline comments as done.Aug 18 2019, 7:07 PM
brute4s99 updated this revision to Diff 63984.Aug 18 2019, 7:14 PM
brute4s99 added a comment.Aug 19 2019, 8:50 PM

may I land this now? 👀

brute4s99 updated this revision to Diff 64114.Aug 20 2019, 12:58 PM
brute4s99 updated this revision to Diff 64115.Aug 20 2019, 1:12 PM
brute4s99 retitled this revision from have the app icon as fallback icon in Snore to have 1024x1024 app icon as fallback icon in Snore.

removed actualSize in favour of always passing an icon of size 1024x1024 to SnoreToast

broulik accepted this revision.Aug 20 2019, 1:13 PM
brute4s99 updated this revision to Diff 64119.Aug 20 2019, 1:18 PM

changed line endings to Unix style

Closed by commit R289:e96bf17d8cac: have 1024x1024 app icon as fallback icon in Snore (authored by brute4s99). · Explain WhyAug 20 2019, 1:19 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/notifybysnore.cpp (26 lines)

Diff 64119

View Options

src/notifybysnore.cpp

Loading...