Skip to content

Commit 2485f3b

Browse files
authored
fix: handle notification timeout correctly when set to 0 (#16849)
- Fix timeout logic to properly handle explicit timeout of 0 - Change condition from falsy check to undefined check to preserve 0 value if set explicitly - Add unit tests for getTimeout method covering various timeout scenarios - Add sample commands demonstrating persistent and vanishing notifications Contributed on behalf of STMicroelectronics
1 parent 4fedbe0 commit 2485f3b

File tree

3 files changed

+137
-1
lines changed

3 files changed

+137
-1
lines changed

examples/api-samples/src/browser/menu/sample-menu-contribution.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ const SampleSelectDialog: Command = {
5858
id: 'sample-command-select-dialog',
5959
label: 'Sample Select Component Dialog'
6060
};
61+
const SamplePersistentNotification: Command = {
62+
id: 'sample-persistent-notification',
63+
label: 'Sample Persistent Notification (No Timeout)'
64+
};
65+
const SampleVanishingNotification: Command = {
66+
id: 'sample-vanishing-notification',
67+
label: 'Sample Vanishing Notification (500ms Timeout)'
68+
};
6169

6270
@injectable()
6371
export class SampleCommandContribution implements CommandContribution {
@@ -218,6 +226,22 @@ export class SampleCommandContribution implements CommandContribution {
218226
});
219227
}
220228
});
229+
commands.registerCommand(SamplePersistentNotification, {
230+
execute: () => {
231+
this.messageService.info(
232+
'This notification will stay visible until you dismiss it manually.',
233+
{ timeout: 0 }
234+
);
235+
}
236+
});
237+
commands.registerCommand(SampleVanishingNotification, {
238+
execute: () => {
239+
this.messageService.info(
240+
'This notification will stay visible for 500ms.',
241+
{ timeout: 500 }
242+
);
243+
}
244+
});
221245
}
222246

223247
}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// *****************************************************************************
2+
// Copyright (C) 2026 STMicroelectronics and others.
3+
//
4+
// This program and the accompanying materials are made available under the
5+
// terms of the Eclipse Public License v. 2.0 which is available at
6+
// http://www.eclipse.org/legal/epl-2.0.
7+
//
8+
// This Source Code may also be made available under the following Secondary
9+
// Licenses when the conditions for such availability set forth in the Eclipse
10+
// Public License v. 2.0 are satisfied: GNU General Public License, version 2
11+
// with the GNU Classpath Exception which is available at
12+
// https://www.gnu.org/software/classpath/license.html.
13+
//
14+
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
15+
// *****************************************************************************
16+
17+
import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
18+
let disableJSDOM = enableJSDOM();
19+
import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
20+
FrontendApplicationConfigProvider.set({});
21+
22+
import { expect } from 'chai';
23+
import { Message } from '@theia/core/lib/common';
24+
import { NotificationManager } from './notifications-manager';
25+
import { NotificationPreferences } from '../common/notification-preferences';
26+
import { NotificationContentRenderer } from './notification-content-renderer';
27+
28+
disableJSDOM();
29+
30+
describe('NotificationManager', () => {
31+
32+
const DEFAULT_TIMEOUT = 30000;
33+
34+
class TestableNotificationManager extends NotificationManager {
35+
public testGetTimeout(plainMessage: Message): number {
36+
return this.getTimeout(plainMessage);
37+
}
38+
}
39+
40+
function createNotificationManager(preferenceTimeout: number = DEFAULT_TIMEOUT): TestableNotificationManager {
41+
const manager = new TestableNotificationManager();
42+
(manager as unknown as { preferences: Partial<NotificationPreferences> }).preferences = {
43+
'notification.timeout': preferenceTimeout
44+
};
45+
(manager as unknown as { contentRenderer: NotificationContentRenderer }).contentRenderer = new NotificationContentRenderer();
46+
return manager;
47+
}
48+
49+
before(() => {
50+
disableJSDOM = enableJSDOM();
51+
});
52+
53+
after(() => {
54+
disableJSDOM();
55+
});
56+
57+
describe('getTimeout', () => {
58+
59+
let manager: TestableNotificationManager = createNotificationManager();
60+
61+
it('should return preference timeout when no options are provided', () => {
62+
const message: Message = { text: 'Test message' };
63+
expect(manager.testGetTimeout(message)).to.equal(DEFAULT_TIMEOUT);
64+
});
65+
66+
it('should return preference timeout when options are provided without timeout', () => {
67+
const message: Message = { text: 'Test message', options: {} };
68+
expect(manager.testGetTimeout(message)).to.equal(DEFAULT_TIMEOUT);
69+
});
70+
71+
it('should return explicit timeout when provided', () => {
72+
const message: Message = { text: 'Test message', options: { timeout: 5000 } };
73+
expect(manager.testGetTimeout(message)).to.equal(5000);
74+
});
75+
76+
it('should return 0 when timeout is explicitly set to 0', () => {
77+
const message: Message = { text: 'Test message', options: { timeout: 0 } };
78+
expect(manager.testGetTimeout(message)).to.equal(0);
79+
});
80+
81+
it('should return 0 when actions are present regardless of timeout', () => {
82+
const message: Message = { text: 'Test message', actions: ['OK'], options: { timeout: 5000 } };
83+
expect(manager.testGetTimeout(message)).to.equal(0);
84+
});
85+
86+
it('should return 0 when actions are present and no timeout is set', () => {
87+
const message: Message = { text: 'Test message', actions: ['OK', 'Cancel'] };
88+
expect(manager.testGetTimeout(message)).to.equal(0);
89+
});
90+
91+
it('should return negative timeout when explicitly set', () => {
92+
const message: Message = { text: 'Test message', options: { timeout: -1 } };
93+
expect(manager.testGetTimeout(message)).to.equal(-1);
94+
});
95+
96+
it('should return explicit timeout even if custom preference timeout is available', () => {
97+
const customTimeout = 60000;
98+
manager = createNotificationManager(customTimeout);
99+
const message: Message = { text: 'Test message', options: { timeout: 5000 } };
100+
expect(manager.testGetTimeout(message)).to.equal(5000);
101+
});
102+
103+
it('should return custom preference timeout if no timeout is set', () => {
104+
const customTimeout = 60000;
105+
manager = createNotificationManager(customTimeout);
106+
const message: Message = { text: 'Test message' };
107+
expect(manager.testGetTimeout(message)).to.equal(customTimeout);
108+
});
109+
110+
});
111+
112+
});

packages/messages/src/browser/notifications-manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ export class NotificationManager extends MessageClient {
229229
// Ignore the timeout if at least one action is set, and we wait for user interaction.
230230
return 0;
231231
}
232-
return plainMessage.options && plainMessage.options.timeout || this.preferences['notification.timeout'];
232+
return plainMessage.options?.timeout !== undefined ? plainMessage.options.timeout : this.preferences['notification.timeout'];
233233
}
234234
protected isExpandable(message: string, source: string | undefined, actions: string[]): boolean {
235235
if (!actions.length && source) {

0 commit comments

Comments
 (0)