Repository: manusa/electronim
Pull request #222 (2022-11-22 03:52:18)

♻️ tab traverse events listened app-globally

sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=manusa_electronim&pullRequest=222)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=222&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=222&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=222&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=222&resolved=false&types=CODE_SMELL)

[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=222&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=222&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=222&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=222&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/src/browser-window/keyboard-shortcuts.js b/src/browser-window/keyboard-shortcuts.js
index 9234761..2544fc7 100644
--- a/src/browser-window/keyboard-shortcuts.js
+++ b/src/browser-window/keyboard-shortcuts.js
@@ -26,12 +26,19 @@ EVENTS.set(eventKey({key: 'Escape'}), () => {
   eventBus.emit(APP_EVENTS.closeDialog);
 });
 
-EVENTS.set(eventKey({key: 'F11'}), () => {
-  eventBus.emit(APP_EVENTS.fullscreenToggle);
-});
+EVENTS.set(eventKey({key: 'F11'}), () => eventBus.emit(APP_EVENTS.fullscreenToggle));
+
+EVENTS.set(eventKey({key: 'Tab', control: true}), () =>
+  eventBus.emit(APP_EVENTS.tabTraverseNext));
+
+EVENTS.set(eventKey({key: 'Tab', shift: true, control: true}), () =>
+  eventBus.emit(APP_EVENTS.tabTraversePrevious));
 
 const registerAppShortcuts = (_, webContents) => {
-  webContents.on('before-input-event', (event, {key, shift, control, alt, meta}) => {
+  webContents.on('before-input-event', (event, {type, key, shift, control, alt, meta}) => {
+    if (type === 'keyUp') {
+      return;
+    }
     const func = EVENTS.get(eventKey({key, shift, control, alt, meta}));
     if (func) {
       event.preventDefault();
diff --git a/src/main/index.js b/src/main/index.js
index 9f57d83..d1d6b44 100644
--- a/src/main/index.js
+++ b/src/main/index.js
@@ -94,6 +94,9 @@ const handleMainWindowResize = event => {
 const handleTabReload = event => event.sender.reloadIgnoringCache();
 
 const handleTabTraverse = getTabIdFunction => () => {
+  if (mainWindow.getBrowserViews().length === 1) {
+    return;
+  }
   const tabId = getTabIdFunction();
   tabContainer.webContents.send(APP_EVENTS.activateTabInContainer, {tabId});
   activateTab(tabId);
@@ -149,8 +152,6 @@ const initTabListener = () => {
   });
   eventBus.on(APP_EVENTS.reload, handleTabReload);
   eventBus.on(APP_EVENTS.tabReorder, handleTabReorder);
-  eventBus.on(APP_EVENTS.tabTraverseNext, handleTabTraverse(tabManager.getNextTab));
-  eventBus.on(APP_EVENTS.tabTraversePrevious, handleTabTraverse(tabManager.getPreviousTab));
   eventBus.on(APP_EVENTS.zoomIn, handleZoomIn);
   eventBus.on(APP_EVENTS.zoomOut, handleZoomOut);
   eventBus.on(APP_EVENTS.zoomReset, handleZoomReset);
@@ -212,6 +213,8 @@ const initGlobalListeners = () => {
   eventBus.handle(APP_EVENTS.settingsLoad, loadSettings);
   eventBus.on(APP_EVENTS.settingsOpenDialog, openSettingsDialog(mainWindow));
   eventBus.on(APP_EVENTS.settingsSave, saveSettings);
+  eventBus.on(APP_EVENTS.tabTraverseNext, handleTabTraverse(tabManager.getNextTab));
+  eventBus.on(APP_EVENTS.tabTraversePrevious, handleTabTraverse(tabManager.getPreviousTab));
 };
 
 const browserVersionsReady = () => {
diff --git a/src/tab-manager/preload.keyboard-shortcuts.js b/src/tab-manager/preload.keyboard-shortcuts.js
index 6a52c2b..f9dc4d7 100644
--- a/src/tab-manager/preload.keyboard-shortcuts.js
+++ b/src/tab-manager/preload.keyboard-shortcuts.js
@@ -16,50 +16,26 @@
 /* eslint-disable no-undef */
 const {ipcRenderer} = require('electron');
 
-const triggerForActionMap = actionMap => ({key}) => {
-  if (actionMap[key]) {
-    ipcRenderer.send(actionMap[key]);
-  }
-};
-
-const triggerCode = event => triggerForActionMap({
-  F5: APP_EVENTS.reload
-})(event);
-
-const triggerControlCode = event => triggerForActionMap({
-  r: APP_EVENTS.reload,
-  R: APP_EVENTS.reload,
-  '+': APP_EVENTS.zoomIn,
-  '-': APP_EVENTS.zoomOut,
-  0: APP_EVENTS.zoomReset,
-  Tab: APP_EVENTS.tabTraverseNext
-})(event);
-
-const triggerControlShiftCode = event => triggerForActionMap({
-  Tab: APP_EVENTS.tabTraversePrevious
-})(event);
-
-const triggerCommandCode = event => triggerForActionMap({
-  r: APP_EVENTS.reload,
-  R: APP_EVENTS.reload
-})(event);
+const eventKey = ({key, shiftKey = false, ctrlKey = false, altKey = false, metaKey = false}) =>
+  `${key}-${shiftKey}-${ctrlKey}-${altKey}-${metaKey}`;
 
+const EVENTS = new Map();
 
-const isPlain = event => event.ctrlKey === false && event.metaKey === false && event.shiftKey === false;
-const isControl = event => event.ctrlKey === true && event.metaKey === false && event.shiftKey === false;
-const isControlShift = event => event.ctrlKey === true && event.metaKey === false && event.shiftKey === true;
-const isCommand = event => event.ctrlKey === false && event.metaKey === true && event.shiftKey === false;
+EVENTS.set(eventKey({key: 'F5'}), () => ipcRenderer.send(APP_EVENTS.reload));
+EVENTS.set(eventKey({key: 'R', ctrlKey: true}), () => ipcRenderer.send(APP_EVENTS.reload));
+EVENTS.set(eventKey({key: 'r', ctrlKey: true}), () => ipcRenderer.send(APP_EVENTS.reload));
+EVENTS.set(eventKey({key: 'R', metaKey: true}), () => ipcRenderer.send(APP_EVENTS.reload));
+EVENTS.set(eventKey({key: 'r', metaKey: true}), () => ipcRenderer.send(APP_EVENTS.reload));
+EVENTS.set(eventKey({key: '+', ctrlKey: true}), () => ipcRenderer.send(APP_EVENTS.zoomIn));
+EVENTS.set(eventKey({key: '-', ctrlKey: true}), () => ipcRenderer.send(APP_EVENTS.zoomOut));
+EVENTS.set(eventKey({key: '0', ctrlKey: true}), () => ipcRenderer.send(APP_EVENTS.zoomReset));
 
 const initKeyboardShortcuts = () => {
   window.addEventListener('keyup', event => {
-    if (isPlain(event)) {
-      triggerCode(event);
-    } else if (isControl(event)) {
-      triggerControlCode(event);
-    } else if (isControlShift(event)) {
-      triggerControlShiftCode(event);
-    } else if (isCommand(event)) {
-      triggerCommandCode(event);
+    const func = EVENTS.get(eventKey(event));
+    if (func) {
+      event.preventDefault();
+      func();
     }
   });
   window.addEventListener('load', () => {


Test patch:
diff --git a/src/browser-window/__tests__/keyboard-shortcuts.test.js b/src/browser-window/__tests__/keyboard-shortcuts.test.js
index d7d17e4..8d8eb6c 100644
--- a/src/browser-window/__tests__/keyboard-shortcuts.test.js
+++ b/src/browser-window/__tests__/keyboard-shortcuts.test.js
@@ -28,11 +28,19 @@ describe('Main :: Global Keyboard Shortcuts module test suite', () => {
     };
   });
   test.each([
-    [{key: 'Escape', appEvent: 'appMenuClose'}], [{key: 'Escape', appEvent: 'closeDialog'}],
-    [{key: 'F11', appEvent: 'fullscreenToggle'}]
-  ])('Key "$key" triggers "$appEvent" app event', ({key, appEvent}) => {
-    browserWindow.listeners['before-input-event'](inputEvent, {key});
-    expect(electron.ipcMain.emit).toHaveBeenCalledWith(appEvent);
+    {key: 'Escape', shift: false, control: false, appEvent: 'appMenuClose'},
+    {key: 'Escape', shift: false, control: false, appEvent: 'closeDialog'},
+    {key: 'F11', shift: false, control: false, appEvent: 'fullscreenToggle'},
+    {key: 'Tab', shift: false, control: true, appEvent: 'tabTraverseNext'},
+    {key: 'Tab', shift: true, control: true, appEvent: 'tabTraversePrevious'}
+  ])('Key "$key" (shift: $shift, ctrl: $control) triggers "$appEvent" app event',
+    ({key, control, shift, appEvent}) => {
+      browserWindow.listeners['before-input-event'](inputEvent, {key, control, shift});
+      expect(electron.ipcMain.emit).toHaveBeenCalledWith(appEvent);
+    });
+  test('ignores keyUp events', () => {
+    browserWindow.listeners['before-input-event'](inputEvent, {type: 'keyUp', key: 'Escape'});
+    expect(electron.ipcMain.emit).not.toHaveBeenCalled();
   });
   describe('preventDefault', () => {
     test('calls preventDefault if key is registered', () => {
diff --git a/src/main/__tests__/global-listeners.test.js b/src/main/__tests__/global-listeners.test.js
index e35cbd9..3b6786d 100644
--- a/src/main/__tests__/global-listeners.test.js
+++ b/src/main/__tests__/global-listeners.test.js
@@ -193,4 +193,45 @@ describe('Main :: Global listeners module test suite', () => {
       expect(electron.nativeTheme.themeSource).toEqual('system');
     });
   });
+  describe('handleTabTraverse', () => {
+    let tabManagerModule;
+    beforeEach(() => {
+      tabManagerModule = require('../../tab-manager');
+      jest.spyOn(tabManagerModule, 'getTab').mockImplementation();
+    });
+    test.each([
+      'tabTraverseNext', 'tabTraversePrevious'
+    ])('%s, with dialog visible, should not traverse', event => {
+      // Given
+      browserWindow.getBrowserViews = jest.fn(() => [new electron.BrowserView()]);
+      main.init();
+      // When
+      eventBus.listeners[event]();
+      // Then
+      expect(tabManagerModule.getTab).not.toHaveBeenCalled();
+    });
+    describe('with tabs visible, should traverse', () => {
+      beforeEach(() => {
+        browserWindow.getBrowserViews = jest.fn(() => [new electron.BrowserView(), new electron.BrowserView()]);
+      });
+      test('tabTraverseNext', () => {
+        jest.spyOn(tabManagerModule, 'getNextTab')
+          .mockImplementation(() => 'nextTabId');
+        main.init();
+        // When
+        eventBus.listeners.tabTraverseNext();
+        // Then
+        expect(tabManagerModule.getTab).toHaveBeenCalledWith('nextTabId');
+      });
+      test('tabTraversePrevious', () => {
+        jest.spyOn(tabManagerModule, 'getPreviousTab')
+          .mockImplementation(() => 'previousTabId');
+        main.init();
+        // When
+        eventBus.listeners.tabTraversePrevious();
+        // Then
+        expect(tabManagerModule.getTab).toHaveBeenCalledWith('previousTabId');
+      });
+    });
+  });
 });
diff --git a/src/main/__tests__/index.test.js b/src/main/__tests__/index.test.js
index 57ec01f..2424d92 100644
--- a/src/main/__tests__/index.test.js
+++ b/src/main/__tests__/index.test.js
@@ -366,29 +366,6 @@ describe('Main module test suite', () => {
       // Then
       expect(event.sender.reloadIgnoringCache).toHaveBeenCalledTimes(1);
     });
-    describe('handleTabTraverse', () => {
-      beforeEach(() => {
-        jest.spyOn(tabManagerModule, 'getTab').mockImplementation();
-      });
-      test('tabTraverseNext', () => {
-        jest.spyOn(tabManagerModule, 'getNextTab')
-          .mockImplementation(() => 'nextTabId');
-        main.init();
-        // When
-        mockIpc.listeners.tabTraverseNext();
-        // Then
-        expect(tabManagerModule.getTab).toHaveBeenCalledWith('nextTabId');
-      });
-      test('tabTraversePrevious', () => {
-        jest.spyOn(tabManagerModule, 'getPreviousTab')
-          .mockImplementation(() => 'previousTabId');
-        main.init();
-        // When
-        mockIpc.listeners.tabTraversePrevious();
-        // Then
-        expect(tabManagerModule.getTab).toHaveBeenCalledWith('previousTabId');
-      });
-    });
     test('handleZoomIn', () => {
       const event = {sender: {
         getZoomFactor: jest.fn(() => 0),
diff --git a/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js b/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js
index ee2df1b..13d6d86 100644
--- a/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js
+++ b/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js
@@ -57,24 +57,6 @@ describe('Browser Keyboard Shortcuts test suite', () => {
       // Then
       expect(mockIpcRenderer.send).not.toHaveBeenCalled();
     });
-    test('ctrl+r, should send reload app event', () => {
-      // Given
-      browserKeyboardShortcuts.initKeyboardShortcuts();
-      // When
-      window.dispatchEvent(new KeyboardEvent('keyup', {key: 'r', ctrlKey: true}));
-      // Then
-      expect(mockIpcRenderer.send).toHaveBeenCalledTimes(1);
-      expect(mockIpcRenderer.send).toHaveBeenCalledWith('reload');
-    });
-    test('ctrl+tab, should send tabTraverseNext event', () => {
-      // Given
-      browserKeyboardShortcuts.initKeyboardShortcuts();
-      // When
-      window.dispatchEvent(new KeyboardEvent('keyup', {key: 'Tab', ctrlKey: true}));
-      // Then
-      expect(mockIpcRenderer.send).toHaveBeenCalledTimes(1);
-      expect(mockIpcRenderer.send).toHaveBeenCalledWith('tabTraverseNext');
-    });
   });
   describe('Command modified events', () => {
     test('cmd+R, should send reload app event', () => {
@@ -87,17 +69,6 @@ describe('Browser Keyboard Shortcuts test suite', () => {
       expect(mockIpcRenderer.send).toHaveBeenCalledWith('reload');
     });
   });
-  describe('Control+Shift modified events', () => {
-    test('ctrl+shift+tab, should send tabTraversePrevious event', () => {
-      // Given
-      browserKeyboardShortcuts.initKeyboardShortcuts();
-      // When
-      window.dispatchEvent(new KeyboardEvent('keyup', {key: 'Tab', ctrlKey: true, shiftKey: true}));
-      // Then
-      expect(mockIpcRenderer.send).toHaveBeenCalledTimes(1);
-      expect(mockIpcRenderer.send).toHaveBeenCalledWith('tabTraversePrevious');
-    });
-  });
   describe('Mouse wheel events', () => {
     test('ctrl+scrollUp, should send zoomIn event', () => {
       // Given