tabbar: support icon-less items (#12804)

VS Code renders action buttons in the tab bar
for commands that do not have icons using
their title text, instead. This commit does the
same in Theia.

Additionally, two related minor issues are fixed:

- the $(icon) specifiers for icons show in the
  tooltip of an action, which is confusing
- the roll-over highlight shows only on action
  buttons for commands that use the "icon"
  property, not those that embed icon specifiers
  in their titles

Fixes #12686

Signed-off-by: Christian W. Damus <cdamus.ext@eclipsesource.com>

Author: cdamus

packages/core/src/browser/label-parser.spec.ts

@@ -124,4 +124,42 @@ describe('StatusBarEntryUtility', () => {
         expect((iconArr[3] as LabelIcon).name).equals('icon3');
     });
 
+    it('should strip nothing from an empty string', () => {
+        text = '';
+        const stripped: string = statusBarEntryUtility.stripIcons(text);
+        expect(stripped).to.be.equal(text);
+    });
+
+    it('should strip nothing from an string containing no icons', () => {
+        // Deliberate double space to verify not concatenating these words
+        text = 'foo  bar';
+        const stripped: string = statusBarEntryUtility.stripIcons(text);
+        expect(stripped).to.be.equal(text);
+    });
+
+    it("should strip a medial '$(icon)' from a string", () => {
+        text = 'foo $(icon) bar';
+        const stripped: string = statusBarEntryUtility.stripIcons(text);
+        expect(stripped).to.be.equal('foo bar');
+    });
+
+    it("should strip a terminal '$(icon)' from a string", () => {
+        // Deliberate double space to verify not concatenating these words
+        text = 'foo  bar $(icon)';
+        const stripped: string = statusBarEntryUtility.stripIcons(text);
+        expect(stripped).to.be.equal('foo  bar');
+    });
+
+    it("should strip an initial '$(icon)' from a string", () => {
+        // Deliberate double space to verify not concatenating these words
+        text = '$(icon) foo  bar';
+        const stripped: string = statusBarEntryUtility.stripIcons(text);
+        expect(stripped).to.be.equal('foo  bar');
+    });
+
+    it("should strip multiple '$(icon)' specifiers from a string", () => {
+        text = '$(icon1) foo $(icon2)$(icon3)  bar $(icon4) $(icon5)';
+        const stripped: string = statusBarEntryUtility.stripIcons(text);
+        expect(stripped).to.be.equal('foo bar');
+    });
 });

packages/core/src/browser/label-parser.ts

@@ -90,4 +90,19 @@ export class LabelParser {
         return parserArray;
     }
 
+    /**
+     * Strips icon specifiers from the given `text`, leaving only a
+     * space-separated concatenation of the non-icon segments.
+     *
+     * @param text text to be stripped of icon specifiers
+     * @returns the `text` with icon specifiers stripped out
+     */
+    stripIcons(text: string): string {
+        return this.parse(text)
+            .filter(item => !LabelIcon.is(item))
+            .map(s => (s as string).trim())
+            .filter(s => s.length)
+            .join(' ');
+    }
+
 }

packages/core/src/browser/shell/tab-bar-toolbar/tab-bar-toolbar.tsx

@@ -33,6 +33,11 @@ export interface TabBarToolbarFactory {
     (): TabBarToolbar;
 }
 
+/**
+ * Class name indicating rendering of a toolbar item without an icon but instead with a text label.
+ */
+const NO_ICON_CLASS = 'no-icon';
+
 /**
  * Tab-bar toolbar widget representing the active [tab-bar toolbar items](TabBarToolbarItem).
  */
@@ -178,8 +183,12 @@ export class TabBarToolbar extends ReactWidget {
     protected renderItem(item: AnyToolbarItem): React.ReactNode {
         let innerText = '';
         const classNames = [];
-        if (item.text) {
-            for (const labelPart of this.labelParser.parse(item.text)) {
+        const command = item.command ? this.commands.getCommand(item.command) : undefined;
+        // Fall back to the item ID in extremis so there is _something_ to render in the
+        // case that there is neither an icon nor a title
+        const itemText = item.text || command?.label || command?.id || item.id;
+        if (itemText) {
+            for (const labelPart of this.labelParser.parse(itemText)) {
                 if (LabelIcon.is(labelPart)) {
                     const className = `fa fa-${labelPart.name}${labelPart.animation ? ' fa-' + labelPart.animation : ''}`;
                     classNames.push(...className.split(' '));
@@ -188,13 +197,23 @@ export class TabBarToolbar extends ReactWidget {
                 }
             }
         }
-        const command = item.command ? this.commands.getCommand(item.command) : undefined;
-        let iconClass = (typeof item.icon === 'function' && item.icon()) || item.icon as string || (command && command.iconClass);
+        const iconClass = (typeof item.icon === 'function' && item.icon()) || item.icon as string || (command && command.iconClass);
         if (iconClass) {
-            iconClass += ` ${ACTION_ITEM}`;
             classNames.push(iconClass);
         }
-        const tooltip = `${item.tooltip || (command && command.label) || ''}${this.resolveKeybindingForCommand(command?.id)}`;
+        const tooltipText = item.tooltip || (command && command.label) || '';
+        const tooltip = `${this.labelParser.stripIcons(tooltipText)}${this.resolveKeybindingForCommand(command?.id)}`;
+
+        // Only present text if there is no icon
+        if (classNames.length) {
+            innerText = '';
+        } else if (innerText) {
+            // Make room for the label text
+            classNames.push(NO_ICON_CLASS);
+        }
+
+        // In any case, this is an action item, with or without icon.
+        classNames.push(ACTION_ITEM);
 
         const toolbarItemClassNames = this.getToolbarItemClassNames(item);
         return <div key={item.id}

packages/core/src/browser/style/tabs.css

@@ -480,6 +480,11 @@
   line-height: 18px;
 }
 
+.p-TabBar-toolbar .item > div.no-icon {
+  /* Make room for a text label instead of an icon. */
+  width: 100%;
+}
+
 .p-TabBar-toolbar .item .collapse-all {
   background: var(--theia-icon-collapse-all) no-repeat;
 }