Pros:
NSUserNotificationCenterCons:
Close #112
Note: no Growl is required :)
Will this still use macOS’es Notification Center?
Yes, it will.
How does this interact with non code signed and non “notarized” apps?
Tested on non code signed apps: Bitcoin-Core.dmg and gitian bitcoin-717f9a25f414-osx-unsigned.dmg.
How do I produce a notification on macOS this way?
02020-10-26T11:43:32Z [main] System tray properties:
12020-10-26T11:43:32Z [main] is available: yes
22020-10-26T11:43:32Z [main] supports balloon messages: yes
But sending transactions back and forth doesn’t trigger a notification. I tried making a DMG and installing from that locally.
I don’t mind making things a bit more annoying for self-compilation on macOS if it allows dropping a whole bunch of deprecated code.
How do I produce a notification on macOS this way?
But sending transactions back and forth doesn’t trigger a notification. I tried making a DMG and installing from that locally.
Tested on macOS 10.15:
Bitcoin-Core.dmg which was cross compiled on Linux Mint 20bitcoin-717f9a25f414-osx-unsigned.dmg~While compiling locally on macOS 10.15 I get an error:~
0% make deploy
1...
2+ Finalizing .dmg disk image +
3hdiutil: convert failed - Resource temporarily unavailable
4make: *** [Makefile:1279: Bitcoin-Core.dmg] Error 1
~Application has actually been installed into Applications folder, but, indeed, has no interactions with the Notification Center.~
~Is make deploy broken?~
Is make deploy broken?
No.
- Finalizing .dmg disk image + hdiutil: convert failed - Resource temporarily unavailable make: *** [Makefile:1279: Bitcoin-Core.dmg] Error 1
This error happens when you run make deploy, but there is already a Bitcoin-Core.dmg mounted.
I tested this branch and notifications seem to work fine, although I don’t remember seeing the cloud with the exclamation point before:
What are your environments (macOS version, Qt version)?
macOS 10.15.7. Qt 5.15.1.
I’ve re-tested one more time.
Is make deploy broken?
No.
Correct. make deploy works fine. It was wrong usage by me :)
To make this PR work on macOS one should run Bitcoin Core.app (installation of Bitcoin-Qt.dmg is not required).
Some screenshots:
Catalina 10.15.7 (19H2)
Big Sur 11.0 Beta (20A5395g)
Is the current notification support on macOS in master broken?
No, it isn’t.
macOS 10.15.7 with QT 5.15.1 via homebrew. I tried using Bitcoin-Qt.app rather than the DMG route and it works fine.
The tray icon is a bit weird, because nothing happens when you click on it:
That’s because trayIconActivated still has an exception for macOS. It’s a bit annoying that QT requires this tray icon to be visible in order for notifications to work, but anyway.
The exclamation mark in notifications could be avoided, by tweaking void Notificator::notifySystray:
0{
1 QSystemTrayIcon::MessageIcon sicon = QSystemTrayIcon::NoIcon;
2 switch(cls) // Set icon based on class
3 {
4#ifdef Q_OS_MAC
5 case Information: sicon = QSystemTrayIcon::NoIcon; break;
6#else
7 case Information: sicon = QSystemTrayIcon::Information; break;
8#endif
Thank you for reviewing!
The tray icon is a bit weird, because nothing happens when you click on it:
I think some behavior could be assigned to the tray icon, but it would be unusual for macOS UX, no?
The exclamation mark in notifications could be avoided, by tweaking
void Notificator::notifySystray:
Do we need to avoid it?
Given that we can’t avoid the tray icon (afaik), it would be good if it shows / hides the window. That way at least it doesn’t feel broken.
Typically on macOS these icons provide a menu with shortcuts to various functions. That might be better, if it’s not too much work:
As for the explanation mark, I would prefer to avoid it, because it’s very non standard. It could be interpreted as an error. That said, it’s not the end of the world.
Given that we can’t avoid the tray icon (afaik), it would be good if it shows / hides the window. That way at least it doesn’t feel broken.
Typically on macOS these icons provide a menu with shortcuts to various functions. That might be better, if it’s not too much work:
I’d leave adding tray icon functionality for a follow up as it will probably raise another discussion.
As for the explanation mark, I would prefer to avoid it, because it’s very non standard. It could be interpreted as an error. That said, it’s not the end of the world.
Ok, Going to patch it.
Nitpick for the notification title. Since the logo is already included its not necessary to repeat “Bitcoin Core”.
WRT the menu bar context menu options. Personally I’d like to be able to quickly start/stop syncing cause I’m not always on a consistent connection.
0Network Activity: Enabled
1Disable Network Activity
2---
3Receive
4Send
5---
6Open Bitcoin Core
@gbks took conceptual look at what the design for a menu bar app could look like. Perhaps he can give some input - it also focused on network activity switch.
– source
Can we use the system notification without a visible global menu icon?
No.
Under the hood, they are two separated frameworks. But maybe Qt combines them as one.
If a tray icon is hidden, m_statusItem == nil and QCocoaSystemTrayIcon::showMessage is noop.
This is the mac Qt code for further analysis: https://github.com/qt/qtbase/blob/81e09ae404b632a92e1e4c27f5875bdf027c5401/src/plugins/platforms/cocoa/qcocoasystemtrayicon.mm
It uses the deprecated NSUserNotificationCenter, and in Qt community I’ve found no signs of moving from NSUserNotificationCenter to UNUserNotificationCenter.
🐙 This pull request conflicts with the target branch and needs rebase.
Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.
While the menu app concept looks nice, I do think users should be able to disable it if they want, and shouldn’t lose notifications by doing so…
Only visible icon allows notifications via QSystemTrayIcon. Adding menu to that icon is not a goal of this PR.
Another approach is to use the new UNUserNotificationCenter, but I have no success to authorize bitcoin-qt for notifications.
I’ll readily close this PR in favor of any alternative one.
MacOS 10.14.6 (18G103) Qt Creator 4.13.3 Based on Qt 5.15.2 (Clang 11.0 (Apple), 64 bit) Built on Nov 13 2020 12:39:07 From revision 524cad144a
I agree with @jonasschnelli on the menu icon. Until more functionality is added to the icon - it should be omitted (imo).
rm -f src/bitcoind && rm -rf Bitcoin-Qt.app && rm -rf ~/Library/Saved\ Application\ State/org.bitcoinfoundation.Bitcoin-Qt.savedState && rm -rf ~/Library/Preferences/org.bitcoin.Bitcoin-Qt.plist && make appbundle && wait && ./Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt
Labels
Up for grabs
macOS
Milestone
22.0