Skip to content

Commit 301678a

Browse files
manusamanusa
authored
✨ initial openUrlsInApp implementation (#452)
Signed-off-by: Marc Nuri <[email protected]>
1 parent 6756708 commit 301678a

File tree

9 files changed

+114
-32
lines changed

9 files changed

+114
-32
lines changed

‎src/components/icon.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ Icon.more = '\ue619';
5151
Icon.moreVert = '\ue5d4';
5252
Icon.notifications = '\ue7f4';
5353
Icon.notificationsOff = '\ue7f6';
54+
Icon.preview = '\uf1c5';
5455
Icon.save = '\ue161';
5556
Icon.settings = '\ue8b8';
5657
Icon.spellcheck = '\ue8ce';

‎src/settings/__tests__/settings.browser.mjs

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ export const ipcRenderer = () => {
2626
disableNotificationsGlobally: false,
2727
tabs: [
2828
{id: '1', url: 'https://initial-tab.com', sandboxed: true},
29-
{id: '2', url: 'https://initial-tab-2.com', disabled: true, disableNotifications: true}
29+
{id: '2', url: 'https://initial-tab-2.com', disabled: true, disableNotifications: true},
30+
{id: '3', url: 'https://initial-tab-3.com', openUrlsInApp: true}
3031
],
3132
theme: 'dark',
3233
trayEnabled: true,

‎src/settings/__tests__/settings.browser.test.mjs

+62-13
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import {jest} from '@jest/globals';
1717
import {loadDOM} from '../../__tests__/index.mjs';
1818
import {ipcRenderer} from './settings.browser.mjs';
19-
import {fireEvent, getByTestId, getByText, waitFor} from '@testing-library/dom';
19+
import {fireEvent, getAllByText, getByTestId, getByText, waitFor} from '@testing-library/dom';
2020

2121
describe('Settings in Browser test suite', () => {
2222
let mockIpcRenderer;
@@ -62,7 +62,8 @@ describe('Settings in Browser test suite', () => {
6262
{
6363
tabs: [
6464
{id: '1', url: 'https://initial-tab.com', sandboxed: true},
65-
{id: '2', url: 'https://initial-tab-2.com', disabled: true, disableNotifications: true}
65+
{id: '2', url: 'https://initial-tab-2.com', disabled: true, disableNotifications: true},
66+
{id: '3', url: 'https://initial-tab-3.com', openUrlsInApp: true}
6667
],
6768
enabledDictionaries: ['en'],
6869
disableNotificationsGlobally: false,
@@ -116,7 +117,7 @@ describe('Settings in Browser test suite', () => {
116117
fireEvent.input($input, {target: {value: 'A'}});
117118
// Then
118119
await waitFor(() => expect($input.value).toBe('A'));
119-
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(2);
120+
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(3);
120121
});
121122
describe.each([
122123
'Enter',
@@ -128,7 +129,7 @@ describe('Settings in Browser test suite', () => {
128129
// When
129130
fireEvent.keyDown($input, {code});
130131
// Then
131-
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(2);
132+
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(3);
132133
expect($addTabButton.hasAttribute('disabled')).toBe(true);
133134
expect($submitButton.hasAttribute('disabled')).toBe(false);
134135
});
@@ -138,10 +139,10 @@ describe('Settings in Browser test suite', () => {
138139
// When
139140
fireEvent.keyDown($input, {code});
140141
// Then
141-
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(2);
142+
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(3);
142143
await waitFor(() =>
143144
expect($input.parentElement.classList.contains('errored')).toBe(true));
144-
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input').length).toBe(2);
145+
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input').length).toBe(3);
145146
expect($input.value).toBe('invalid:1337:url');
146147
expect($addTabButton.hasAttribute('disabled')).toBe(true);
147148
expect($submitButton.hasAttribute('disabled')).toBe(true);
@@ -153,9 +154,9 @@ describe('Settings in Browser test suite', () => {
153154
fireEvent.keyDown($input, {code});
154155
// Then
155156
await waitFor(() =>
156-
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(3));
157+
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(4));
157158
expect($input.classList.contains('is-success')).toBe(false);
158-
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input')[2].value)
159+
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input')[3].value)
159160
.toBe('https://info.cern.ch');
160161
expect($input.value).toBe('');
161162
expect($addTabButton.hasAttribute('disabled')).toBe(true);
@@ -168,10 +169,10 @@ describe('Settings in Browser test suite', () => {
168169
fireEvent.keyDown($input, {code});
169170
// Then
170171
await waitFor(() =>
171-
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(3));
172+
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(4));
172173
await waitFor(() =>
173174
expect($input.classList.contains('is-success')).toBe(false));
174-
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input')[2].value)
175+
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input')[3].value)
175176
.toBe('http://info.cern.ch');
176177
expect($input.value).toBe('');
177178
expect($addTabButton.hasAttribute('disabled')).toBe(true);
@@ -206,8 +207,8 @@ describe('Settings in Browser test suite', () => {
206207
fireEvent.click($addTabButton);
207208
// Then
208209
await waitFor(() =>
209-
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(3));
210-
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input')[2].value)
210+
expect($tabContainer.querySelectorAll('.settings__tab').length).toBe(4));
211+
expect($tabContainer.querySelectorAll('.settings__tab .settings__tab-main input')[3].value)
211212
.toBe('https://info.cern.ch');
212213
expect($input.value).toBe('');
213214
expect($addTabButton.hasAttribute('disabled')).toBe(true);
@@ -256,7 +257,7 @@ describe('Settings in Browser test suite', () => {
256257
});
257258
test('Notification enabled icon click, should disable notification', async () => {
258259
// Given
259-
const $notificationEnabledIcon = getByText(document.querySelector('.settings__tabs'), '\ue7f4');
260+
const $notificationEnabledIcon = getAllByText(document.querySelector('.settings__tabs'), '\ue7f4')[0];
260261
// When
261262
fireEvent.click($notificationEnabledIcon);
262263
// Then
@@ -370,6 +371,54 @@ describe('Settings in Browser test suite', () => {
370371
expect($settingsTab.textContent).toContain('\ue88d');
371372
});
372373
});
374+
describe('with openUrlsInApp=true', () => {
375+
let $settingsTab;
376+
let $toggleIcon;
377+
let $openUrlsInAppEntry;
378+
beforeEach(async () => {
379+
$settingsTab = document.querySelector('.settings__tab[data-id="3"]');
380+
$toggleIcon = $settingsTab.querySelector('.expand-button');
381+
$openUrlsInAppEntry = $settingsTab.querySelector('.open-urls-in-app-toggle');
382+
if ($toggleIcon.title.startsWith('Expand')) {
383+
fireEvent.click($toggleIcon);
384+
// eslint-disable-next-line jest/no-standalone-expect
385+
await waitFor(() => expect($toggleIcon.title).toEqual('Collapse'));
386+
}
387+
});
388+
test('click on switch, should turn off', async () => {
389+
// Given
390+
const $switch = $openUrlsInAppEntry.querySelector('.material3.switch');
391+
// When
392+
fireEvent.click($switch);
393+
// Then
394+
await waitFor(() => expect($switch.classList.contains('switch--checked')).toBe(false));
395+
expect($switch.classList).not.toContain('switch--checked');
396+
});
397+
});
398+
describe('with openUrlsInApp=false', () => {
399+
let $settingsTab;
400+
let $toggleIcon;
401+
let $openUrlsInAppEntry;
402+
beforeEach(async () => {
403+
$settingsTab = document.querySelector('.settings__tab[data-id="1"]');
404+
$toggleIcon = $settingsTab.querySelector('.expand-button');
405+
$openUrlsInAppEntry = $settingsTab.querySelector('.open-urls-in-app-toggle');
406+
if ($toggleIcon.title.startsWith('Expand')) {
407+
fireEvent.click($toggleIcon);
408+
// eslint-disable-next-line jest/no-standalone-expect
409+
await waitFor(() => expect($toggleIcon.title).toEqual('Collapse'));
410+
}
411+
});
412+
test('click on switch, should turn on', async () => {
413+
// Given
414+
const $switch = $openUrlsInAppEntry.querySelector('.material3.switch');
415+
// When
416+
fireEvent.click($switch);
417+
// Then
418+
await waitFor(() => expect($switch.classList.contains('switch--checked')).toBe(true));
419+
expect($switch.classList).toContain('switch--checked');
420+
});
421+
});
373422
});
374423
});
375424
describe('URL edit', () => {

‎src/settings/settings.browser.css

+4-2
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@
5151

5252
.settings__tab .settings__tab-advanced {
5353
position: relative;
54+
transform-origin: 0 0;
5455
transform: scaleY(0);
55-
height: 0;
56+
max-height: 0;
5657
margin: 0;
5758
transition:
5859
margin-top 0.3s ease-out,
@@ -62,7 +63,8 @@
6263

6364
.settings__tab--expanded .settings__tab-advanced {
6465
transform: scaleY(1);
65-
height: 100%;
66+
max-height: 100%;
67+
height: 100%; /* TODO: use calc-size() when bump chrome version */
6668
margin-top: var(--material3-card-spacing) ;
6769
overflow-y: clip;
6870
}

‎src/settings/settings.reducer.browser.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export const reducer = (state, action) => {
6666
id: newId(),
6767
disabled: false,
6868
sandboxed: false,
69+
openUrlsInApp: false,
6970
disableNotifications: false,
7071
url: prependProtocol(state.newTabValue)
7172
}],

‎src/settings/settings.services.browser.mjs

+9-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const ExpandButton = ({dispatch, id, expanded = false}) => {
3434
};
3535

3636
const TabAdvancedSettings = (
37-
{dispatch, id, sandboxed = false}
37+
{dispatch, id, sandboxed = false, openUrlsInApp = false}
3838
) => html`
3939
<div class='settings__tab-advanced'>
4040
<${SettingsOption}
@@ -45,6 +45,14 @@ const TabAdvancedSettings = (
4545
className='sandboxed-toggle'
4646
title='Use an isolated/sandboxed session for this tab'
4747
/>
48+
<${SettingsOption}
49+
onClick=${toggleTabProperty(dispatch, 'openUrlsInApp', id)}
50+
checked=${openUrlsInApp}
51+
icon=${Icon.preview}
52+
label='Open URLs in app'
53+
className='open-urls-in-app-toggle'
54+
title='Open URLs within ElectronIM instead of opening them in the default system browser'
55+
/>
4856
</div>
4957
`;
5058

‎src/tab-manager/__tests__/index.test.js

+12
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,18 @@ describe('Tab Manager module test suite', () => {
152152
expect(require('electron').WebContentsView).toHaveBeenCalledWith({
153153
webPreferences: expect.objectContaining({session: expect.anything()})});
154154
});
155+
test('openUrlsInApp=true, should not set setWindowOpenHandler', () => {
156+
// When
157+
tabManager.addTabs({send: jest.fn()})([{id: 1337, url: 'https://localhost', openUrlsInApp: true}]);
158+
// Then
159+
expect(mockView.webContents.setWindowOpenHandler).not.toHaveBeenCalled();
160+
});
161+
test('openUrlsInApp=true, should not set will-navigate event handler', () => {
162+
// When
163+
tabManager.addTabs({send: jest.fn()})([{id: 1337, url: 'https://localhost', openUrlsInApp: true}]);
164+
// Then
165+
expect(mockView.listeners['will-navigate']).not.toBeDefined();
166+
});
155167
test('Tab webContents should be configured and loaded', () => {
156168
// Given
157169
const mockIpcSender = {send: jest.fn()};

‎src/tab-manager/index.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ const cleanUserAgent = view => {
6868
const addTabs = ipcSender => tabsMetadata => {
6969
const useNativeSpellChecker = getUseNativeSpellChecker();
7070
const enabledDictionaries = getEnabledDictionaries();
71-
tabsMetadata.forEach(({id, url, sandboxed = false}) => {
71+
tabsMetadata.forEach(({
72+
id, url, sandboxed = false, openUrlsInApp = false
73+
}) => {
7274
const tabPreferences = {...webPreferences};
7375
if (sandboxed) {
7476
tabPreferences.session = session.fromPartition(`persist:${id}`, {cache: true});
@@ -94,8 +96,10 @@ const addTabs = ipcSender => tabsMetadata => {
9496
cleanUserAgent(tab);
9597
tab.webContents.loadURL(url);
9698

97-
tab.webContents.on('will-navigate', handleRedirect(tab));
98-
tab.webContents.setWindowOpenHandler(windowOpenHandler(tab));
99+
if (!openUrlsInApp) {
100+
tab.webContents.on('will-navigate', handleRedirect(tab));
101+
tab.webContents.setWindowOpenHandler(windowOpenHandler(tab));
102+
}
99103

100104
const handlePageTitleUpdatedForCurrentTab = handlePageTitleUpdated(ipcSender, id);
101105
tab.webContents.on('page-title-updated', handlePageTitleUpdatedForCurrentTab);

‎src/tab-manager/redirect.js

+16-12
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,24 @@ const openExternal = urlString => {
7575
shell.openExternal(urlString).then(() => {});
7676
};
7777

78-
const handleRedirect = view => (e, urlString) => {
79-
const url = new URL(urlString);
80-
if (shouldOpenInExternalBrowser(view, url)) {
81-
e.preventDefault();
82-
openExternal(urlString);
83-
}
78+
const handleRedirect = view => {
79+
return (e, urlString) => {
80+
const url = new URL(urlString);
81+
if (shouldOpenInExternalBrowser(view, url)) {
82+
e.preventDefault();
83+
openExternal(urlString);
84+
}
85+
};
8486
};
8587

86-
const windowOpenHandler = view => ({url}) => {
87-
if (!shouldOpenInExternalBrowser(view, new URL(url))) {
88-
return {action: 'allow'};
89-
}
90-
openExternal(url);
91-
return {action: 'deny'};
88+
const windowOpenHandler = view => {
89+
return ({url}) => {
90+
if (!shouldOpenInExternalBrowser(view, new URL(url))) {
91+
return {action: 'allow'};
92+
}
93+
openExternal(url);
94+
return {action: 'deny'};
95+
};
9296
};
9397

9498
module.exports = {

0 commit comments

Comments
 (0)