feat: replace deprecated NSUserNotifications with User Notifications#47817
Conversation
|
💖 Thanks for opening this pull request! 💖 Semantic PR titlesWe use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Commit signingThis repo enforces commit signatures for all incoming PRs. PR tipsThings that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
|
Waiting a few more days for all feedback, then will provide an update to incorporate. Running list thus far
|
|
Test failure
Commit Signing
Investigate using NotificationImageRetainer
|
From API WG review, the added requirement of code signing should be mentioned in the docs and added to the list of breaking changes. |
Removes deprecated NSUserNotification API, now using User Notifications
It replaces API calls for generating, scheduling, and receiving native
macOS notifications with equivalent API calls from the new framework,
or functionally equivalent implementations.
To preserve the existing Notification module API, special handling was
required in certain cases:
- Dynamically declared notification actions
Typically, notification actions should be declared at app launch time
when using the User Notifications framework. However, this isn’t
compatible with Electron’s architecture. Instead, we dynamically
declare new notifications actions when necessary and carefully manage
the existing actions registered at runtime.
- Localizations for ‘Reply’ and ‘Show’ labels
New translation files are added and processed through GRIT to add
localizations for “Reply” and “Show” button labels which were
initially supplied by the NSUserNotification framework.
8c0a5e2 to
478f9ca
Compare
There was a problem hiding this comment.
Hello @Developer-Ecosystem-Engineering! It looks like this pull request touches one of our dependency or CI files, and per our contribution policy we do not accept these types of changes in PRs.
Dismissed, a maintainer pushed these changes to the branch
samuelmaddock
left a comment
There was a problem hiding this comment.
API LGTM
In the API WG meeting, we discussed moving forward with these changes and fixing any issues that arise throughout the Electron 42 release process which sounds like a reasonable plan to me.
I also gave this a code review and it lgtm!
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Release Notes Persisted
|
This is a follow-up to electron#47817. According to https://www.electronjs.org/blog/electron-42-0#behavior-changed-macos-notifications-now-use-unnotification-api > If an application is not code-signed, notifications will emit a `failed` event on the `Notification` object.
Description of Change
Removes deprecated NSUserNotification API, now using User Notifications
It replaces API calls for generating, scheduling, and receiving native macOS notifications with equivalent API calls from the new framework, or functionally equivalent implementations.
To preserve the existing Notification module API, special handling was required in certain cases:
Dynamically declared notification actions
Localizations for ‘Reply’ and ‘Show’ labels
Checklist
npm testpassesRelease Notes
Notes: Replaced deprecated NSUserNotification with User Notifications