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

Tray icon Fix #2109

Merged
merged 4 commits into from
Feb 19, 2024
Merged

Tray icon Fix #2109

merged 4 commits into from
Feb 19, 2024

Conversation

stkrknds
Copy link
Contributor

The tray icon does not change when there are unread messages. This is due to the broken chatsIcon selector.

@devCKVargas
Copy link

devCKVargas commented Feb 2, 2024

This didn't fix the issue. How could I show the logs?

Electron 24.8.6
win32 10.0.22631
Locale: en-US

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 2, 2024

Hi @devCKVargas. Just to be sure, are you using caprine from https://github.com/stkrknds/caprine/tree/tray-icon ?
I've changed the selector again, so please try the new version and let me know if it fixes the issue. Thanks!

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 2, 2024

For me, this didn't fix the issue with CPU usage and I still had to put "electron": "^28.2.1", for my devDependencies or it just showed a white screen.

The status tray icon still didn't change when I have an unread notification and it still showed elevated usage.

git clone https://github.com/sindresorhus/caprine

cd caprine

## Pull PR ##
git pull https://github.com/sindresorhus/caprine refs/pull/2109/head

nvm use
npm install

## Start ##
npm start

nano package.json
"electron": "^28.2.1",

@devCKVargas
Copy link

Hi @devCKVargas. Just to be sure, are you using caprine from https://github.com/stkrknds/caprine/tree/tray-icon ? I've changed the selector again, so please try the new version and let me know if it fixes the issue. Thanks!

Unfortunately, still broken.
Pulled the latest commit did npm install && npm start.
Issue still exists. Tray icon didn't light up when I received new messages, same with notification.

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Thanks for your comments. To be honest, I have no idea why it is not working for you.. Changed the selector again, could you please check if it works now. Again, thanks a lot for your time.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Are you talking about the Chat icon in the Messenger App sidebar? OR the Chat Icon in the Systray/XFCE panel?

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Are you talking about the Chat icon in the Messenger App sidebar? OR the Chat Icon in the Systray/XFCE panel?

Caprine uses the Chats icon in Messenger's sidebar to update the tray icon.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

The tray icon doesn't work, but the Messenger icon does.

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

@sgtcoder Could you provide the class name of the Chats icon? You can do that by first, opening the console(pressing F12). Then Ctrl+Shift+C and click on the "Chats" icon. The element we want is the one with the aria-label attribute.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

aria-label="Chats, 1 unread"

class="x1i10hfl xjbqb8w xjqpnuy xa49m3k xqeqjp1 x2hbi6w x13fuv20 xu3j5b3 x1q0q8m5 x26u7qi x972fbf xcfux6l x1qhh985 xm0m39n x9f619 x1ypdohk xdl72j9 x2lah0s xe8uvvx x2lwn1j xeuugli x4uap5 xkhd6sd x1n2onr6 x16tdsg8 x1hl2dhg xggy1nq x1ja2u2z x1t137rt x87ps6o x1lku1pv x1a2a7pz x6s0dn4 x1q0g3np xn3w4p2 x1nn3v0j x1120s5i x1lq5wgf xgqcy7u x30kzoy x9jhf4c xdj266r x11i5rnm xat24cr x1mh8g0r x78zum5"

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

looks like there is a slight difference in the one in your code change.

So you guys have to do this like everytime Facebook pushes a new update out?

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Not saying it's a good idea, but it's not like Facebook labels ids on these, can you just do first span > a?

Or pull it from the aria-label, would just have to somehow wildcard after the "Chats"

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

looks like there is a slight difference in the one in your code change.

So you guys have to do this like everytime Facebook pushes a new update out?

Yes, unfortunately.

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Not saying it's a good idea, but it's not like Facebook labels ids on these, can you just do first span > a?

Tried to do something similar with the previous commits as it is harder to break, but that still didn't work for you, while it worked for me.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

MWCMInboxLeftRailSidebar class doesn't exist for me. Let me see if I can find something that will work, now that I know what this is for.

That's crazy you guys have to do this all the time.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Something like this

chatsIcon: '[aria-label="Inbox switcher"][aria-label^="Chats"]', // ! Tray icon dependency

but I am trying to figure out why that doesn't work.

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Something like this

chatsIcon: '[aria-label="Inbox switcher"][aria-label^="Chats"]', // ! Tray icon dependency

but I am trying to figure out why that doesn't work.

It's better to avoid using aria-label in a selector, because it will not work for every language.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Good point on that, didn't realize. OMG, I got it to work with updating the class. And no crazy CPU usage.

Now let me see if I can get it more dynamic.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

chatsIcon: '[class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] > span:nth-child(1) > a', // ! Tray icon dependency

This works too, it's not too terrible.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Notification alerts are coming in now too.

I noticed that the Messenger settings, etc aren't working either.

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

chatsIcon: '[class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] > span:nth-child(1) > a', // ! Tray icon dependency

This works too, it's not too terrible.

