Initial notification icon support

[MrPenguin07]

As icon locations are platform dependent, iterate through paths & add to notification util.
Confirmed working well for linux(deb/rpm), AppImage & Windows platforms.

Todo:

• Confirm mac & flatpak icon paths are correct (i'm unable to test)
  @aaronliu0130 appreciate if you're able to test.

Before and after shots:

Linux notifications:

(bottom before, top after)

Windows:

Win.

[aaronliu0130]

Doesn't resources/128x128.png always exist regardless of system?

[MrPenguin07]

Doesn't resources/128x128.png always exist regardless of system?

In my testing a relative path to resources/128x128.png definitely doesn't handle AppImage (eg. the appimage path seemingly requires the path prefixed __appImage-x64/.), and best practice for linux package managers is to move the icons to the /usr/share/icons standard dir during install which would then break reliance on this relative path.

This could be rewritten to search for the icon - though I find that to likely be less efficient than a handful of hardcoded locations. Sometimes it's upscayl.png, others 128x128.png, sometimes can use relative path, other times can't.... so in addition to efficiency I think reliability also reduced performing a lookup for it.

edit: more info

[aaronliu0130]

Maybe it has to be a ./? I currently don't have access to my Arch laptop so I can't test it yet

[MrPenguin07]

Maybe it has to be a ./? I currently don't have access to my Arch laptop so I can't test it yet

You mean as below?

  const iconPaths = [
    "./resources/128x128.png",
  ];

As much as i'd love that simplicity to be true, confirming this doesn't work for AppImage (and I suspect others).

[aaronliu0130]

Hmm, maybe it's only an AppImage thing? AppImage/AppImageKit#1035

I'd suspect the ./ to have no actual effect here.

[MrPenguin07]

Hmm, maybe it's only an AppImage thing? AppImage/AppImageKit#1035

I'd suspect the ./ to have no actual effect here.

There very well may be an appimage variable or some such to the icon however confirming that resources/128x128.png will not work it would require an extra path in the array regardless.
The path prefixed with __appImage-x64 works - and is unlikely to change? - so may not be worth the headache looking further :)

Can confirm the deb/rpm work simply with resources/128x128.png however again i'd hesitate to rely on that, package managers may not retain an otherwise redundant icon (especially since we fixed #680 ) in future.
So an extra path to the freedesktop icon spec location under /usr/share/icons is wise.

Windows I believe does use resources/128x128.png.

So this justifies my thought process in hardcoding a handful of paths, while not my favorite bit of code it oddly seems the simplest and can't imagine these paths changing anytime soon.

The flatpak and mac builds i'm curious about, would appreciate feedback from someone who can test these.

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you are still working on this, please reply to indicate so.

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you are still working on this, please reply to indicate so.

[MrPenguin07]

So would appreciate some input on this;
What exactly needs be done to merge this?
Needs more emoji?!

I've confirmed this PR works on majority of builds - even if it didn't work, for eg., on the mac build the notification would still show same as without the PR.

[NayamAmarshe]

So would appreciate some input on this; What exactly needs be done to merge this? Needs more emoji?!

I've confirmed this PR works on majority of builds - even if it didn't work, for eg., on the mac build the notification would still show same as without the PR.

The PR was in draft, I thought you were still working on it.

[NayamAmarshe]

LGTM! We just need to update one thing :)

[MrPenguin07]

The PR was in draft, I thought you were still working on it.

Just hoped someone could test it for mac/flatpak :)
Cheers

[aaronliu0130]

Continuing discussion in b2a3e54