[scm] the order of icon in scm is different in Chrom and Firefox

[kpge]

Description

the order of command icon in scm is different in Chrom and Firefox

Reproduction Steps

• open instance in chrome and firefox
• open scm

OS and Theia version:

• OS: ubuntu18
• chrome: 75.0.3770.100
• firefox: 60.7.2esr
• theia :latest
  Diagnostics:

[akosyakov]

@kpge Will you have chance to investigate the cause? PRs are welcomed if they don't involve big structural changes.

[vince-fugnitto]

It doesn't look like a problem for the scm widget but generally to all toolbar items.
It looks like chrome and firefox differ from the priority and will be reverses of each other.

For example, in the search-in-workspace widget:

Firefox

Chrome

[kpge]

I think the problem is here :

the sort funtion in Chrome and Firefox has diffenrent behavior

• Chrome:
  
• Firefox:
  

[kittaakos]

Isn’t the comparator function the strange? Assuming I have left and right with the following items:

"{"id":"search-in-workspace.refresh","command":"search-in-workspace.refresh","tooltip":"Refresh","priority":0}"

"{"id":"search-in-workspace.clear-all","command":"search-in-workspace.clear-all","tooltip":"Clear Search Results","priority":1}"

When I compare them, them following branch will return with 1:

        if (left.group === undefined || left.group === 'navigation') {
            return 1;
        }

Meaning that left is greater than right, hence it should come after right, but the priority is the other way and ignored.

[kittaakos]

Also, I have managed to "break it" on Chrome:

[akosyakov]

Could someone look at the comparator? It seems to be relatively easy fix. The logic should be following:

• items should be compare by groups first
• navigator group is special and always have the higher priority
• undefined group is navigator group as well
• if items have the same group then priority should be used for comparison within the group

It seems that the last point is not true for items within the navigator group.

[kittaakos]

diff --git a/packages/core/src/browser/shell/tab-bar-toolbar.tsx b/packages/core/src/browser/shell/tab-bar-toolbar.tsx
index 8978286cc..a6caeaccd 100644
--- a/packages/core/src/browser/shell/tab-bar-toolbar.tsx
+++ b/packages/core/src/browser/shell/tab-bar-toolbar.tsx
@@ -292,26 +292,18 @@ export namespace TabBarToolbarItem {
      */
     export const PRIORITY_COMPARATOR = (left: TabBarToolbarItem, right: TabBarToolbarItem) => {
         // The navigation group is special as it will always be sorted to the top/beginning of a menu.
-        if (left.group === undefined || left.group === 'navigation') {
-            return 1;
-        }
-        if (right.group === undefined || right.group === 'navigation') {
-            return -1;
-        }
-        if (left.group && right.group) {
-            if (left.group < right.group) {
-                return -1;
-            } else if (left.group > right.group) {
-                return 1;
-            } else {
-                return 0;
+        const compareGroup = (leftGroup: string | undefined = 'navigation', rightGroup: string | undefined = 'navigation') => {
+            if (leftGroup === 'navigation') {
+                return rightGroup === 'navigation' ? 0 : -1;
             }
-        }
-        if (left.group) {
-            return -1;
-        }
-        if (right.group) {
-            return 1;
+            if (rightGroup === 'navigation') {
+                return leftGroup === 'navigation' ? 0 : 1;
+            }
+            return leftGroup.localeCompare(rightGroup);
+        };
+        const result = compareGroup(left.group, right.group);
+        if (result !== 0) {
+            return result;
         }
         return (left.priority || 0) - (right.priority || 0);
     };

?

[kittaakos]

I will take care of it.