search

maximbaz / yubikey-touch-detector

A tool to detect when your YubiKey is waiting for a touch (to send notification or display a visual indicator on the screen)
ISC License
415 stars 31 forks source link

Add default icon to libnotify notifications #46

Closed NeiRo21 closed 11 months ago

maximbaz commented 1 year ago

Thanks for the PR! It is kinda intentional that there's no built-in icon, you can drop your favorite icon into any standard icon location, name it yubikey-touch-detector.png and it will be picked up. I am not sold yet that we need to have something more than this, and especially to replace custom icons with a single hardcoded one.

NeiRo21 commented 1 year ago

Makes sense. I think the only problem with this approach is that it's unlikely to work out of the box, so it looks like libnotify integration is broken. Besides there seems to be no mention of the setup required in the readme. What's your opinion on making notification icon configurable with the Yubico logo as a fallback?

On Mon, 2 Oct 2023, 20:57 Maxim Baz, @.***> wrote:

Thanks for the PR! It is kinda intentional that there's no built-in icon, you can drop your favorite icon into any standard icon location, name it yubikey-touch-detector.png and it will be picked up. I am not sold yet that we need to have something more than this, and especially to replace custom icons with a single hardcoded one.

— Reply to this email directly, view it on GitHub https://github.com/maximbaz/yubikey-touch-detector/pull/46#issuecomment-1743668431, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRDMDKJZNOZDA45YLQ67W3X5MMDFAVCNFSM6AAAAAA5OTD3X2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBTGY3DQNBTGE . You are receiving this because you authored the thread.Message ID: @.***>

maximbaz commented 1 year ago

What if instead of specifying a path to the icon in XDG data, we keep yubikey-touch-detector as it is today (so no change in Go code), and simply install yubico icon into /usr/share/icons/hicolor/128x128/apps/yubikey-touch-detector.png? Then it would be automatically picked up, unless people put their override in ~/.local/share/icons/.

maximbaz commented 11 months ago

Amazing, thanks so much! Looks good to me, should I merge, or is there anything else you want to do?

NeiRo21 commented 11 months ago

I think this is it for now. :)

AndrewKvalheim commented 4 months ago

@NeiRo21 Did you see the existing discussion in #25? It links to an official touch icon that is probably a better fit than the Yubico Inc. logo.

screenshot