Skip to content

Commit

Permalink
Fixing dashboard tabs not loading when the extension is not activated. (
Browse files Browse the repository at this point in the history
#24674)

* init

* Fixing core logic and adding loading notification

* Adding ways to reject tabs taking a long time

* Adding loading state for tabs

* Some accessibility fixes

* fixing lint issues

* Fixing error message

* Reordering listener to prevent race condition
  • Loading branch information
aasimkhan30 authored and siyangMicrosoft committed Nov 1, 2023
1 parent 3f2c4c7 commit cef8fd0
Show file tree
Hide file tree
Showing 11 changed files with 191 additions and 42 deletions.
29 changes: 29 additions & 0 deletions src/sql/base/browser/ui/panel/media/loading.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 31 additions & 0 deletions src/sql/base/browser/ui/panel/media/loading_inverse.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 16 additions & 0 deletions src/sql/base/browser/ui/panel/media/panel.css
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ panel {
outline: none;
}

.tabbedPanel .tabList .tab-header.loading,
.hc-light .tabbedPanel .tabList .tab-header.loading {
pointer-events:none;
cursor: not-allowed;
background-image: url("./loading.svg");
background-position: center right 5px;
background-repeat: no-repeat;
background-size: 16px;
opacity: 0.5;
}

.vs-dark .tabbedPanel .tabList .tab-header.loading,
.hc-black .tabbedPanel .tabList .tab-header.loading {
background-image: url("./loading_inverse.svg");
}

.tabbedPanel.horizontal > .title .tabList .tab .tabLabel,
.tabbedPanel.vertical > .title .tabList .tab .tabLabel {
text-overflow: ellipsis;
Expand Down
4 changes: 4 additions & 0 deletions src/sql/base/browser/ui/panel/panel.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ export class PanelComponent extends Disposable {
}

if (foundTab) {
// Don't do anything if the tab is loading
if (foundTab.loading) {
return;
}
const tab = foundTab;
// since we need to compare identifiers in this next step we are going to go through and make sure all tabs have one
this._tabs.forEach(i => {
Expand Down
1 change: 1 addition & 0 deletions src/sql/base/browser/ui/panel/tab.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export class TabComponent implements OnDestroy {
private _selected = false;
@Input() public identifier!: string;
@Input() public type: TabType = 'tab';
@Input() public loading: boolean = false;
@Input() private visibilityType: 'if' | 'visibility' = 'if';
private rendered = false;
private destroyed: boolean = false;
Expand Down
3 changes: 2 additions & 1 deletion src/sql/base/browser/ui/panel/tabHeader.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { CloseTabAction } from 'sql/base/browser/ui/panel/tabActions';
@Component({
selector: 'tab-header',
template: `
<div #actionHeader role="tab" [attr.aria-selected]="tab.selected" [attr.aria-label]="tab.title" class="tab-header" style="flex: 0 0; flex-direction: row;" [class.selected]="tab.selected" [attr.tabindex] = "_tabIndex" (click)="selectTab(tab)" (keyup)="onKey($event)">
<div #actionHeader role="tab" [attr.aria-selected]="tab.selected" [class.selected]="tab.selected" [class.loading]="tab.loading" [attr.aria-busy]="tab.loading" [attr.aria-label]="tab.title" class="tab-header" style="flex: 0 0; flex-direction: row;" [class.selected]="tab.selected" [attr.tabindex] = "_tabIndex" (click)="selectTab(tab)" (keyup)="onKey($event)">
<div class="tab" role="presentation">
<a #tabIcon *ngIf="showIcon && tab.iconClass" class="tabIcon codicon icon {{tab.iconClass}}"></a>
<a class="tabLabel" [class.selected]="tab.selected" [title]="tab.title" #tabLabel>{{tab.title}}</a>
Expand All @@ -32,6 +32,7 @@ export class TabHeaderComponent extends Disposable implements AfterContentInit,
@Input() public tab!: TabComponent;
@Input() public showIcon?: boolean;
@Input() public selected?: boolean;
@Input() public loading?: boolean;
@Output() public onSelectTab: EventEmitter<TabComponent> = new EventEmitter<TabComponent>();
@Output() public onCloseTab: EventEmitter<TabComponent> = new EventEmitter<TabComponent>();
@Output() public onFocusTab: EventEmitter<TabComponent> = new EventEmitter<TabComponent>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</div>
<div [style.height]="getContentAreaHeight()" [style.overflow]="containerOverflowStyle">
<tab [visibilityType]="'visibility'" *ngFor="let tab of tabs" [title]="tab.title" class="fullsize"
[identifier]="tab.id" [canClose]="tab.canClose" [actions]="tab.actions" [type]="tab.type"
[identifier]="tab.id" [canClose]="tab.canClose" [actions]="tab.actions" [type]="tab.type" [loading]="tab.loading"
[iconClass]="tab.iconClass">
<ng-template>
<dashboard-home-container *ngIf="tab.id === 'homeTab'; else not_home" [properties]="propertiesWidget" [tab]="tab">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import { LabeledMenuItemActionItem } from 'sql/platform/actions/browser/menuEntr
import { DASHBOARD_BORDER, TOOLBAR_OVERFLOW_SHADOW } from 'sql/workbench/common/theme';
import { IActionViewItem } from 'vs/base/browser/ui/actionbar/actionbar';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';

const dashboardRegistry = Registry.as<IDashboardRegistry>(DashboardExtensions.DashboardContributions);
const homeTabGroupId = 'home';
Expand Down Expand Up @@ -130,7 +131,8 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
@Inject(IContextKeyService) contextKeyService: IContextKeyService,
@Inject(IMenuService) private menuService: IMenuService,
@Inject(IWorkbenchThemeService) private themeService: IWorkbenchThemeService,
@Inject(IInstantiationService) private instantiationService: IInstantiationService
@Inject(IInstantiationService) private instantiationService: IInstantiationService,
@Inject(IExtensionService) private extensionService: IExtensionService,
) {
super();
this._tabName = DashboardPage.tabName.bindTo(contextKeyService);
Expand All @@ -141,7 +143,7 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
this.updateTheme(this.themeService.getColorTheme());
}

protected init() {
protected async init() {
this.dashboardService.dashboardContextKey.set(this.context);
if (!this.dashboardService.connectionManagementService.connectionInfo) {
this.notificationService.notify({
Expand All @@ -166,7 +168,7 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
layout: NavigationBarLayout.vertical,
showIcon: true
};
this.createTabs(tempWidgets);
await this.createTabs(tempWidgets);
}

this._register(this.themeService.onDidColorThemeChange((event: IColorTheme) => {
Expand Down Expand Up @@ -257,8 +259,8 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
const separator: HTMLElement = Taskbar.createTaskbarSeparator();
content.push({ element: separator });
}
const refreshAction = new RefreshWidgetAction(() => {
this.refresh();
const refreshAction = new RefreshWidgetAction(async () => {
await this.refresh();
}, this);
content.push({ action: refreshAction });
}
Expand Down Expand Up @@ -295,7 +297,7 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
return undefined;
}

private createTabs(homeWidgets: WidgetConfig[]) {
private async createTabs(homeWidgets: WidgetConfig[]) {
// Clear all tabs
this.tabs = [];
this._tabSettingConfigs = [];
Expand All @@ -305,15 +307,15 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
let allTabs = dashboardHelper.filterConfigs(dashboardRegistry.tabs, this);

// Before separating tabs into pinned / shown, ensure that the home tab is always set up as expected
allTabs = this.setAndRemoveHomeTab(allTabs, homeWidgets);
allTabs = await this.setAndRemoveHomeTab(allTabs, homeWidgets);

this.loadNewTabs(allTabs.filter((tab) => tab.group === homeTabGroupId));
await this.loadNewTabs(allTabs.filter((tab) => tab.group === homeTabGroupId));

// Load tab setting configs
this._tabSettingConfigs = this.dashboardService.getSettings<Array<TabSettingConfig>>([this.context, 'tabs'].join('.'));

this.addCustomTabGroups(allTabs);
this.addExtensionsTabGroup(allTabs);
await this.addCustomTabGroups(allTabs);
await this.addExtensionsTabGroup(allTabs);

this.panelActions = [];

Expand All @@ -329,20 +331,21 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
this.rewriteConfig();
}));

this._tabsDispose.push(this.dashboardService.onAddNewTabs(e => {
this.loadNewTabs(e, true);
this._tabsDispose.push(this.dashboardService.onAddNewTabs(async (e) => {
await this.loadNewTabs(e, true);
}));
}

/**
* Add the custom tab groups and their child tabs.
* @param allTabs The available tabs
*/
private addCustomTabGroups(allTabs: IDashboardTab[]): void {
dashboardRegistry.tabGroups.forEach((tabGroup) => {
private async addCustomTabGroups(allTabs: IDashboardTab[]): Promise<void> {
for (let i = 0; i < dashboardRegistry.tabGroups.length; i++) {
const tabGroup = dashboardRegistry.tabGroups[i];
const tabs = allTabs.filter(tab => tab.group === tabGroup.id);
if (tabs.length > 0) {
this.addNewTab({
await this.addNewTab({
id: tabGroup.id,
provider: Constants.anyProviderName,
originalConfig: [],
Expand All @@ -354,19 +357,19 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
canClose: false,
actions: []
});
this.loadNewTabs(tabs);
await this.loadNewTabs(tabs);
}
});
}
}

/**
* Add the "Extensions" tab group, tabs without a group will be added here.
* @param allTabs The available tabs
*/
private addExtensionsTabGroup(allTabs: IDashboardTab[]): void {
private async addExtensionsTabGroup(allTabs: IDashboardTab[]): Promise<void> {
const tabs = allTabs.filter(tab => !tab.group);
if (tabs.length > 0) {
this.addNewTab({
await this.addNewTab({
id: 'generalTabGroupHeader',
provider: Constants.anyProviderName,
originalConfig: [],
Expand All @@ -378,11 +381,11 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
canClose: false,
actions: []
});
this.loadNewTabs(tabs);
await this.loadNewTabs(tabs);
}
}

private setAndRemoveHomeTab(allTabs: IDashboardTab[], homeWidgets: WidgetConfig[]): IDashboardTab[] {
private async setAndRemoveHomeTab(allTabs: IDashboardTab[], homeWidgets: WidgetConfig[]): Promise<IDashboardTab[]> {
const homeTabConfig: TabConfig = {
id: this.homeTabId,
provider: Constants.anyProviderName,
Expand All @@ -405,7 +408,7 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
homeTabConfig.id = tabConfig.id;
homeTabConfig.container = tabConfig.container;
}
this.addNewTab(homeTabConfig);
await this.addNewTab(homeTabConfig);
return allTabs;
}

Expand All @@ -416,17 +419,20 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
this.dashboardService.writeSettings([this.context, 'tabs'].join('.'), writeableConfig, target);
}

private loadNewTabs(dashboardTabs: IDashboardTab[], openLastTab: boolean = false) {
private async loadNewTabs(dashboardTabs: IDashboardTab[], openLastTab: boolean = false) {
if (dashboardTabs && dashboardTabs.length > 0) {
const selectedTabs = dashboardTabs.map(v => this.initTabComponents(v)).map(v => {
const tabs = dashboardTabs.map(v => this.initTabComponents(v));
const selectedTabs = [];
for (let i = 0; i < tabs.length; i++) {
const v = tabs[i];
const config = v as TabConfig;
config.context = this.context;
config.editable = false;
config.canClose = false;
config.actions = [];
this.addNewTab(config);
return config;
});
await this.addNewTab(config);
selectedTabs.push(config);
}

if (openLastTab) {
// put this immediately on the stack so that is ran *after* the tab is rendered
Expand Down Expand Up @@ -495,14 +501,70 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
return tab.container ? Object.keys(tab.container)[0] : '';
}

private addNewTab(tab: TabConfig): void {
const existedTab = this.tabs.find(i => i.id === tab.id);
if (!existedTab) {
if (!tab.iconClass && tab.type !== 'group-header') {
tab.iconClass = 'default-tab-icon';
private async addNewTab(tab: TabConfig): Promise<void> {
try {
// Finding the owning extension for the tab
const owningExtension = this.extensionService.extensions.find(extension => {
if (extension?.contributes !== undefined && extension.contributes['dashboard.tabs']) {
const tabIndex = extension.contributes['dashboard.tabs'].findIndex(eTab => eTab.id === tab.id);
if (tabIndex !== -1) {
return true;
}
}
return false;
});

tab.loading = true;

// If the tab is contributed by an extension, we'll wait for the extension to be activated before adding the tab.
// Not doing so causes issues with the tab UI not being rendered correctly.
if (owningExtension) {
// wait for the extension to be activated
new Promise<void>((resolve) => {
const resolveTab = () => {
tab.loading = false;
this._cd.detectChanges();
resolve();
}
// We'll listen for the extension status change event and resolve the promise when the extension is activated
const disposable = this.extensionService.onDidChangeExtensionsStatus(e => {
e.forEach(extension => {
if (extension.value === owningExtension.id) {
disposable.dispose();
resolveTab();
}
})
});
// If the extension is already activated, we can resolve immediately
if (this.extensionService.getExtensionsStatus()[owningExtension.id]?.activationTimes?.activateResolvedTime !== undefined) {
tab.loading = false;
resolveTab();
}
}).catch(e => {
this.logService.error(`Error adding tab ${tab.title} contributed by extension ${owningExtension.id}. Error: ${e}`);
})
} else {
tab.loading = false;
}

const existedTab = this.tabs.find(i => i.id === tab.id);
if (!existedTab) {
if (!tab.iconClass && tab.type !== 'group-header') {
tab.iconClass = 'default-tab-icon';
}
this.tabs.push(tab);
this._cd.detectChanges();
} else {
this.logService.error(`Cannot add tab '${tab.title}'. Tab with id ${tab.id} already exists`);
}
this.tabs.push(tab);
this._cd.detectChanges();

} catch (error) {
// If we fail to load a tab, we'll just log the error and continue
this.notificationService.notify({
severity: Severity.Error,
message: nls.localize('dashboard.tabLoadingError', "Error loading tab {0}", tab.title)
});
this.logService.error(error);
}
}

Expand All @@ -527,9 +589,9 @@ export abstract class DashboardPage extends AngularDisposable implements IConfig
}
}

public refresh(refreshConfig: boolean = false): void {
public async refresh(refreshConfig: boolean = false): Promise<void> {
if (refreshConfig) {
this.init();
await this.init();
} else {
if (this._tabs) {
const tab = this._tabs.find(t => t.id === this._tabName.get());
Expand Down
Loading

0 comments on commit cef8fd0

Please sign in to comment.