Merge pull request microsoft#103738 from jeanp413:fix-feedback-widget-position

Fixes feedback widget appears wrong when invoked from command palette

Author: sbatten

src/vs/workbench/contrib/feedback/browser/feedback.ts

@@ -5,8 +5,7 @@
 
 import 'vs/css!./media/feedback';
 import * as nls from 'vs/nls';
-import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
-import { Dropdown } from 'vs/base/browser/ui/dropdown/dropdown';
+import { IDisposable, DisposableStore, Disposable } from 'vs/base/common/lifecycle';
 import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import * as dom from 'vs/base/browser/dom';
 import { ICommandService } from 'vs/platform/commands/common/commands';
@@ -24,6 +23,8 @@ import { IOpenerService } from 'vs/platform/opener/common/opener';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { KeyCode } from 'vs/base/common/keyCodes';
 import { Codicon } from 'vs/base/common/codicons';
+import { Emitter } from 'vs/base/common/event';
+import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService';
 
 export interface IFeedback {
 	feedback: string;
@@ -36,17 +37,18 @@ export interface IFeedbackDelegate {
 }
 
 export interface IFeedbackWidgetOptions {
-	contextViewProvider: IContextViewService;
 	feedbackService: IFeedbackDelegate;
-	onFeedbackVisibilityChange?: (visible: boolean) => void;
 }
 
-export class FeedbackWidget extends Dropdown {
+export class FeedbackWidget extends Disposable {
+	private visible: boolean | undefined;
+	private _onDidChangeVisibility = new Emitter<boolean>();
+	readonly onDidChangeVisibility = this._onDidChangeVisibility.event;
+
 	private maxFeedbackCharacters: number;
 
 	private feedback: string = '';
 	private sentiment: number = 1;
-	private autoHideTimeout?: number;
 
 	private readonly feedbackDelegate: IFeedbackDelegate;
 
@@ -63,8 +65,9 @@ export class FeedbackWidget extends Dropdown {
 	private isPure: boolean = true;
 
 	constructor(
-		container: HTMLElement,
-		private options: IFeedbackWidgetOptions,
+		options: IFeedbackWidgetOptions,
+		@IContextViewService private readonly contextViewService: IContextViewService,
+		@IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService,
 		@ICommandService private readonly commandService: ICommandService,
 		@ITelemetryService private readonly telemetryService: ITelemetryService,
 		@IIntegrityService private readonly integrityService: IIntegrityService,
@@ -73,7 +76,7 @@ export class FeedbackWidget extends Dropdown {
 		@IProductService productService: IProductService,
 		@IOpenerService private readonly openerService: IOpenerService
 	) {
-		super(container, options);
+		super();
 
 		this.feedbackDelegate = options.feedbackService;
 		this.maxFeedbackCharacters = this.feedbackDelegate.getCharacterLimit(this.sentiment);
@@ -88,22 +91,24 @@ export class FeedbackWidget extends Dropdown {
 			}
 		});
 
-		this.element.classList.add('send-feedback');
-		this.element.title = nls.localize('sendFeedback', "Tweet Feedback");
+		// Hide feedback widget whenever notifications appear
+		this._register(this.layoutService.onDidChangeNotificationsVisibility(visible => {
+			if (visible) {
+				this.hide();
+			}
+		}));
 	}
 
-	protected override getAnchor(): HTMLElement | IAnchor {
-		const position = dom.getDomNodePagePosition(this.element);
+	private getAnchor(): HTMLElement | IAnchor {
+		const dimension = this.layoutService.dimension;
 
 		return {
-			x: position.left + position.width, // center above the container
-			y: position.top - 26, // above status bar and beak
-			width: position.width,
-			height: position.height
+			x: dimension.width - 8,
+			y: dimension.height - 31
 		};
 	}
 
-	protected override renderContents(container: HTMLElement): IDisposable {
+	private renderContents(container: HTMLElement): IDisposable {
 		const disposables = new DisposableStore();
 
 		container.classList.add('monaco-menu-container');
@@ -379,40 +384,53 @@ export class FeedbackWidget extends Dropdown {
 		return element;
 	}
 
-	override show(): void {
-		super.show();
-
-		if (this.options.onFeedbackVisibilityChange) {
-			this.options.onFeedbackVisibilityChange(true);
+	show(): void {
+		if (this.visible) {
+			return;
 		}
 
+		this.visible = true;
+		this.contextViewService.showContextView({
+			getAnchor: () => this.getAnchor(),
+
+			render: (container) => {
+				return this.renderContents(container);
+			},
+
+			onDOMEvent: (e, activeElement) => {
+				this.onEvent(e, activeElement);
+			},
+
+			onHide: () => this._onDidChangeVisibility.fire(false)
+		});
+
+		this._onDidChangeVisibility.fire(true);
+
 		this.updateCharCountText();
 	}
 
-	protected override onHide(): void {
-		if (this.options.onFeedbackVisibilityChange) {
-			this.options.onFeedbackVisibilityChange(false);
+	hide(): void {
+		if (!this.visible) {
+			return;
 		}
-	}
 
-	override hide(): void {
 		if (this.feedbackDescriptionInput) {
 			this.feedback = this.feedbackDescriptionInput.value;
 		}
 
-		if (this.autoHideTimeout) {
-			clearTimeout(this.autoHideTimeout);
-			this.autoHideTimeout = undefined;
-		}
-
 		if (this.hideButton && !this.hideButton.checked) {
 			this.statusbarService.updateEntryVisibility('status.feedback', false);
 		}
 
-		super.hide();
+		this.visible = false;
+		this.contextViewService.hideContextView();
+	}
+
+	isVisible(): boolean {
+		return !!this.visible;
 	}
 
-	override onEvent(e: Event, activeElement: HTMLElement): void {
+	private onEvent(e: Event, activeElement: HTMLElement): void {
 		if (e instanceof KeyboardEvent) {
 			const keyboardEvent = <KeyboardEvent>e;
 			if (keyboardEvent.keyCode === 27) { // Escape

src/vs/workbench/contrib/feedback/browser/feedbackStatusbarItem.ts

@@ -5,7 +5,6 @@
 
 import { Disposable } from 'vs/base/common/lifecycle';
 import { FeedbackWidget, IFeedback, IFeedbackDelegate } from 'vs/workbench/contrib/feedback/browser/feedback';
-import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { IProductService } from 'vs/platform/product/common/productService';
 import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
@@ -16,8 +15,6 @@ import { IOpenerService } from 'vs/platform/opener/common/opener';
 import { URI } from 'vs/base/common/uri';
 import { MenuRegistry, MenuId } from 'vs/platform/actions/common/actions';
 import { CATEGORIES } from 'vs/workbench/common/actions';
-import { assertIsDefined } from 'vs/base/common/types';
-import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService';
 import { HIDE_NOTIFICATIONS_CENTER, HIDE_NOTIFICATION_TOAST } from 'vs/workbench/browser/parts/notifications/notificationsCommands';
 import { isIOS } from 'vs/base/common/platform';
 
@@ -65,15 +62,11 @@ export class FeedbackStatusbarConribution extends Disposable implements IWorkben
 		@IStatusbarService private readonly statusbarService: IStatusbarService,
 		@IProductService productService: IProductService,
 		@IInstantiationService private readonly instantiationService: IInstantiationService,
-		@IContextViewService private readonly contextViewService: IContextViewService,
-		@IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService,
-		@ICommandService private readonly commandService: ICommandService
-	) {
+		@ICommandService private readonly commandService: ICommandService) {
 		super();
 
 		if (productService.sendASmile && !isIOS) {
 			this.createFeedbackStatusEntry();
-			this.registerListeners();
 		}
 	}
 
@@ -93,44 +86,22 @@ export class FeedbackStatusbarConribution extends Disposable implements IWorkben
 		});
 	}
 
-	private registerListeners(): void {
-
-		// Hide feedback widget whenever notifications appear
-		this._register(this.layoutService.onDidChangeNotificationsVisibility(visible => {
-			if (visible) {
-				this.widget?.hide();
-			}
-		}));
-	}
-
-	private createFeedbackWidget(): void {
-		const statusContainer = document.getElementById('status.feedback');
-		if (statusContainer) {
-			const icon = assertIsDefined(statusContainer.getElementsByClassName('codicon').item(0) as HTMLElement | null);
-
-			this.widget = this._register(this.instantiationService.createInstance(FeedbackWidget, icon, {
-				contextViewProvider: this.contextViewService,
-				feedbackService: this.instantiationService.createInstance(TwitterFeedbackService),
-				onFeedbackVisibilityChange: visible => this.entry?.update(this.getStatusEntry(visible))
-			}));
-		}
-	}
-
 	private toggleFeedback(): void {
 		if (!this.widget) {
-			this.createFeedbackWidget();
-		}
-
-		// Hide when visible
-		if (this.widget?.isVisible()) {
-			this.widget.hide();
+			this.widget = this._register(this.instantiationService.createInstance(FeedbackWidget, {
+				feedbackService: this.instantiationService.createInstance(TwitterFeedbackService)
+			}));
+			this._register(this.widget.onDidChangeVisibility(visible => this.entry!.update(this.getStatusEntry(visible))));
 		}
 
-		// Show when hidden
-		else {
-			this.commandService.executeCommand(HIDE_NOTIFICATION_TOAST);
-			this.commandService.executeCommand(HIDE_NOTIFICATIONS_CENTER);
-			this.widget?.show();
+		if (this.widget) {
+			if (!this.widget.isVisible()) {
+				this.commandService.executeCommand(HIDE_NOTIFICATION_TOAST);
+				this.commandService.executeCommand(HIDE_NOTIFICATIONS_CENTER);
+				this.widget.show();
+			} else {
+				this.widget.hide();
+			}
 		}
 	}