Repository: manusa/electronim (js)

Issue:
Use current tab order when cycling tabs
**Current behavior**
Looks like ElectronIM uses the Tabs order of creation to cycle through them using `Ctrl+Tab`.

**Expected behavior**
Use the current tab order to cycle tabs.

Pull request:
🐛 updated tab order respected while traversing tabs
Fixes #385 

Patch:
diff --git a/src/main/index.js b/src/main/index.js
index 57ff32e..ad690c9 100644
--- a/src/main/index.js
+++ b/src/main/index.js
@@ -128,6 +128,7 @@ const handleTabReorder = (_event, {tabIds: visibleTabIds}) => {
     ...visibleTabIds.map(tabId => currentTabMap[tabId]),
     ...hiddenTabIds.map(tabId => currentTabMap[tabId])
   ];
+  tabManager.sortTabs(visibleTabIds);
   updateSettings({tabs});
 };
 
diff --git a/src/tab-manager/index.js b/src/tab-manager/index.js
index f80e690..14d4966 100644
--- a/src/tab-manager/index.js
+++ b/src/tab-manager/index.js
@@ -116,6 +116,22 @@ const addTabs = ipcSender => tabsMetadata => {
   ipcSender.send(APP_EVENTS.addTabs, tabsMetadata);
 };
 
+const sortTabs = tabIds => {
+  if (tabIds.length !== Object.keys(tabs).length) {
+    // Skip in case there are inconsistencies
+    // eslint-disable-next-line no-console
+    console.error(`Inconsistent tab state, skipping sort operation (${tabIds.length} !== ${Object.keys(tabs).length}).`);
+    return;
+  }
+  const oldTabs = {...tabs};
+  // Clean previous state
+  Object.keys(tabs).forEach(key => delete tabs[key]);
+  // Set the tabs with the correct ordering
+  for (const tabId of tabIds) {
+    tabs[tabId] = oldTabs[tabId];
+  }
+};
+
 const getTab = tabId => (tabId ? tabs[tabId.toString()] : null);
 
 const getActiveTab = () => activeTab;
@@ -166,5 +182,6 @@ const canNotify = tabId => {
 };
 
 module.exports = {
-  addTabs, getTab, getTabAt, getActiveTab, setActiveTab, getNextTab, getPreviousTab, canNotify, reload, removeAll
+  addTabs, sortTabs, getTab, getTabAt, getActiveTab, setActiveTab, getNextTab, getPreviousTab,
+  canNotify, reload, removeAll
 };


Test patch:
diff --git a/src/main/__tests__/index.test.js b/src/main/__tests__/index.test.js
index 0bc1fa3..c204b48 100644
--- a/src/main/__tests__/index.test.js
+++ b/src/main/__tests__/index.test.js
@@ -398,6 +398,18 @@ describe('Main module test suite', () => {
         // Then
         expect(settingsModule.updateSettings).toHaveBeenCalledWith({tabs: [{id: '313373'}, {id: '1337'}]});
       });
+      test('Several tabs, order changed, should update tabManager order', () => {
+        // Given
+        mockSettings = {
+          tabs: [{id: '1337'}, {id: '313373'}]
+        };
+        jest.spyOn(tabManagerModule, 'sortTabs').mockImplementation();
+        main.init();
+        // When
+        mockIpc.listeners.tabReorder({}, {tabIds: ['313373', '1337']});
+        // Then
+        expect(tabManagerModule.sortTabs).toHaveBeenCalledWith(['313373', '1337']);
+      });
       test('Several tabs with hidden, order changed, should update settings keeping hidden tags', () => {
         // Given
         mockSettings = {
diff --git a/src/tab-manager/__tests__/index.test.js b/src/tab-manager/__tests__/index.test.js
index 06fdc20..effeb02 100644
--- a/src/tab-manager/__tests__/index.test.js
+++ b/src/tab-manager/__tests__/index.test.js
@@ -13,6 +13,7 @@
    See the License for the specific language governing permissions and
    limitations under the License.
  */
+/* eslint-disable no-console */
 describe('Tab Manager module test suite', () => {
   let mockBrowserView;
   let userAgent;
@@ -238,6 +239,25 @@ describe('Tab Manager module test suite', () => {
       });
     });
   });
+  describe('sortTabs', () => {
+    test('Aborts in case of inconsistency', () => {
+      // Given
+      jest.spyOn(console, 'error').mockImplementationOnce(() => {});
+      // When
+      tabManager.sortTabs(['1', '2']);
+      // Then
+      expect(console.error).toHaveBeenCalledWith('Inconsistent tab state, skipping sort operation (2 !== 0).');
+    });
+    test('Sorts tabs with new order', () => {
+      // Given
+      tabManager.addTabs({send: jest.fn()})([{id: 'A1337', url: 'https://localhost'}, {id: 'B31337', url: 'https://example.com'}]);
+      // When
+      tabManager.sortTabs(['B31337', 'A1337']);
+      // Then
+      expect(tabManager.getTabAt(1)).toBe('B31337');
+      expect(tabManager.getTabAt(2)).toBe('A1337');
+    });
+  });
   describe('activeTab', () => {
     test('setActiveTab/getActiveTab, should set/return currently active tab', () => {
       // Given