Skip to content

Commit

Permalink
Windows 7 Notifications: Disable Grouping (#2338)
Browse files Browse the repository at this point in the history
Grouping of notifications is not supported on Windows 7 due this bug:
electron/electron#11189

*   [x] Disable notification grouping (using `tag`) on Windows 7.
*   [x] Log notification grouping support.
*   [x] **Infrastructure:** Use 2-space indentation for all files
        (better integration with Prettier.)
  • Loading branch information
gasi-signal committed May 4, 2018
2 parents f0896b5 + 0121dc7 commit 0188abd
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 6 deletions.
2 changes: 0 additions & 2 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,4 @@ end_of_line = lf
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true

[{js/modules/**/*.js, ts/**/*.ts, test/app/**/*.js, test/modules/**/*.js}]
indent_size = 2
12 changes: 8 additions & 4 deletions js/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
const isAudioNotificationEnabled =
storage.get('audio-notification') || false;
const isAudioNotificationSupported = Settings.isAudioNotificationSupported();
const isNotificationGroupingSupported = Settings.isNotificationGroupingSupported();
const numNotifications = this.length;
const userSetting = this.getUserSetting();

Expand All @@ -50,7 +51,12 @@
userSetting,
});

console.log('Update notifications:', status);
console.log(
'Update notifications:',
Object.assign({}, status, {
isNotificationGroupingSupported,
})
);

if (status.type !== 'ok') {
if (status.shouldClearNotifications) {
Expand Down Expand Up @@ -103,13 +109,11 @@
const notification = new Notification(title, {
body: message,
icon: iconUrl,
tag: 'signal',
tag: isNotificationGroupingSupported ? 'signal' : undefined,
silent: !status.shouldPlayNotificationSound,
});

notification.onclick = () => this.onClick(last.get('conversationId'));

// We don't want to notify the user about these same messages again
this.clear();
},
getUserSetting() {
Expand Down
62 changes: 62 additions & 0 deletions ts/test/types/Settings_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,66 @@ describe('Settings', () => {
});
});
});

describe('isNotificationGroupingSupported', () => {
context('on macOS', () => {
beforeEach(() => {
sandbox.stub(process, 'platform').value('darwin');
});

afterEach(() => {
sandbox.restore();
});

it('should return true', () => {
assert.isTrue(Settings.isNotificationGroupingSupported());
});
});

context('on Windows', () => {
context('version 7', () => {
beforeEach(() => {
sandbox.stub(process, 'platform').value('win32');
sandbox.stub(os, 'release').returns('7.0.0');
});

afterEach(() => {
sandbox.restore();
});

it('should return false', () => {
assert.isFalse(Settings.isNotificationGroupingSupported());
});
});

context('version 8+', () => {
beforeEach(() => {
sandbox.stub(process, 'platform').value('win32');
sandbox.stub(os, 'release').returns('8.0.0');
});

afterEach(() => {
sandbox.restore();
});

it('should return true', () => {
assert.isTrue(Settings.isNotificationGroupingSupported());
});
});
});

context('on Linux', () => {
beforeEach(() => {
sandbox.stub(process, 'platform').value('linux');
});

afterEach(() => {
sandbox.restore();
});

it('should return true', () => {
assert.isTrue(Settings.isNotificationGroupingSupported());
});
});
});
});
5 changes: 5 additions & 0 deletions ts/types/Settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ const MIN_WINDOWS_VERSION = '8.0.0';

export const isAudioNotificationSupported = () =>
OS.isWindows(MIN_WINDOWS_VERSION) || OS.isMacOS();

// Using `Notification::tag` has a bug on Windows 7:
// https://github.com/electron/electron/issues/11189
export const isNotificationGroupingSupported = () =>
!OS.isWindows() || OS.isWindows(MIN_WINDOWS_VERSION);

0 comments on commit 0188abd

Please sign in to comment.