Repository: manusa/electronim (js)

Issue:
add shortcut Ctrl+Tab and Ctrl+Shift+Tab
Tabs traversing with the keyboard are pretty generic on firefox, chrome, etc. and I was expecting to work in this (nice!) application. They are very useful to avoid using the mouse to click and traverse through different messages on different platforms

Pull request description:
✨ tab traverse shortcuts
Fix #161

Patch:
diff --git a/docs/Keyboard-shortcuts.md b/docs/Keyboard-shortcuts.md
index de80ac7..69c9bb8 100644
--- a/docs/Keyboard-shortcuts.md
+++ b/docs/Keyboard-shortcuts.md
@@ -1,8 +1,10 @@
 # Keyboard Shortcuts
 
-| Key combination                                            | Description         |
-|------------------------------------------------------------|---------------------|
-| `Ctrl+r` `Cmd+r` `F5`                                      | Reload current tab. |
-| `Ctrl++` `Cmd++` <br /> `Ctrl+ScrollUp` `Cmd+ScrollUp`     | Zoom in.            |
-| `Ctrl+-` `Cmd+-` <br /> `Ctrl+ScrollDown` `Cmd+ScrollDown` | Zoom out.           |
-| `Ctrl+0` `Cmd+0`                                           | Reset zoom.         |
\ No newline at end of file
+| Key combination                                            | Description                    |
+|------------------------------------------------------------|--------------------------------|
+| `Ctrl+r` `Cmd+r` `F5`                                      | Reload current tab.            |
+| `Ctrl++` `Cmd++` <br /> `Ctrl+ScrollUp` `Cmd+ScrollUp`     | Zoom in.                       |
+| `Ctrl+-` `Cmd+-` <br /> `Ctrl+ScrollDown` `Cmd+ScrollDown` | Zoom out.                      |
+| `Ctrl+0` `Cmd+0`                                           | Reset zoom.                    |
+| `Ctrl+Tab`                                                 | Jump to the next open tab.     |
+| `Ctrl+Shift+Tab`                                           | Jump to the previous open tab. |
diff --git a/src/constants/index.js b/src/constants/index.js
index 8ef2c5a..fe725fc 100644
--- a/src/constants/index.js
+++ b/src/constants/index.js
@@ -47,6 +47,8 @@ const APP_EVENTS = {
   canNotify: 'canNotify',
   tabsReady: 'tabsReady',
   tabReorder: 'tabReorder',
+  tabTraverseNext: 'tabTraverseNext',
+  tabTraversePrevious: 'tabTraversePrevious',
   zoomIn: 'zoomIn',
   zoomOut: 'zoomOut',
   zoomReset: 'zoomReset'
diff --git a/src/main/index.js b/src/main/index.js
index e44c228..51f2ba4 100644
--- a/src/main/index.js
+++ b/src/main/index.js
@@ -89,6 +89,12 @@ const handleMainWindowResize = event => {
 
 const handleTabReload = event => event.sender.reloadIgnoringCache();
 
+const handleTabTraverse = getTabIdFunction => () => {
+  const tabId = getTabIdFunction();
+  tabContainer.webContents.send(APP_EVENTS.activateTabInContainer, {tabId});
+  activateTab(tabId);
+};
+
 const handleZoomIn = event => event.sender.setZoomFactor(event.sender.getZoomFactor() + 0.1);
 
 const handleZoomOut = event => {
@@ -139,6 +145,8 @@ const initTabListener = () => {
   });
   ipc.on(APP_EVENTS.reload, handleTabReload);
   ipc.on(APP_EVENTS.tabReorder, handleTabReorder);
+  ipc.on(APP_EVENTS.tabTraverseNext, handleTabTraverse(tabManager.getNextTab));
+  ipc.on(APP_EVENTS.tabTraversePrevious, handleTabTraverse(tabManager.getPreviousTab));
   ipc.on(APP_EVENTS.zoomIn, handleZoomIn);
   ipc.on(APP_EVENTS.zoomOut, handleZoomOut);
   ipc.on(APP_EVENTS.zoomReset, handleZoomReset);
diff --git a/src/tab-manager/index.js b/src/tab-manager/index.js
index 4f03d46..c4b4cfa 100644
--- a/src/tab-manager/index.js
+++ b/src/tab-manager/index.js
@@ -141,6 +141,20 @@ const setActiveTab = tabId => {
   settings.updateSettings({activeTab});
 };
 
+const getTabTraverse = operation => () => {
+  const tabIds = Object.keys(tabs);
+  const idx = operation(tabIds.indexOf(getActiveTab()));
+  if (idx < 0) {
+    return tabIds[tabIds.length - 1];
+  } else if (idx >= tabIds.length) {
+    return tabIds[0];
+  }
+  return tabIds[idx];
+};
+
+const getNextTab = getTabTraverse(idx => idx + 1);
+const getPreviousTab = getTabTraverse(idx => idx - 1);
+
 const removeAll = () => {
   Object.values(tabs).forEach(browserView => browserView.webContents.destroy());
   Object.keys(tabs).forEach(key => delete tabs[key]);
@@ -158,5 +172,5 @@ const canNotify = tabId => {
 };
 
 module.exports = {
-  addTabs, getTab, getActiveTab, setActiveTab, canNotify, reload, removeAll
+  addTabs, getTab, getActiveTab, setActiveTab, getNextTab, getPreviousTab, canNotify, reload, removeAll
 };
diff --git a/src/tab-manager/preload.keyboard-shortcuts.js b/src/tab-manager/preload.keyboard-shortcuts.js
index 18c341e..6a52c2b 100644
--- a/src/tab-manager/preload.keyboard-shortcuts.js
+++ b/src/tab-manager/preload.keyboard-shortcuts.js
@@ -16,40 +16,50 @@
 /* eslint-disable no-undef */
 const {ipcRenderer} = require('electron');
 
-const codeActionMap = {
-  F5: APP_EVENTS.reload
+const triggerForActionMap = actionMap => ({key}) => {
+  if (actionMap[key]) {
+    ipcRenderer.send(actionMap[key]);
+  }
 };
 
-const controlCodeActionMap = {
+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
-};
+  0: APP_EVENTS.zoomReset,
+  Tab: APP_EVENTS.tabTraverseNext
+})(event);
 
-const commandCodeActionMap = {
+const triggerControlShiftCode = event => triggerForActionMap({
+  Tab: APP_EVENTS.tabTraversePrevious
+})(event);
+
+const triggerCommandCode = event => triggerForActionMap({
   r: APP_EVENTS.reload,
   R: APP_EVENTS.reload
-};
+})(event);
 
-const triggerForActionMap = actionMap => key => {
-  if (actionMap[key]) {
-    ipcRenderer.send(actionMap[key]);
-  }
-};
+
+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;
 
 const initKeyboardShortcuts = () => {
-  const triggerCodeActionMap = triggerForActionMap(codeActionMap);
-  const triggerControlCodeActionMap = triggerForActionMap(controlCodeActionMap);
-  const triggerCommandCodeActionMap = triggerForActionMap(commandCodeActionMap);
   window.addEventListener('keyup', event => {
-    if (event.ctrlKey === false && event.metaKey === false) {
-      triggerCodeActionMap(event.key);
-    } else if (event.ctrlKey === true && event.metaKey === false) {
-      triggerControlCodeActionMap(event.key);
-    } else if (event.ctrlKey === false && event.metaKey === true) {
-      triggerCommandCodeActionMap(event.key);
+    if (isPlain(event)) {
+      triggerCode(event);
+    } else if (isControl(event)) {
+      triggerControlCode(event);
+    } else if (isControlShift(event)) {
+      triggerControlShiftCode(event);
+    } else if (isCommand(event)) {
+      triggerCommandCode(event);
     }
   });
   window.addEventListener('load', () => {


Test patch:
diff --git a/src/main/__tests__/index.test.js b/src/main/__tests__/index.test.js
index 911c7f2..7c594d3 100644
--- a/src/main/__tests__/index.test.js
+++ b/src/main/__tests__/index.test.js
@@ -350,6 +350,29 @@ 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__/index.test.js b/src/tab-manager/__tests__/index.test.js
index 4652dfa..90557c7 100644
--- a/src/tab-manager/__tests__/index.test.js
+++ b/src/tab-manager/__tests__/index.test.js
@@ -84,6 +84,51 @@ describe('Tab Manager module test suite', () => {
       expect(result).toBeNull();
     });
   });
+  describe('getTabTraverse', () => {
+    beforeEach(() => {
+      tabManager.addTabs({send: jest.fn()})([
+        {id: 'A'},
+        {id: 'B'},
+        {id: 'C'}
+      ]);
+    });
+    describe('getNextTab with tabs [A, B, C]', () => {
+      test('with currentTab = A, should return B', () => {
+        // Given
+        tabManager.setActiveTab('A');
+        // When
+        const nextTab = tabManager.getNextTab();
+        // Then
+        expect(nextTab).toBe('B');
+      });
+      test('with currentTab = C, should return A', () => {
+        // Given
+        tabManager.setActiveTab('C');
+        // When
+        const nextTab = tabManager.getNextTab();
+        // Then
+        expect(nextTab).toBe('A');
+      });
+    });
+    describe('getPreviousTab', () => {
+      test('with currentTab = B, should return A', () => {
+        // Given
+        tabManager.setActiveTab('B');
+        // When
+        const nextTab = tabManager.getPreviousTab();
+        // Then
+        expect(nextTab).toBe('A');
+      });
+      test('with currentTab = A, should return C', () => {
+        // Given
+        tabManager.setActiveTab('A');
+        // When
+        const nextTab = tabManager.getPreviousTab();
+        // Then
+        expect(nextTab).toBe('C');
+      });
+    });
+  });
   describe('addTabs', () => {
     test('webPreferences is sandboxed and has no node integration', () => {
       // When
diff --git a/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js b/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js
index 67e817b..ee2df1b 100644
--- a/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js
+++ b/src/tab-manager/__tests__/preload.keyboard-shortcuts.test.js
@@ -17,11 +17,7 @@ describe('Browser Keyboard Shortcuts test suite', () => {
   let mockIpcRenderer;
   let browserKeyboardShortcuts;
   beforeEach(() => {
-    global.APP_EVENTS = {
-      reload: 'reload',
-      zoomIn: 'zoomIn',
-      zoomOut: 'zoomOut'
-    };
+    global.APP_EVENTS = require('../../constants').APP_EVENTS;
     mockIpcRenderer = {
       send: jest.fn()
     };
@@ -70,6 +66,15 @@ describe('Browser Keyboard Shortcuts test suite', () => {
       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', () => {
@@ -82,6 +87,17 @@ 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