Skip to content

Commit d582d02

Browse files
codedreadcodedread
committed
Fix issue #11 by restoring popup click handlers in popup.js and add a unit test.
1 parent e5d4165 commit d582d02

File tree

8 files changed

+232
-62
lines changed

8 files changed

+232
-62
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ All notable changes to this project will be documented in this file.
1010
- Updated all code to modern JavaScript and improved documentation.
1111
- Fixed [issue #3](https://github.com/codedread/spaces/issues/3) by escaping
1212
HTML for all extension content.
13-
- Increased unit test coverage from 0% to 6.99%.
13+
- Increased unit test coverage from 0% to 7.51%.

‎js/background/background.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@ async function processMessage(request, sender) {
255255
return requestCurrentSpace();
256256

257257
case 'generatePopupParams':
258-
return generatePopupParams(request.action, request.tabUrl);
258+
// TODO: Investigate if || request.action should be removed.
259+
return generatePopupParams(request.popupAction || request.action, request.tabUrl);
259260

260261
case 'loadSession':
261262
sessionId = cleanParameter(request.sessionId);

‎js/popup.js‎

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,42 +7,57 @@ import { checkSessionOverwrite, escapeHtml } from './utils.js';
77
const UNSAVED_SESSION = '(unnamed window)';
88
const NO_HOTKEY = 'no hotkey set';
99

10+
/**
11+
* Handles popup menu clicks by generating popup params and reloading
12+
* @param {string} action The popup action ('switch' or 'move')
13+
*/
14+
export async function handlePopupMenuClick(action) {
15+
const params = await chrome.runtime.sendMessage({'action': 'generatePopupParams', 'popupAction': action});
16+
if (!params) return;
17+
window.location.hash = params;
18+
window.location.reload();
19+
}
20+
1021
const nodes = {};
1122
let globalCurrentSpace;
1223
let globalTabId;
1324
let globalUrl;
1425
let globalWindowId;
1526
let globalSessionName;
1627

17-
/**
18-
* POPUP INIT
19-
*/
20-
21-
document.addEventListener('DOMContentLoaded', async () => {
22-
const url = getHashVariable('url', window.location.href);
23-
globalUrl = url !== '' ? decodeURIComponent(url) : false;
24-
const currentWindow = await chrome.windows.getCurrent({ populate: true });
25-
const windowId = currentWindow.id;
26-
globalWindowId = windowId !== '' ? windowId : false;
27-
globalTabId = getHashVariable('tabId', window.location.href);
28-
const sessionName = getHashVariable(
29-
'sessionName',
30-
window.location.href
31-
);
32-
globalSessionName =
33-
sessionName && sessionName !== 'false' ? sessionName : false;
34-
const action = getHashVariable('action', window.location.href);
35-
36-
const requestSpacePromise = globalWindowId
37-
? chrome.runtime.sendMessage({ action: 'requestSpaceFromWindowId', windowId: globalWindowId })
38-
: chrome.runtime.sendMessage({ action: 'requestCurrentSpace' });
39-
40-
requestSpacePromise.then(space => {
41-
globalCurrentSpace = space;
42-
renderCommon();
43-
routeView(action);
28+
/** Initialize the popup window. */
29+
function initializePopup() {
30+
document.addEventListener('DOMContentLoaded', async () => {
31+
const url = getHashVariable('url', window.location.href);
32+
globalUrl = url !== '' ? decodeURIComponent(url) : false;
33+
const currentWindow = await chrome.windows.getCurrent({ populate: true });
34+
const windowId = currentWindow.id;
35+
globalWindowId = windowId !== '' ? windowId : false;
36+
globalTabId = getHashVariable('tabId', window.location.href);
37+
const sessionName = getHashVariable(
38+
'sessionName',
39+
window.location.href
40+
);
41+
globalSessionName =
42+
sessionName && sessionName !== 'false' ? sessionName : false;
43+
const action = getHashVariable('action', window.location.href);
44+
45+
const requestSpacePromise = globalWindowId
46+
? chrome.runtime.sendMessage({ action: 'requestSpaceFromWindowId', windowId: globalWindowId })
47+
: chrome.runtime.sendMessage({ action: 'requestCurrentSpace' });
48+
49+
requestSpacePromise.then(space => {
50+
globalCurrentSpace = space;
51+
renderCommon();
52+
routeView(action);
53+
});
4454
});
45-
});
55+
}
56+
57+
// Auto-initialize when loaded in browser context
58+
if (typeof document !== 'undefined' && typeof window !== 'undefined') {
59+
initializePopup();
60+
}
4661

4762
function routeView(action) {
4863
if (action === 'move') {
@@ -138,22 +153,10 @@ async function renderMainCard() {
138153
});
139154
document
140155
.querySelector('#switcherLink .optionText')
141-
.addEventListener('click', async () => {
142-
const params = await chrome.runtime.sendMessage({'action': 'switch'});
143-
if (!params) return;
144-
window.location.hash = params;
145-
window.location.reload();
146-
renderSwitchCard();
147-
});
156+
.addEventListener('click', () => handlePopupMenuClick('switch'));
148157
document
149158
.querySelector('#moverLink .optionText')
150-
.addEventListener('click', async () => {
151-
const params = await chrome.runtime.sendMessage({'action': 'generatePopupParams'});
152-
if (!params) return;
153-
window.location.hash = params;
154-
window.location.reload();
155-
// renderMoveCard()
156-
});
159+
.addEventListener('click', () => handlePopupMenuClick('move'));
157160
}
158161

159162
async function requestHotkeys() {

‎tests/cleanUrl.test.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,10 @@
33
*/
44

55
import { cleanUrl } from '../js/background/spacesService.js';
6+
import { setupMinimalChromeMocks } from './helpers.js';
67

7-
// Mock chrome.runtime.id for testing
8-
global.chrome = {
9-
runtime: {
10-
id: 'test-extension-id-12345'
11-
}
12-
};
8+
// Setup minimal Chrome mocks for testing
9+
setupMinimalChromeMocks();
1310

1411
describe('cleanUrl', () => {
1512
describe('basic functionality', () => {

‎tests/filterInternalWindows.test.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,10 @@
33
*/
44

55
import { filterInternalWindows } from '../js/background/spacesService.js';
6+
import { setupMinimalChromeMocks } from './helpers.js';
67

7-
// Mock chrome.runtime.id for testing
8-
global.chrome = {
9-
runtime: {
10-
id: 'test-extension-id-12345'
11-
}
12-
};
8+
// Setup minimal Chrome mocks for testing
9+
setupMinimalChromeMocks();
1310

1411
describe('filterInternalWindows', () => {
1512
describe('normal windows (should not be filtered)', () => {

‎tests/generateSessionHash.test.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,10 @@
33
*/
44

55
import { generateSessionHash } from '../js/background/spacesService.js';
6+
import { setupMinimalChromeMocks } from './helpers.js';
67

7-
// Mock chrome.runtime.id for testing
8-
global.chrome = {
9-
runtime: {
10-
id: 'test-extension-id-12345'
11-
}
12-
};
8+
// Setup minimal Chrome mocks for testing
9+
setupMinimalChromeMocks();
1310

1411
describe('generateSessionHash', () => {
1512
describe('deterministic behavior', () => {

‎tests/helpers.js‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,69 @@
11
/**
22
* Shared test helper functions
33
*/
4+
import { jest } from '@jest/globals';
45
import { dbService } from '../js/background/dbService.js';
56

7+
// Re-export jest for convenience
8+
export { jest };
9+
10+
/**
11+
* Sets up global Chrome API mocks for testing
12+
*/
13+
export const setupChromeMocks = () => {
14+
global.chrome = {
15+
runtime: {
16+
sendMessage: jest.fn(),
17+
},
18+
windows: {
19+
getCurrent: jest.fn(),
20+
},
21+
tabs: {
22+
query: jest.fn(),
23+
}
24+
};
25+
};
26+
27+
/**
28+
* Sets up minimal Chrome API mocks for testing (just runtime.id)
29+
*/
30+
export const setupMinimalChromeMocks = () => {
31+
global.chrome = {
32+
runtime: {
33+
id: 'test-extension-id-12345'
34+
}
35+
};
36+
};
37+
38+
/**
39+
* Sets up global DOM mocks for testing
40+
*/
41+
export const setupDOMMocks = () => {
42+
global.document = {
43+
addEventListener: jest.fn(),
44+
getElementById: jest.fn(),
45+
querySelector: jest.fn(),
46+
querySelectorAll: jest.fn(),
47+
};
48+
49+
global.window = {
50+
location: {
51+
href: 'popup.html#',
52+
hash: '',
53+
reload: jest.fn(),
54+
},
55+
close: jest.fn(),
56+
};
57+
};
58+
59+
/**
60+
* Sets up all common test mocks (Chrome APIs and DOM)
61+
*/
62+
export const setupTestMocks = () => {
63+
setupChromeMocks();
64+
setupDOMMocks();
65+
};
66+
667
/**
768
* Creates a mock for console methods (error, warn, log, etc.)
869
* @param {string} method - The console method to mock ('error', 'warn', 'log', etc.)

‎tests/popupClickHandlers.test.js‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* Unit tests for popup.js functionality
3+
* Tests the popup menu item click handlers that send messages to background script
4+
*/
5+
6+
import { jest, setupTestMocks } from './helpers.js';
7+
import { handlePopupMenuClick } from '../js/popup.js';
8+
9+
// Setup all test mocks
10+
setupTestMocks();
11+
12+
describe('Popup Menu Click Handlers', () => {
13+
beforeEach(() => {
14+
jest.clearAllMocks();
15+
16+
// Reset window location
17+
window.location.hash = '';
18+
window.location.reload.mockClear();
19+
chrome.runtime.sendMessage.mockClear();
20+
});
21+
22+
test('handlePopupMenuClick with switch action sends correct message and reloads popup', async () => {
23+
// Setup: Mock successful response from background script
24+
const mockParams = 'action=switch&windowId=123&sessionName=TestSpace&tabId=456';
25+
chrome.runtime.sendMessage.mockResolvedValue(mockParams);
26+
27+
// Execute the click handler with switch action
28+
await handlePopupMenuClick('switch');
29+
30+
// Verify the message was sent with correct parameters
31+
expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({
32+
'action': 'generatePopupParams',
33+
'popupAction': 'switch'
34+
});
35+
36+
// Verify popup was reloaded with correct parameters
37+
expect(window.location.hash).toBe(mockParams);
38+
expect(window.location.reload).toHaveBeenCalled();
39+
});
40+
41+
test('handlePopupMenuClick with move action sends correct message and reloads popup', async () => {
42+
// Setup: Mock successful response from background script
43+
const mockParams = 'action=move&windowId=123&sessionName=TestSpace&tabId=456';
44+
chrome.runtime.sendMessage.mockResolvedValue(mockParams);
45+
46+
// Execute the click handler with move action
47+
await handlePopupMenuClick('move');
48+
49+
// Verify the message was sent with correct parameters
50+
expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({
51+
'action': 'generatePopupParams',
52+
'popupAction': 'move'
53+
});
54+
55+
// Verify popup was reloaded with correct parameters
56+
expect(window.location.hash).toBe(mockParams);
57+
expect(window.location.reload).toHaveBeenCalled();
58+
});
59+
60+
test('handlePopupMenuClick handles empty response gracefully', async () => {
61+
// Setup: Mock empty response from background script
62+
chrome.runtime.sendMessage.mockResolvedValue('');
63+
64+
// Execute the click handler
65+
await handlePopupMenuClick('test');
66+
67+
// Verify the message was sent
68+
expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({
69+
'action': 'generatePopupParams',
70+
'popupAction': 'test'
71+
});
72+
73+
// Verify popup was NOT reloaded due to empty response
74+
expect(window.location.hash).toBe('');
75+
expect(window.location.reload).not.toHaveBeenCalled();
76+
});
77+
78+
test('handlePopupMenuClick handles null response gracefully', async () => {
79+
// Setup: Mock null response from background script
80+
chrome.runtime.sendMessage.mockResolvedValue(null);
81+
82+
// Execute the click handler
83+
await handlePopupMenuClick('move');
84+
85+
// Verify the message was sent
86+
expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({
87+
'action': 'generatePopupParams',
88+
'popupAction': 'move'
89+
});
90+
91+
// Verify popup was NOT reloaded due to null response
92+
expect(window.location.hash).toBe('');
93+
expect(window.location.reload).not.toHaveBeenCalled();
94+
});
95+
96+
test('handlePopupMenuClick handles sendMessage rejection', async () => {
97+
// Setup: Mock sendMessage rejection
98+
const mockError = new Error('Background script error');
99+
chrome.runtime.sendMessage.mockRejectedValue(mockError);
100+
101+
// Execute the click handler - it should throw since there's no error handling
102+
await expect(handlePopupMenuClick('switch')).rejects.toThrow('Background script error');
103+
104+
// Verify the message was sent
105+
expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({
106+
'action': 'generatePopupParams',
107+
'popupAction': 'switch'
108+
});
109+
110+
// Verify popup was NOT reloaded due to error
111+
expect(window.location.hash).toBe('');
112+
expect(window.location.reload).not.toHaveBeenCalled();
113+
});
114+
});

0 commit comments

Comments
 (0)