Skip to content

Commit aafa2ca

Browse files
authored
refactor(app): Cherry-pick support notifications in secondary windows (#20040) (#20080)
1 parent 658fbd9 commit aafa2ca

File tree

6 files changed

+38
-31
lines changed

6 files changed

+38
-31
lines changed

app-shell/src/notifications/__tests__/notifications.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('registerNotify', () => {
3838
it('should set browser window when connectionStore has no browser window', () => {
3939
registerNotify(dispatch, mainWindow as any)(MOCK_ACTION)
4040

41-
expect(connectionStore.setBrowserWindow).toHaveBeenCalledWith(mainWindow)
41+
expect(connectionStore.addBrowserWindow).toHaveBeenCalledWith(mainWindow)
4242
})
4343

4444
it('should subscribe when action type is shell:NOTIFY_SUBSCRIBE', () => {

app-shell/src/notifications/__tests__/store.test.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { connectionStore } from '../store'
44

55
const MOCK_IP = 'MOCK_IP'
66
const MOCK_ROBOT = 'MOCK_ROBOT'
7-
const MOCK_WINDOW = {} as any
7+
const MOCK_WINDOW = { once: () => {} } as any
88
const MOCK_CLIENT = { connected: true } as any
99
const MOCK_TOPIC = 'MOCK_TOPIC' as any
1010

@@ -13,10 +13,12 @@ describe('ConnectionStore', () => {
1313
connectionStore.clearStore()
1414
})
1515

16-
describe('getBrowserWindow', () => {
17-
it('should return the browser window', () => {
18-
connectionStore.setBrowserWindow(MOCK_WINDOW)
19-
expect(connectionStore.getBrowserWindow()).toBe(MOCK_WINDOW)
16+
describe('getBrowserWindows', () => {
17+
it('should return a set containing the browser window', () => {
18+
connectionStore.addBrowserWindow(MOCK_WINDOW)
19+
const windows = connectionStore.getBrowserWindows()
20+
expect(windows).toBeInstanceOf(Set)
21+
expect(windows.has(MOCK_WINDOW)).toBe(true)
2022
})
2123
})
2224

@@ -92,10 +94,11 @@ describe('ConnectionStore', () => {
9294
})
9395
})
9496

95-
describe('setBrowserWindow', () => {
96-
it('should set the browser window', () => {
97-
connectionStore.setBrowserWindow(MOCK_WINDOW)
98-
expect(connectionStore.getBrowserWindow()).toBe(MOCK_WINDOW)
97+
describe('addBrowserWindow', () => {
98+
it('should add the browser window to the set', () => {
99+
connectionStore.addBrowserWindow(MOCK_WINDOW)
100+
const windows = connectionStore.getBrowserWindows()
101+
expect(windows.has(MOCK_WINDOW)).toBe(true)
99102
})
100103
})
101104

@@ -211,12 +214,12 @@ describe('ConnectionStore', () => {
211214
describe('clearStore', () => {
212215
it('should clear all connections and robot names', async () => {
213216
await connectionStore.setPendingConnection(MOCK_ROBOT)
214-
connectionStore.setBrowserWindow(MOCK_WINDOW)
217+
connectionStore.addBrowserWindow(MOCK_WINDOW)
215218
expect(connectionStore.getAllBrokersInStore()).not.toEqual([])
216-
expect(connectionStore.getBrowserWindow()).not.toBeNull()
219+
expect(connectionStore.getBrowserWindows().size).toBeGreaterThan(0)
217220
connectionStore.clearStore()
218221
expect(connectionStore.getAllBrokersInStore()).toEqual([])
219-
expect(connectionStore.getBrowserWindow()).toBeNull()
222+
expect(connectionStore.getBrowserWindows().size).toBe(0)
220223
})
221224
})
222225

app-shell/src/notifications/deserialize.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,12 @@ export function sendDeserialized({
2828
message,
2929
}: SendToBrowserParams): void {
3030
try {
31-
const browserWindow = connectionStore.getBrowserWindow()
32-
browserWindow?.webContents.send('notify', ip, topic, message)
31+
const browserWindows = connectionStore.getBrowserWindows()
32+
browserWindows.forEach(window => {
33+
if (!window.isDestroyed()) {
34+
window.webContents.send('notify', ip, topic, message)
35+
}
36+
})
3337
} catch {} // Prevents shell erroring during app shutdown event.
3438
}
3539

app-shell/src/notifications/index.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,9 @@ import type { RobotData } from './connect'
2222

2323
export function registerNotify(
2424
dispatch: Dispatch,
25-
mainWindow: BrowserWindow
25+
window: BrowserWindow
2626
): (action: Action) => unknown {
27-
if (connectionStore.getBrowserWindow() == null) {
28-
connectionStore.setBrowserWindow(mainWindow)
29-
}
27+
connectionStore.addBrowserWindow(window)
3028

3129
return function handleAction(action: Action) {
3230
switch (action.type) {

app-shell/src/notifications/store.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,20 @@ class ConnectionStore {
2222

2323
private robotNamesByIP: Record<string, string> = {}
2424

25-
private browserWindow: BrowserWindow | null = null
25+
private readonly browserWindows = new Set<BrowserWindow>()
2626

2727
private readonly knownPortBlockedIPs = new Set<string>()
2828

29-
public getBrowserWindow(): BrowserWindow | null {
30-
return this.browserWindow
29+
public getBrowserWindows(): Set<BrowserWindow> {
30+
return this.browserWindows
31+
}
32+
33+
public addBrowserWindow(window: BrowserWindow): void {
34+
this.browserWindows.add(window)
35+
36+
window.once('closed', () => {
37+
this.browserWindows.delete(window)
38+
})
3139
}
3240

3341
public getAllBrokersInStore(): string[] {
@@ -65,10 +73,6 @@ class ConnectionStore {
6573
return this.robotNamesByIP[ip] ?? null
6674
}
6775

68-
public setBrowserWindow(window: BrowserWindow): void {
69-
this.browserWindow = window
70-
}
71-
7276
public setPendingConnection(robotName: string): Promise<void> {
7377
return new Promise((resolve, reject) => {
7478
if (!this.isConnectingToBroker(robotName)) {
@@ -194,7 +198,7 @@ class ConnectionStore {
194198
public clearStore(): void {
195199
this.hostsByRobotName = {}
196200
this.robotNamesByIP = {}
197-
this.browserWindow = null
201+
this.browserWindows.clear()
198202
}
199203

200204
public isConnectedToBroker(robotName: string): boolean {

app/src/pages/Desktop/LivestreamViewer/index.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,13 @@ import { useTranslation } from 'react-i18next'
22
import { useSearchParams } from 'react-router-dom'
33

44
import { Chip } from '@opentrons/components'
5-
// eslint-disable-next-line no-restricted-imports
6-
import { useRunQuery } from '@opentrons/react-api-client'
75

86
import { useHlsVideo } from '/app/pages/Desktop/LivestreamViewer/hooks/useHlsVideo'
97
import {
108
LivestreamInfoScreen,
119
useLivestreamInfoScreen,
1210
} from '/app/pages/Desktop/LivestreamViewer/LivestreamInfoScreen'
11+
import { useNotifyRunQuery } from '/app/resources/runs'
1312

1413
import styles from './livestream.module.css'
1514

@@ -18,8 +17,7 @@ const RUN_POLLING_INTERVAL_MS = 5000
1817
export function LivestreamViewer(): JSX.Element {
1918
const [searchParams] = useSearchParams()
2019
const runId = searchParams.get('runId') ?? ''
21-
// TODO(jh, 11-04-25): Notifications are not working in secondary windows. Investigate further.
22-
const { data: runData, isLoading: isRunLoading } = useRunQuery(runId, {
20+
const { data: runData, isLoading: isRunLoading } = useNotifyRunQuery(runId, {
2321
refetchInterval: RUN_POLLING_INTERVAL_MS,
2422
})
2523
const runStatus = runData?.data.status ?? null

0 commit comments

Comments
 (0)