From d17284f3d45aaae9930dd74cc039390c9e7f273d Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Mon, 7 Aug 2023 09:39:31 -0400 Subject: [PATCH] tabbar: support icon-less items 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 --- .../core/src/browser/label-parser.spec.ts | 38 +++++++++++++++++++ packages/core/src/browser/label-parser.ts | 15 ++++++++ .../shell/tab-bar-toolbar/tab-bar-toolbar.tsx | 31 ++++++++++++--- packages/core/src/browser/style/tabs.css | 5 +++ 4 files changed, 83 insertions(+), 6 deletions(-) diff --git a/packages/core/src/browser/label-parser.spec.ts b/packages/core/src/browser/label-parser.spec.ts index 9224692b44665..2c1be315cf6c7 100644 --- a/packages/core/src/browser/label-parser.spec.ts +++ b/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'); + }); }); diff --git a/packages/core/src/browser/label-parser.ts b/packages/core/src/browser/label-parser.ts index 5918d60c59b7d..2fcb9e13b9bf8 100644 --- a/packages/core/src/browser/label-parser.ts +++ b/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(' '); + } + } diff --git a/packages/core/src/browser/shell/tab-bar-toolbar/tab-bar-toolbar.tsx b/packages/core/src/browser/shell/tab-bar-toolbar/tab-bar-toolbar.tsx index dac2d86a66425..599f35c7f197b 100644 --- a/packages/core/src/browser/shell/tab-bar-toolbar/tab-bar-toolbar.tsx +++ b/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). */ @@ -176,8 +181,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(' ')); @@ -186,13 +195,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.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; }