I think it looks nicer than the kind of angry default icon and is the expected behaviour
Details
Details
- Reviewers
albertvaka - Group Reviewers
KDE Connect - Commits
- R224:732462135f56: Show no notification icon if there is no
Diff Detail
Diff Detail
- Repository
- R224 KDE Connect
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Also note there's 2 unrelated white-space changes.
| plugins/notifications/notification.cpp | ||
|---|---|---|
| 105 | QString()? | |
| plugins/notifications/notification.cpp | ||
|---|---|---|
| 105 | setIconName("none") is different to setIconName(""). | |
| plugins/notifications/notification.cpp | ||
|---|---|---|
| 105 | setIconName("none") will try to find an icon named "none". Will display nothing because there is no icon with that name. The proper way of not displaying an icon is to set it to empty string, that is QString(). | |
| plugins/notifications/notification.cpp | ||
|---|---|---|
| 105 | setIconName(QString()) gives me some default icon, but setIconName("foobar") gives me my desired behaviour (no icon at all) | |
| plugins/notifications/notification.cpp | ||
|---|---|---|
| 105 | Meh. Not your fault then. Can you at least add a comment saying so? | |
Comment Actions
Maybe just change "none" to something like "nonexistent_icon_name_so_we_display_nothing", so it's more obvious this is not a hidden feature of KNotification or something...
| plugins/notifications/notification.cpp | ||
|---|---|---|
| 105 | ||