It doesn't work when the sidebar is expanded. I would suggest to modify this selector [data-pagelet="MWCMInboxLeftRailSidebar"] > div > div > div:nth-child(1) a by replacing [data-pagelet="MWCMInboxLeftRailSidebar"] with something that works for you. For example, something like this: [role="navigation"] > div > div > div > div > div > div:nth-child(1) a

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Notification alerts are coming in now too.

I noticed that the Messenger settings, etc aren't working either.

This is related to #2108

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Gotcha. Still working on it, but role navigation isn't quite unique

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Okay.. I modified the selector that you found, in order to work with the expanded sidebar as well. Can you test if it works on your end?

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Yes, I saw your commit and that worked. Should we do nth selector on that a tag? Either one works though.

chatsIcon: '[class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] a:nth-child(1)', // ! Tray icon dependency

@stkrknds
Copy link
Contributor Author

stkrknds commented Feb 3, 2024

Yes, I saw your commit and that worked. Should we do nth selector on that a tag? Either one works though.

chatsIcon: '[class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] a:nth-child(1)', // ! Tray icon dependency

Doesn't really matter. It picks the first one that it finds regardless. I think we can leave like this for now. Thanks for your help.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

I think also it wouldn't be bad to update some of the packages in package.json. I have a list of ones that can be updated without noticing any breaking. Or at least element-ready since if notifications break again at least the CPU won't cook.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 3, 2024

Ahh, I see, wasn't sure how that was connected in the Electron app. Thanks for clarifying and you are welcome.

Let me know if you need anything else. I am a web developer/programmer as well. Caprine is great on my build so I don't have to pull out the phone.

@devCKVargas
Copy link

devCKVargas commented Feb 4, 2024

Latest commit seemed to have fixed it including the notification. Thank you @stkrknds & @sgtcoder!

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 4, 2024

https://github.com/sgtcoder/caprine/releases/tag/v2.59.3

Here is a working version from my FORKED version with all the package dependencies updated that could be updated, with the Tray icon fix and the hidden dialog fix as well. Seems to be working well for me, I will continue to test/use.

@vietthedev
Copy link

https://github.com/sgtcoder/caprine/releases/tag/v2.59.3

Here is a working version with all the package dependencies updated that could be updated, with the Tray icon fix and the hidden dialog fix as well. Seems to be working well for me, I will continue to test/use.

Awesome. Thank you. It has been 4 months since the last PR was merged.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 4, 2024

@hlqviet But keep in mind this is under my forked copy of it. Probably good for a temp solution until main Caprine gets updated.

@vietthedev
Copy link

@hlqviet But keep in mind this is under my forked copy of it. Probably good for a temp solution until main Caprine gets updated.

There still seems to be some CPU usage while the app is idle but it's much better than before.

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 4, 2024

I do agree that there is some usage (quite low, 33 percent on 1 thread). Not as bad as Mailspring, and I wonder if the usage has always been there.

Either way, if the Caprine team can point me in the direction of looking at the resource usage of caprine components, I might be able to take a peak. My guess is maybe monitoring of certain things like notifications. Like because you would have to monitor that in the app. Like every second or 5 seconds or something?

@sgtcoder
Copy link
Contributor

sgtcoder commented Feb 5, 2024

So the other thing is the Marketplace notifications aren't working. Maybe we can pass a 2nd paramater to track this?

@mquevill
Copy link
Collaborator

So the other thing is the Marketplace notifications aren't working. Maybe we can pass a 2nd paramater to track this?

The selector [class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] a will return all 5 categories and in theory we could loop over each one and add them together. However, since the elementReady() only applies to one selector, it could be fragile to assume the other elements would be fully ready.

A useful function would be observeReadyElements() sindresorhus/element-ready#36, but it's only available in v6.1.0+ and v6.0.0 is when that package changed to an ESM package.

@sgtcoder
Copy link
Contributor

It sounds like we want to consider detecting if the sidebar is open or closed and running the specific classes based off of that. That should make it a little more effective.

@sgtcoder
Copy link
Contributor

chatsIcon: '[class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] a:nth-of-type(1)',

Hey guys, have we tried this, it's working on closed and open for me.

@sgtcoder
Copy link
Contributor

Also, you guys mentioned you only want the chat icon notifications, but I would like the Marketplace as well.

@mquevill
Copy link
Collaborator

mquevill commented Feb 17, 2024

chatsIcon: '[class="x9f619 x1n2onr6 x1ja2u2z x78zum5 xdt5ytf x2lah0s x193iq5w xdj266r"] a:nth-of-type(1)',

Hey guys, have we tried this, it's working on closed and open for me.

This change works for the expanded sidebar, but not for the collapsed one, since each a element is within a span element, so that selector can still select the first a element in each span. So yes, this selector will work, but with the implicit selection of the first match. Below is a simplified diagram of the elements.

#collapsed
div
    span
        a
    span
        a
    span
        a
    span
        a
    span
        a

