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

Adds basic support for QSystemTrayIcon #118

Merged
merged 3 commits into from Sep 25, 2019

Conversation

@soonoo
Copy link
Contributor

soonoo commented Sep 24, 2019

Some basic functions for system tray are added. (isVisible, hide, show, setIcon)

It seems working but I'm still confused about macros. (I think QSystemTrayIcon shouldn't be yoga widget, is this right?)
I will add documentation if this PR has no problem.

Thank you!

@@ -134,3 +139,4 @@ win.show();
win.setWindowState(WindowState.WindowActive);

(global as any).win = win; // To prevent win from being garbage collected.
(global as any).systemTray = tray; // To prevent system tray from being garbage collected.

This comment has been minimized.

Copy link
@soonoo

soonoo Sep 24, 2019

Author Contributor

tray is garbage collected since QSystemTrayIcon isn't a part of window. Is there a better way to prevent grabage collection?

This comment has been minimized.

Copy link
@marcus-sa

marcus-sa Sep 24, 2019

Contributor

I don't think there is (unless you're putting it into a Map or Set), plus it's cool, because if you're creating a renderer (like React or Angular) it'd make more sense that it SHOULD be garbage collected when in a test environment (such as when using https://github.com/ng-qt/ng-test combined with https://jestjs.io, even though Jest can force exit a process that haven't finished)

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

For now lets leave it at this. I will try and come up with a better API that we can use to prevent gc on objects.
Something like React Native's AppRegistry API : https://facebook.github.io/react-native/docs/appregistry

Copy link
Collaborator

master-atul left a comment

Amazing stuff @soonoo

#include "core/NodeWidget/nodewidget.h"
#include "napi.h"

class NSystemTrayIcon: public QSystemTrayIcon, public NodeWidget

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

Hi @soonoo Great work btw!
Yes, you are absolutely right, QSystemTrayIcon doesnt need to inherit from YogaWidget.
NodeWidget inherits from both YogaWidget and EventWidget. And in the case of NSystemTrayIcon it can inherit straight from EventWidget instead of NodeWidget.

class NSystemTrayIcon: public QSystemTrayIcon, public NodeWidget
{
Q_OBJECT
NODEWIDGET_IMPLEMENTATIONS(QSystemTrayIcon)

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

Instead of NODEWIDGET_IMPLEMENTATIONS you could straightaway call
EVENTWIDGET_IMPLEMENTATIONS(QSystemTrayIcon) (This would avoid yoga properties being set on this widget.

You would still need Q_OBJECT.

public:
using QSystemTrayIcon::QSystemTrayIcon; //inherit all constructors of QSystemTrayIcon
void connectWidgetSignalsToEventEmitter() {
}

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

Maybe in a follow up PR! You could add the signals mentioned here: https://doc.qt.io/qt-5/qsystemtrayicon.html#signals

Napi::Value setIcon(const Napi::CallbackInfo& info);
Napi::Value isVisible(const Napi::CallbackInfo& info);

EVENTWIDGET_WRAPPED_METHODS_DECLARATION

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

🏅 Awesome!

@@ -134,3 +139,4 @@ win.show();
win.setWindowState(WindowState.WindowActive);

(global as any).win = win; // To prevent win from being garbage collected.
(global as any).systemTray = tray; // To prevent system tray from being garbage collected.

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

For now lets leave it at this. I will try and come up with a better API that we can use to prevent gc on objects.
Something like React Native's AppRegistry API : https://facebook.github.io/react-native/docs/appregistry

export const QSystemTrayIconEvents = Object.freeze({
...BaseWidgetEvents,
});
export class QSystemTrayIcon extends NodeWidget {

This comment has been minimized.

Copy link
@master-atul

master-atul Sep 24, 2019

Collaborator

Instead of inheriting from NodeWidget you can inherit from EventWidget instead here.
I know at this point EventWidget inherits from YogaWidget (I will fix that in a follow up PR).

So to give you a bit of background.
NodeWidget is a class that contains all the methods of QWidget.
In Qt world every widget inherits from QWidget
So similarly in NodeGUI world every widget inherits from NodeWidget.
Since QSystemTrayIcon is not a widget (ie, it doesnt inherit from QWidget in Qt world) we do not inherits from NodeWidget in nodegui world.

@master-atul master-atul referenced this pull request Sep 24, 2019
@soonoo

This comment has been minimized.

Copy link
Contributor Author

soonoo commented Sep 25, 2019

Thank you for careful reviews, @master-atul @marcus-sa.
QSystemTrayIcon now inherits EventWidget. I also changed tray icon to white colored nodegui logo.(It may vary from OS to OS but color of notification area is black in my mac, Linux machine)

@master-atul

This comment has been minimized.

Copy link
Collaborator

master-atul commented Sep 25, 2019

Amazing stuff @soonoo
Will it be possible for you to help out with the docs as well for QSystemTrayIcon ? If its not too much to ask maybe also help out in adding qt signals aswell.

I am going to merge this 👍

This closes #13

@master-atul master-atul merged commit 5080c84 into nodegui:master Sep 25, 2019
@soonoo

This comment has been minimized.

Copy link
Contributor Author

soonoo commented Sep 25, 2019

@master-atul Sure! But I've been busy these days, so give me a little time please.

@master-atul

This comment has been minimized.

Copy link
Collaborator

master-atul commented Sep 25, 2019

Yes yes. its an open source project (take your time 😄 ) and thanks again @soonoo

@dimitarnestorov dimitarnestorov referenced this pull request Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.