Skip to content

Commit

Permalink
Upgrade NW.js and remove freedesktop notifications
Browse files Browse the repository at this point in the history
Upgrade to 0.26.1 and use Chromium's native implementation instead,
which will work on all dbus session bus connections (abstract-socket).

TODO:
Consider using chrome.notifications instead of the Notification DOM API,
so that the contextMessage can be removed (streamlink-twitch-gui).
  • Loading branch information
bastimeyer committed Oct 26, 2017
1 parent 5290769 commit 0a0ca0f
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 776 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
},
"dependencies": {
"bootstrap": "3.3.1",
"dbus-native": "github:sidorares/node-dbus#9b464a1c99ce149dd660e1e88382318d248dff60",
"ember-localstorage-adapter": "1.0.0",
"flag-icon-css": "2.8.0",
"font-awesome": "4.7.0",
Expand Down
4 changes: 2 additions & 2 deletions src/app/initializers/localstorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ function upgradeSettings() {
});

// update notification provider
if ( settings.notify_provider === "libnotify" ) {
settings.notify_provider = "freedesktop";
if ( [ "libnotify", "freedesktop" ].includes( settings.notify_provider ) ) {
settings.notify_provider = "native";
}

// map quality number IDs to strings
Expand Down
8 changes: 0 additions & 8 deletions src/app/models/localstorage/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,14 +241,6 @@ export default Model.extend({
notes: "\"Banner notifications\" need to be enabled in the system preferences"
}
},
{
value: "freedesktop",
label: {
name: "Freedesktop notifications",
description: "Native notifications on Linux",
notes: "Implementation of the Desktop Notifications Specification"
}
},
{
value: "growl",
label: {
Expand Down
186 changes: 0 additions & 186 deletions src/app/services/NotificationService/providers/freedesktop.js

This file was deleted.

2 changes: 0 additions & 2 deletions src/app/services/NotificationService/providers/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import NotificationProviderAuto from "./auto";
import NotificationProviderSnoreToast from "./snoretoast";
import NotificationProviderNative from "./native";
import NotificationProviderFreedesktop from "./freedesktop";
import NotificationProviderGrowl from "./growl";
import NotificationProviderRich from "./rich";

Expand Down Expand Up @@ -35,7 +34,6 @@ export default {
"native": NotificationProviderNative,
// helper providers for native notifications
"snoretoast": NotificationProviderSnoreToast,
"freedesktop": NotificationProviderFreedesktop,
// non-native providers
"growl": NotificationProviderGrowl,
"rich": NotificationProviderRich
Expand Down
19 changes: 9 additions & 10 deletions src/app/services/NotificationService/providers/native.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import {
window as Window
} from "nwjs/Window";
import { isDarwin } from "utils/node/platform";
import {
isDarwin,
isLinux
} from "utils/node/platform";


const { Notification } = Window;
Expand All @@ -27,19 +30,12 @@ const { Notification } = Window;
* since Chromium 61 (NWjs 0.25) behind a feature flag
* https://crbug.com/676220
*
* Issues:
* - "settings" action button can't be disabled
* Clicking the settings action will open a new NWjs window that tries to load Chromium's
* settings page, which doesn't exist in NWjs and is therefore empty.
* This is "acceptable" on macOS (native notifications would require 3rd party applications),
* but on Linux, we have the freedesktop provider that can set the correct actions.
*
* @class NotificationProviderNative
* @implements NotificationProvider
*/
export default class NotificationProviderNative {
static isSupported() {
return isDarwin;
return isDarwin || isLinux;
}

async setup() {}
Expand All @@ -56,7 +52,10 @@ export default class NotificationProviderNative {
actions: []
});

notification.addEventListener( "click", () => data.click() );
notification.addEventListener( "click", () => data.click instanceof Function
? data.click()
: null
);
notification.addEventListener( "show", () => resolve() );
notification.addEventListener( "error", () =>
reject( new Error( "Could not show notification" ) )
Expand Down
2 changes: 1 addition & 1 deletion src/config/main.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"display-name": "Streamlink Twitch GUI",
"app-identifier": "streamlink-twitch-gui",
"nwjs-version": "0.25.3",
"nwjs-version": "0.26.1",
"urls": {
"homepage": "https://github.com/streamlink/streamlink-twitch-gui",
"release": "https://github.com/streamlink/streamlink-twitch-gui/releases/tag/v{version}",
Expand Down
Loading

0 comments on commit 0a0ca0f

Please sign in to comment.