-
Notifications
You must be signed in to change notification settings - Fork 25
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
Only support the Windows 10 Creators Update and newer #22
Conversation
lib/notification-support.ts
Outdated
// Only Windows 10 and newer are supported | ||
return majorVersion >= 10 | ||
// Only Windows 10 (15063) and newer are supported | ||
return majorVersion >= 10 && buildNumber >= CreatorsUpdateBuildNumber |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is wrong… 🤦 it should be:
return majorVersion >= 10 && buildNumber >= CreatorsUpdateBuildNumber | |
return majorVersion > 10 || (majorVersion === 10 && buildNumber >= CreatorsUpdateBuildNumber) |
Release 0.2.3
I made a mess with the temporary release 0.2.3 😓 I made another release 0.2.4 (first pre-release, then the actual release) with the fix in the check from fca5378. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ Thanks for explaining the thoughts process behind it.
This will disable desktop-notifications for Windows versions prior to the Creators Update (see desktop/desktop-notifications#22)
This will disable desktop-notifications for Windows versions prior to the Creators Update (see desktop/desktop-notifications#22)
After investigating some issues in GitHub Desktop running on Windows Server 2016 (desktop/desktop#14714) it seems there are some limitations in Windows 10 prior to the Creators Update regarding the notifications API: https://docs.microsoft.com/en-us/uwp/api/windows.ui.notifications.toastnotification.tag?view=winrt-22621#remarks
I've changed a few things but it seems there is an important limitation in the notification payload, which makes it difficult for us to implement notifications as we'd like them.
Because of that, we're gonna support notifications from this
desktop-notifications
library on Windows 10 Creators Update and newer, while previous versions will rely on the HTML notifications API (just like W7 and W8 do nowadays, without the improvements from #13).