Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification badge in system tray icon #261

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

alchzh
Copy link

@alchzh alchzh commented Nov 30, 2023

On Linux desktop, notification badges don't work for most people because there isn't a good standard. The library that Electron calls (libunity) is old and not many people have it. We don't have much control over the panel image either because that's controlled by the .desktop file. Personally, I find myself relying on notification badges frequently. System tray icon is easily modifiable, unlike the main panel.

This PR:

  • Adds support for unread message and notification badges in the system tray
    Image showing Vesktop and system tray unread message badge    Image showing Vesktop and system tray notification badge with number 1
  • Adds a corresponding setting and menu entry in Vesktop Settings
Image showing Vesktop settings with tray notification badge setting
  • Adds a build script to generate system tray icons with badges using badge files in static/badges. This allows users who wish to modify the default icon to get the badges as well.
  • We need to access both mainWin and tray in appBadge.ts now. Moves mainWin and tray in mainWindow.ts to a new object globals to avoid circular dependencies and reordered some imports. Can revert these changes if the require() hacks are preferred.

I am only able to test this on Linux running KDE at the moment. Would appreciate others test this on other platforms!

Can implement the badge image creation live instead of in build step in case other modifications to the tray icon are needed (voice connected, etc.)

@daliacoss
Copy link

just tested this on i3 window manager, seems to work well!

src/main/mainWindow.ts Outdated Show resolved Hide resolved
@alchzh alchzh force-pushed the integrated-tray-icon branch from 712d59f to 5d774ad Compare December 21, 2023 22:10
package.json Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
scripts/build/composeTrayIcons.mts Outdated Show resolved Hide resolved
import { mainWin } from "./mainWindow";
import { globals } from "./mainWindow";
// !!IMPORTANT!! ./appBadge import must occur after ./mainWindow
import { setBadgeCount } from "./appBadge";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider inline requiring this module to fix the circular dependency issues in a cleaner way

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised to hear casted require being called cleaner than just normal imports in a typescript context... but sure

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vendicated to confirm since we'd need the require hacks for settings as well, you'd prefer the first version over the second version below?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the current inclusion of // eslint-disable-next-line simple-import-sort/imports in ipc.ts is the roadblock on getting this merged, I'm happy to revert it to the require hacks. At this point I just want to not have to maintain my own branch on my system to keep up with updates...

src/main/mainWindow.ts Outdated Show resolved Hide resolved
src/renderer/appBadge.ts Outdated Show resolved Hide resolved
scripts/build/build.mts Outdated Show resolved Hide resolved
@Vendicated
Copy link
Member

this is pretty cool, thanks for working on this!

Can implement the badge image creation live instead of in build step in case other modifications to the tray icon are needed (voice connected, etc.)

that does sound like the most future proof / maintainable solution, i agree. just passing the arrayBuffer to the renderer, creating the image with canvas api and passing it back to node. i think that's actually also what the Discord app does

@Vendicated
Copy link
Member

why is mac unsupported anyway? can you not customise the menu bar icons?

@alchzh
Copy link
Author

alchzh commented Dec 28, 2023

why is mac unsupported anyway? can you not customise the menu bar icons?

yes but they can only contain black and alpha

@JackDotJS
Copy link

any updates on this PR? would love to see this merged cus i miss notifications a lot, and there's seemingly absolutely no other indicator for new messages atm

@DRAGONTOS

This comment was marked as spam.

- New file TrayNotificationBadgeToggle.tsx according to refactor
@alchzh
Copy link
Author

alchzh commented Mar 13, 2024

The PR was behind on some refactors in the newest versions. I've merged the changes following the refactors so it should work again now

Copy link

@ErrorNoInternet ErrorNoInternet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflict?

pnpm-lock.yaml Outdated Show resolved Hide resolved
@jakjakob
Copy link

why is mac unsupported anyway? can you not customise the menu bar icons?

macOS has it's own notification badge built in (the same in iOS), and is already used; so I think in any case it shouldn't be necessary. (I do not know the answer on your question though)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants