Skip to content

Commit

Permalink
Fix overflow behavior of sidebars (#13483)
Browse files Browse the repository at this point in the history 
* Fix overflow behavior of sidebars

- Ensure that last visible element also gets added to `...` overflow menu if there is not enough space
- Remove special behavior for only one overflowing element
  -  This behavior would always add a second element to the `overflow` menu if only one element is currently overflowing. This did not work for the case where only two tabs where open in the first place. In addition, it unnecessarily hide a tab in some cases even if there was enough space to render it.
- Extract logic to compute and hide hidden tabs into `hideOverflowingTabs` method
- Invoke this method from `computeOverflowingTabsData`. At this point the actual tabs bar is already
  rendered so we can use the actual position/height information to compute overflowing tabs instead
  of manually composing the available height with the hidden tabs bar in `updateTabs`
- Update `AdditionalViewsMenuWidget` to use a dedicated menu path for each side.  This ensures that
  only the hidden tabs of the corresponding sidebar are displayed when clicking the '...' button

Fixes #13482

Contributed on behalf of STMicroelectronics
  • Loading branch information
@tortmayr
tortmayr authored Jun 4, 2024
1 parent 249fd13 commit 4456fde
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 67 deletions.
11 changes: 6 additions & 5 deletions packages/core/src/browser/frontend-application-module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import {
MenuCommandAdapterRegistry,
MenuCommandExecutor,
MenuCommandAdapterRegistryImpl,
MenuCommandExecutorImpl
MenuCommandExecutorImpl,
MenuPath
} from '../common';
import { KeybindingRegistry, KeybindingContext, KeybindingContribution } from './keybinding';
import { FrontendApplication } from './frontend-application';
Expand Down Expand Up @@ -137,7 +138,7 @@ import { MarkdownRenderer, MarkdownRendererFactory, MarkdownRendererImpl } from
import { StylingParticipant, StylingService } from './styling-service';
import { bindCommonStylingParticipants } from './common-styling-participants';
import { HoverService } from './hover-service';
import { AdditionalViewsMenuWidget, AdditionalViewsMenuWidgetFactory } from './shell/additional-views-menu-widget';
import { AdditionalViewsMenuPath, AdditionalViewsMenuWidget, AdditionalViewsMenuWidgetFactory } from './shell/additional-views-menu-widget';
import { LanguageIconLabelProvider } from './language-icon-provider';
import { bindTreePreferences } from './tree';
import { OpenWithService } from './open-with-service';
Expand Down Expand Up @@ -177,9 +178,9 @@ export const frontendApplicationModule = new ContainerModule((bind, _unbind, _is
bind(SidebarBottomMenuWidgetFactory).toAutoFactory(SidebarBottomMenuWidget);
bind(AdditionalViewsMenuWidget).toSelf();
bind(AdditionalViewsMenuWidgetFactory).toFactory(ctx => (side: 'left' | 'right') => {
const widget = ctx.container.resolve(AdditionalViewsMenuWidget);
widget.side = side;
return widget;
const childContainer = ctx.container.createChild();
childContainer.bind<MenuPath>(AdditionalViewsMenuPath).toConstantValue(['additional_views_menu', side]);
return childContainer.resolve(AdditionalViewsMenuWidget);
});
bind(SplitPositionHandler).toSelf().inSingletonScope();

Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/browser/shell/additional-views-menu-widget.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ import { SideTabBar } from './tab-bars';
export const AdditionalViewsMenuWidgetFactory = Symbol('AdditionalViewsMenuWidgetFactory');
export type AdditionalViewsMenuWidgetFactory = (side: 'left' | 'right') => AdditionalViewsMenuWidget;

export const ADDITIONAL_VIEWS_MENU_PATH: MenuPath = ['additional_views_menu'];

export const AdditionalViewsMenuPath = Symbol('AdditionalViewsMenuPath');
@injectable()
export class AdditionalViewsMenuWidget extends SidebarMenuWidget {
static readonly ID = 'sidebar.additional.views';

side: 'left' | 'right';
@inject(AdditionalViewsMenuPath)
protected menuPath: MenuPath;

@inject(CommandRegistry)
protected readonly commandRegistry: CommandRegistry;
Expand All @@ -47,7 +47,7 @@ export class AdditionalViewsMenuWidget extends SidebarMenuWidget {
title: nls.localizeByDefault('Additional Views'),
iconClass: codicon('ellipsis'),
id: AdditionalViewsMenuWidget.ID,
menuPath: ADDITIONAL_VIEWS_MENU_PATH,
menuPath: this.menuPath,
order: 0
});
}
Expand All @@ -66,6 +66,6 @@ export class AdditionalViewsMenuWidget extends SidebarMenuWidget {
});
}
}));
this.menuDisposables.push(this.menuModelRegistry.registerMenuAction(ADDITIONAL_VIEWS_MENU_PATH, { commandId: command.id, order: index.toString() }));
this.menuDisposables.push(this.menuModelRegistry.registerMenuAction(this.menuPath, { commandId: command.id, order: index.toString() }));
}
}
3 changes: 2 additions & 1 deletion packages/core/src/browser/shell/side-panel-handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ export class SidePanelHandler {
protected createAdditionalViewsWidget(): AdditionalViewsMenuWidget {
const widget = this.additionalViewsMenuFactory(this.side);
widget.addClass('theia-sidebar-menu');
widget.addClass('theia-additional-views-menu');
return widget;
}

Expand Down Expand Up @@ -653,7 +654,7 @@ export class SidePanelHandler {
}

protected onTabsOverflowChanged(sender: SideTabBar, event: { titles: Title<Widget>[], startIndex: number }): void {
if (event.startIndex >= 0 && event.startIndex <= sender.currentIndex) {
if (event.startIndex > 0 && event.startIndex <= sender.currentIndex) {
sender.revealTab(sender.currentIndex);
} else {
this.additionalViewsMenu.updateAdditionalViews(sender, event);
Expand Down
95 changes: 39 additions & 56 deletions packages/core/src/browser/shell/tab-bars.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,8 +1084,6 @@ export class SideTabBar extends ScrollableTabBar {
startIndex: number
};

protected _rowGap: number;

constructor(options?: TabBar.IOptions<Widget> & PerfectScrollbar.Options) {
super(options);

Expand Down Expand Up @@ -1142,31 +1140,6 @@ export class SideTabBar extends ScrollableTabBar {
}
}

// Queries the tabRowGap value of the content node. Needed to properly compute overflowing
// tabs that should be hidden
protected get tabRowGap(): number {
// We assume that the tab row gap is static i.e. we compute it once an then cache it
if (!this._rowGap) {
this._rowGap = this.computeTabRowGap();
}
return this._rowGap;

}

protected computeTabRowGap(): number {
const style = window.getComputedStyle(this.contentNode);
const rowGapStyle = style.getPropertyValue('row-gap');
const numericValue = parseFloat(rowGapStyle);
const unit = rowGapStyle.match(/[a-zA-Z]+/)?.[0];

const tempDiv = document.createElement('div');
tempDiv.style.height = '1' + unit;
document.body.appendChild(tempDiv);
const rowGapValue = numericValue * tempDiv.offsetHeight;
document.body.removeChild(tempDiv);
return rowGapValue;
}

/**
* Reveal the tab with the given index by moving it into the non-overflowing tabBar section
* if necessary.
Expand Down Expand Up @@ -1207,18 +1180,13 @@ export class SideTabBar extends ScrollableTabBar {
const hiddenContent = this.hiddenContentNode;
const n = hiddenContent.children.length;
const renderData = new Array<Partial<SideBarRenderData>>(n);
const availableWidth = this.node.clientHeight - this.tabRowGap;
let actualWidth = 0;
let overflowStartIndex = -1;
for (let i = 0; i < n; i++) {
const hiddenTab = hiddenContent.children[i];
// Extract tab padding from the computed style
// Extract tab padding, and margin from the computed style
const tabStyle = window.getComputedStyle(hiddenTab);
const paddingTop = parseFloat(tabStyle.paddingTop!);
const paddingBottom = parseFloat(tabStyle.paddingBottom!);
const rd: Partial<SideBarRenderData> = {
paddingTop,
paddingBottom
paddingTop: parseFloat(tabStyle.paddingTop!),
paddingBottom: parseFloat(tabStyle.paddingBottom!)
};
// Extract label size from the DOM
const labelElements = hiddenTab.getElementsByClassName('p-TabBar-tabLabel');
Expand All @@ -1231,38 +1199,21 @@ export class SideTabBar extends ScrollableTabBar {
if (iconElements.length === 1) {
const icon = iconElements[0];
rd.iconSize = { width: icon.clientWidth, height: icon.clientHeight };
actualWidth += icon.clientHeight + paddingTop + paddingBottom + this.tabRowGap;

if (actualWidth > availableWidth && i !== 0) {
rd.visible = false;
if (overflowStartIndex === -1) {
overflowStartIndex = i;
}
}
renderData[i] = rd;
}
}

// Special handling if only one element is overflowing.
if (overflowStartIndex === n - 1 && renderData[overflowStartIndex]) {
if (!this.tabsOverflowData) {
overflowStartIndex--;
renderData[overflowStartIndex].visible = false;
} else {
renderData[overflowStartIndex].visible = true;
overflowStartIndex = -1;
}
renderData[i] = rd;
}
// Render into the visible node
this.renderTabs(this.contentNode, renderData);
this.computeOverflowingTabsData(overflowStartIndex);
this.computeOverflowingTabsData();
});
}
}

protected computeOverflowingTabsData(startIndex: number): void {
protected computeOverflowingTabsData(): void {
// ensure that render tabs has completed
window.requestAnimationFrame(() => {
const startIndex = this.hideOverflowingTabs();
if (startIndex === -1) {
if (this.tabsOverflowData) {
this.tabsOverflowData = undefined;
Expand All @@ -1286,6 +1237,38 @@ export class SideTabBar extends ScrollableTabBar {
});
}

/**
* Hide overflowing tabs and return the index of the first hidden tab.
*/
protected hideOverflowingTabs(): number {
const availableHeight = this.node.clientHeight;
const invisibleClass = 'p-mod-invisible';
let startIndex = -1;
const n = this.contentNode.children.length;
for (let i = 0; i < n; i++) {
const tab = this.contentNode.children[i] as HTMLLIElement;
if (tab.offsetTop + tab.offsetHeight >= availableHeight) {
tab.classList.add(invisibleClass);
if (startIndex === -1) {
startIndex = i;
/* If only one element is overflowing and the additional menu widget is visible (i.e. this.tabsOverflowData is set)
* there might already be enough space to show the last tab. In this case, we need to include the size of the
* additional menu widget and recheck if the last tab is visible */
if (startIndex === n - 1 && this.tabsOverflowData) {
const additionalViewsMenu = this.node.parentElement?.querySelector('.theia-additional-views-menu') as HTMLDivElement;
if (tab.offsetTop + tab.offsetHeight < availableHeight + additionalViewsMenu.offsetHeight) {
tab.classList.remove(invisibleClass);
startIndex = -1;
}
}
}
} else {
tab.classList.remove(invisibleClass);
}
}
return startIndex;
}

/**
* Render the tab bar using the given DOM element as host. The optional `renderData` is forwarded
* to the TabBarRenderer.
Expand Down

0 comments on commit 4456fde

Please sign in to comment.