#expanded
div
    a
    a
    a
    a
    a

@sgtcoder
Copy link
Contributor

Mine is working while collapsed and opened. What are you doing to test while closed and it not working?

@sgtcoder
Copy link
Contributor

Plus this just grabs the first a tag whether it's in a span or not a:nth-of-type(1)

@mquevill
Copy link
Collaborator

@sgtcoder It's not that the selector doesn't work, but it technically matches all 5 of the categories listed. The selector a:nth-of-type(1) grabs any matching a that is the first within its parent. Ideally, I'd like it to be an exact match.

image

@sgtcoder
Copy link
Contributor

Can you just have it return after the first element selection? It's going to be difficult getting just the first element only and support closed and open states.

@stkrknds
Copy link
Contributor Author

Can you just have it return after the first element selection? It's going to be difficult getting just the first element only and support closed and open states.

From https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector:

Note: The matching is done using depth-first pre-order traversal of the document's nodes starting with the first element in the document's markup and iterating through sequential nodes by order of the number of child nodes.

This means that querySelector, as it is now implemented, will always return the chats icon, which is the first matching element(depth-wise).

@sgtcoder
Copy link
Contributor

Yes, which is exactly what you wanted?

Otherwise, we might need to add a MarketPlace Icon entry and listen in on that (or add an option), which is why I just have it listening to the entire sidebar for right now on my fork. My phone is on silent 24/7, so these are important notifications too.

@stkrknds
Copy link
Contributor Author

If you want to do something similar for marketplace messages, I suggest you take a look at how the tray icon works, and in particular the following snippets:

const leftSidebar = await elementReady(`${selectors.leftSidebar}:has(${selectors.chatsIcon})`, {stopOnDomReady: false});

if (leftSidebar) {
const chatsIconObserver = new MutationObserver(async () => updateTrayIcon());
chatsIconObserver.observe(leftSidebar, {
subtree: true,
childList: true,
attributes: true,
attributeFilter: ['aria-label'],
});
}

async function updateTrayIcon(): Promise<void> {
const chatsIcon = await elementReady(selectors.chatsIcon, {
stopOnDomReady: false,
});
// Extract messageCount from ariaLabel
const messageCount = chatsIcon?.ariaLabel?.match(/\d+/g) ?? 0;
ipc.callMain('update-tray-icon', messageCount);
}

caprine/source/index.ts

Lines 418 to 421 in 91689db

// Update badge on conversations change
ipc.answerRenderer('update-tray-icon', async (messageCount: number) => {
updateBadge(messageCount);
});

caprine/source/index.ts

Lines 102 to 139 in 91689db

async function updateBadge(messageCount: number): Promise<void> {
if (!is.windows) {
if (config.get('showUnreadBadge') && !isDNDEnabled) {
app.badgeCount = messageCount;
}
if (
is.macos
&& !isDNDEnabled
&& config.get('bounceDockOnMessage')
&& previousMessageCount !== messageCount
) {
app.dock.bounce('informational');
previousMessageCount = messageCount;
}
}
if (!is.macos) {
if (config.get('showUnreadBadge')) {
tray.setBadge(messageCount > 0);
}
if (config.get('flashWindowOnMessage')) {
mainWindow.flashFrame(messageCount !== 0);
}
}
tray.update(messageCount);
if (is.windows) {
if (!config.get('showUnreadBadge') || messageCount === 0) {
mainWindow.setOverlayIcon(null, '');
} else {
// Delegate drawing of overlay icon to renderer process
updateOverlayIcon(await ipc.callRenderer(mainWindow, 'render-overlay-icon', messageCount));
}
}
}

I've never used the marketplace, so I can't help any further. I think this feature can be covered in another PR.

@dusansimic
Copy link
Collaborator

Hi everyone. Sorry for the radio silence, it's not easy at the moment to be present all the time so you'll have to excuse me for that.

Regarding the PR. I see that @mquevill has joined in to review it. Seems like most of the issues have been resolved. Is this ready for merging?

@mquevill
Copy link
Collaborator

Hi everyone. Sorry for the radio silence, it's not easy at the moment to be present all the time so you'll have to excuse me for that.

Regarding the PR. I see that @mquevill has joined in to review it. Seems like most of the issues have been resolved. Is this ready for merging?

This selector works for now for both expanded and collapsed left sidebar. There's some additional work on other notification numbers, but that will be in another PR for later.

@dusansimic dusansimic merged commit a26c6b1 into sindresorhus:main Feb 19, 2024
1 check passed
@dusansimic
Copy link
Collaborator

Thank you everyone for contributing to fixing this issue! Especially @stkrknds for doing the initial work!

@sgtcoder
Copy link
Contributor

If you want to do something similar for marketplace messages, I suggest you take a look at how the tray icon works, and in particular the following snippets:

@stkrknds

Thank you for sharing this. I will look into this and see what I can put together.

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.

6 participants