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

new feature: synchronizes all system notifications with the browser notifications #829

Closed
mahdi00021 opened this issue Feb 25, 2023 · 5 comments

Comments

@mahdi00021
Copy link

mahdi00021 commented Feb 25, 2023

@andersevenrud
Hello, we have added a new feature to the system which synchronizes all system notifications with the browser. If you are interested, we would like to send a pull request as a new feature to be merged into the main system.

File Notification.js is in path:
src/client

This file called in src/client/index.js

screenshot of test in os-js:
in windows and ubuntu

push-hafez-with-browser

push-hafez-with-browser-ubuntu

@andersevenrud
Copy link
Member

I'm not really sure if there's a use-case for this to be provided by default.

But if you create a service provider package (npm compatible) and provide the full source I will gladly add it to the documentation.

@mahdi00021
Copy link
Author

There is no need for a service provider, here you only need a file for notification that is written with JavaScript and called in the index.js
Also, a listener is used in the index file to listen to create notifications

@mahdi00021
Copy link
Author

mahdi00021 commented Feb 25, 2023

this file called in index.js and in

osjs.on('osjs/notification:create', (notification) => {
		   let message = notification.options.message;
		   
		   sendNotification(notification.options.title,message,'#'); 
		   console.log('notificationnotificationnotification',message);
		  
	});

this is Notification.js:

export default function sendNotification(title, message, url) {
    //First, check notification is supported or not in your browser!
    if (!Notification) {
      console.log('Push notifications are not supported in your browser..');
      return;
    }

    //Ask for permission from user 
    if (Notification.permission !== "granted") {
      Notification.requestPermission();
    } else {
      var notification = new Notification(title, {
        icon: 'favicon.png',
        body: message,
      });

      // Remove the notification when clicked and open the URL.
      notification.onclick = function() {
      
      };

      // Callback function when the notification is closed.
      notification.onclose = function() {
        console.log('Notification closed');
      };
    }
}

@andersevenrud
Copy link
Member

Looks like this is actually creating native notifications. I thought by "synchronize" that it was the other way around.

Creating native notifications is actually already supported globally with configuration notifications.native or the native option in the service API.

If you want the option to do both, simply publish a PR for the client with changes to the Notification to allow that :)

@andersevenrud
Copy link
Member

Yes, you can open a Pull Request to add a feature to support both "OS.js notifications" and "Native notifications" at the same time.

But just to make sure we're talking about the same thing here; this needs to be implemented in the actual client module.

I.e.: https://github.com/os-js/osjs-client/blob/master/src/notification.js where existing functionality exists.

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

No branches or pull requests

2 participants