Skip to content
This repository has been archived by the owner on Oct 18, 2024. It is now read-only.

Add support for custom events fired by clicking on notifications. #4

Closed
wants to merge 1 commit into from

Conversation

LukeTowers
Copy link

https://nativephp.com/docs/1/the-basics/notifications#notification-click-events says that you can send custom events when clicking on a notification. However, this code currently always sends the same static event, and furthermore can be improved by sending the payload of the click event for better handling of the click event in PHP.

This code will also benefit from a related PR to nativephp/laravel.

https://nativephp.com/docs/1/the-basics/notifications#notification-click-events says that you can send custom events when clicking on a notification. However, this code currently always sends the same static event, and furthermore can be improved by sending the payload of the click event for better handling of the click event in PHP.

This code will also benefit from a related PR to nativephp/laravel.
@@ -4,14 +4,15 @@ import {notifyLaravel} from "../utils";
const router = express.Router();

router.post('/', (req, res) => {
const {title, body} = req.body
const {title, body, customEvent: event} = req.body

Choose a reason for hiding this comment

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

Suggested change
const {title, body, customEvent: event} = req.body
const {title, body, event: customEvent} = req.body

Copy link
Author

Choose a reason for hiding this comment

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

Did you mean to also submit suggestions for line 8? And I'm not actually sure what the response from the API is for retrieving the event name provided when registering a notification.

@simonhamp
Copy link
Member

@LukeTowers I'm not sure where you're up to with this PR, but I'm going to close it as this repo is moving into the main electron package now.

If you decide to pick this back up, please feel free to PR on that repo instead.

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

Successfully merging this pull request may close these issues.

3 participants