Callout Dialog Fixes + WYSIWYG Improvements for Insert Link (#14494)

* wip

* Works in all edit modes

* Default value set

* wip

* preventdefault

* cleanup, add tests

* markup -> markdown

* Ensure selection is persisted for WYSIWYG

* Add simple dialog tests and some PR feedback

* floating promise

* PR comments, formatted markdown refactor

* Change escaping logic + PR comments

* PR feedback

src/sql/workbench/contrib/notebook/browser/calloutDialog/common/utils.ts

@@ -0,0 +1,36 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as strings from 'vs/base/common/strings';
+
+/**
+ * Escape string to be used as label in markdown link
+ * @param unescapedLabel label to escape
+ */
+export function escapeLabel(unescapedLabel: string): string {
+	let firstEscape = strings.escape(unescapedLabel);
+	return firstEscape.replace(/[[]]/g, function (match) {
+		switch (match) {
+			case '[': return '\[';
+			case ']': return '\]';
+			default: return match;
+		}
+	});
+}
+
+/**
+ * Escape string to be used as url in markdown link
+ * @param unescapedUrl url to escapes
+ */
+export function escapeUrl(unescapedUrl: string): string {
+	let firstEscape = strings.escape(unescapedUrl);
+	return firstEscape.replace(/[()]/g, function (match) {
+		switch (match) {
+			case '(': return '%28';
+			case ')': return '%29';
+			default: return match;
+		}
+	});
+}

src/sql/workbench/contrib/notebook/browser/calloutDialog/imageCalloutDialog.ts

@@ -5,7 +5,6 @@
 
 import 'vs/css!./media/imageCalloutDialog';
 import * as DOM from 'vs/base/browser/dom';
-import * as strings from 'vs/base/common/strings';
 import * as styler from 'vs/platform/theme/common/styler';
 import { URI } from 'vs/base/common/uri';
 import * as constants from 'sql/workbench/contrib/notebook/browser/calloutDialog/common/constants';
@@ -27,10 +26,11 @@ import { Checkbox } from 'sql/base/browser/ui/checkbox/checkbox';
 import { RadioButton } from 'sql/base/browser/ui/radioButton/radioButton';
 import { DialogWidth } from 'sql/workbench/api/common/sqlExtHostTypes';
 import { attachModalDialogStyler } from 'sql/workbench/common/styler';
+import { escapeUrl } from 'sql/workbench/contrib/notebook/browser/calloutDialog/common/utils';
 
 export interface IImageCalloutDialogOptions {
 	insertTitle?: string,
-	insertMarkup?: string,
+	insertEscapedMarkdown?: string,
 	imagePath?: string,
 	embedImage?: boolean
 }
@@ -186,7 +186,7 @@ export class ImageCalloutDialog extends CalloutDialog<IImageCalloutDialogOptions
 	public insert(): void {
 		this.hide();
 		this._selectionComplete.resolve({
-			insertMarkup: `<img src="${strings.escape(this._imageUrlInputBox.value)}">`,
+			insertEscapedMarkdown: `![](${escapeUrl(this._imageUrlInputBox.value)})`,
 			imagePath: this._imageUrlInputBox.value,
 			embedImage: this._imageEmbedCheckbox.checked
 		});
@@ -196,7 +196,7 @@ export class ImageCalloutDialog extends CalloutDialog<IImageCalloutDialogOptions
 	public cancel(): void {
 		super.cancel();
 		this._selectionComplete.resolve({
-			insertMarkup: '',
+			insertEscapedMarkdown: '',
 			imagePath: undefined,
 			embedImage: undefined
 		});

src/sql/workbench/contrib/notebook/browser/calloutDialog/linkCalloutDialog.ts

@@ -5,7 +5,6 @@
 
 import 'vs/css!./media/linkCalloutDialog';
 import * as DOM from 'vs/base/browser/dom';
-import * as strings from 'vs/base/common/strings';
 import * as styler from 'vs/platform/theme/common/styler';
 import * as constants from 'sql/workbench/contrib/notebook/browser/calloutDialog/common/constants';
 import { CalloutDialog } from 'sql/workbench/browser/modal/calloutDialog';
@@ -20,12 +19,17 @@ import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry';
 import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
 import { Deferred } from 'sql/base/common/promise';
 import { InputBox } from 'sql/base/browser/ui/inputBox/inputBox';
-import { DialogWidth } from 'sql/workbench/api/common/sqlExtHostTypes';
 import { attachModalDialogStyler } from 'sql/workbench/common/styler';
+import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
+import { escapeLabel, escapeUrl } from 'sql/workbench/contrib/notebook/browser/calloutDialog/common/utils';
+
+const DEFAULT_DIALOG_WIDTH = 452;
 
 export interface ILinkCalloutDialogOptions {
 	insertTitle?: string,
-	insertMarkup?: string
+	insertEscapedMarkdown?: string,
+	insertUnescapedLinkLabel?: string,
+	insertUnescapedLinkUrl?: string
 }
 
 export class LinkCalloutDialog extends CalloutDialog<ILinkCalloutDialogOptions> {
@@ -34,11 +38,12 @@ export class LinkCalloutDialog extends CalloutDialog<ILinkCalloutDialogOptions>
 	private _linkTextInputBox: InputBox;
 	private _linkAddressLabel: HTMLElement;
 	private _linkUrlInputBox: InputBox;
+	private _previouslySelectedRange: Range;
 
 	constructor(
 		title: string,
-		width: DialogWidth,
 		dialogProperties: IDialogProperties,
+		private readonly _defaultLabel: string = '',
 		@IContextViewService private readonly _contextViewService: IContextViewService,
 		@IThemeService themeService: IThemeService,
 		@ILayoutService layoutService: ILayoutService,
@@ -50,7 +55,7 @@ export class LinkCalloutDialog extends CalloutDialog<ILinkCalloutDialogOptions>
 	) {
 		super(
 			title,
-			width,
+			DEFAULT_DIALOG_WIDTH,
 			dialogProperties,
 			themeService,
 			layoutService,
@@ -60,12 +65,17 @@ export class LinkCalloutDialog extends CalloutDialog<ILinkCalloutDialogOptions>
 			logService,
 			textResourcePropertiesService
 		);
+		let selection = window.getSelection();
+		if (selection.rangeCount > 0) {
+			this._previouslySelectedRange = selection?.getRangeAt(0);
+		}
 	}
 
 	/**
 	 * Opens the dialog and returns a promise for what options the user chooses.
 	 */
 	public open(): Promise<ILinkCalloutDialogOptions> {
+		this._selectionComplete = new Deferred<ILinkCalloutDialogOptions>();
 		this.show();
 		return this._selectionComplete.promise;
 	}
@@ -97,6 +107,7 @@ export class LinkCalloutDialog extends CalloutDialog<ILinkCalloutDialogOptions>
 				placeholder: constants.linkTextPlaceholder,
 				ariaLabel: constants.linkTextLabel
 			});
+		this._linkTextInputBox.value = this._defaultLabel;
 		DOM.append(linkTextRow, linkTextInputContainer);
 
 		let linkAddressRow = DOM.$('.row');
@@ -121,18 +132,45 @@ export class LinkCalloutDialog extends CalloutDialog<ILinkCalloutDialogOptions>
 		this._register(styler.attachInputBoxStyler(this._linkUrlInputBox, this._themeService));
 	}
 
+	protected onAccept(e?: StandardKeyboardEvent) {
+		// EventHelper.stop() will call preventDefault. Without it, text cell will insert an extra newline when pressing enter on dialog
+		DOM.EventHelper.stop(e, true);
+		this.insert();
+	}
+
+	protected onClose(e?: StandardKeyboardEvent) {
+		DOM.EventHelper.stop(e, true);
+		this.cancel();
+	}
+
 	public insert(): void {
 		this.hide();
-		this._selectionComplete.resolve({
-			insertMarkup: `<a href="${strings.escape(this._linkUrlInputBox.value)}">${strings.escape(this._linkTextInputBox.value)}</a>`,
-		});
-		this.dispose();
+		let escapedLabel = escapeLabel(this._linkTextInputBox.value);
+		let escapedUrl = escapeUrl(this._linkUrlInputBox.value);
+
+		if (this._previouslySelectedRange) {
+			// Reset selection to previous state before callout was open
+			let selection = window.getSelection();
+			selection.removeAllRanges();
+			selection.addRange(this._previouslySelectedRange);
+
+			this._selectionComplete.resolve({
+				insertEscapedMarkdown: `[${escapedLabel}](${escapedUrl})`,
+				insertUnescapedLinkLabel: this._linkTextInputBox.value,
+				insertUnescapedLinkUrl: this._linkUrlInputBox.value
+			});
+		}
 	}
 
 	public cancel(): void {
 		super.cancel();
 		this._selectionComplete.resolve({
-			insertMarkup: ''
+			insertEscapedMarkdown: '',
+			insertUnescapedLinkLabel: escapeLabel(this._linkTextInputBox.value)
 		});
 	}
+
+	public set url(val: string) {
+		this._linkUrlInputBox.value = val;
+	}
 }

src/sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component.ts

@@ -7,15 +7,18 @@ import * as DOM from 'vs/base/browser/dom';
 import { Button, IButtonStyles } from 'sql/base/browser/ui/button/button';
 import { Component, Input, Inject, ViewChild, ElementRef } from '@angular/core';
 import { localize } from 'vs/nls';
-import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
+import { CellEditModes, ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces';
 import { ITaskbarContent, Taskbar } from 'sql/base/browser/ui/taskbar/taskbar';
 import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
-import { TransformMarkdownAction, MarkdownTextTransformer, MarkdownButtonType, ToggleViewAction } from 'sql/workbench/contrib/notebook/browser/markdownToolbarActions';
-import { INotebookService } from 'sql/workbench/services/notebook/browser/notebookService';
+import { TransformMarkdownAction, MarkdownTextTransformer, MarkdownButtonType, ToggleViewAction, insertFormattedMarkdown } from 'sql/workbench/contrib/notebook/browser/markdownToolbarActions';
+import { ICellEditorProvider, INotebookEditor, INotebookService } from 'sql/workbench/services/notebook/browser/notebookService';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { DropdownMenuActionViewItem } from 'sql/base/browser/ui/buttonMenu/buttonMenu';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { AngularDisposable } from 'sql/base/browser/lifecycle';
+import { ILinkCalloutDialogOptions, LinkCalloutDialog } from 'sql/workbench/contrib/notebook/browser/calloutDialog/linkCalloutDialog';
+import { TextModel } from 'vs/editor/common/model/textModel';
+import { IEditor } from 'vs/editor/common/editorCommon';
 
 export const MARKDOWN_TOOLBAR_SELECTOR: string = 'markdown-toolbar-component';
 
@@ -43,6 +46,8 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 	public optionHeading2 = localize('optionHeading2', "Heading 2");
 	public optionHeading3 = localize('optionHeading3', "Heading 3");
 	public optionParagraph = localize('optionParagraph', "Paragraph");
+	public insertLinkHeading = localize('callout.insertLinkHeading', "Insert link");
+	public insertImageHeading = localize('callout.insertImageHeading', "Insert image");
 
 	public richTextViewButton = localize('richTextViewButton', "Rich Text View");
 	public splitViewButton = localize('splitViewButton', "Split View");
@@ -51,12 +56,16 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 	private _taskbarContent: Array<ITaskbarContent>;
 	private _wysiwygTaskbarContent: Array<ITaskbarContent>;
 	private _previewModeTaskbarContent: Array<ITaskbarContent>;
+	private _linkCallout: LinkCalloutDialog;
 
 	@Input() public cellModel: ICellModel;
+	@Input() public output: ElementRef;
 	private _actionBar: Taskbar;
 	_toggleTextViewAction: ToggleViewAction;
 	_toggleSplitViewAction: ToggleViewAction;
 	_toggleMarkdownViewAction: ToggleViewAction;
+	private _notebookEditor: INotebookEditor;
+	private _cellEditor: ICellEditorProvider;
 
 	constructor(
 		@Inject(INotebookService) private _notebookService: INotebookService,
@@ -92,8 +101,8 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 			};
 			linkButton.style(buttonStyle);
 
-			this._register(DOM.addDisposableListener(linkButtonContainer, DOM.EventType.CLICK, e => {
-				this.onInsertButtonClick(e, MarkdownButtonType.LINK_PREVIEW);
+			this._register(DOM.addDisposableListener(linkButtonContainer, DOM.EventType.CLICK, async e => {
+				await this.onInsertButtonClick(e, MarkdownButtonType.LINK_PREVIEW);
 			}));
 
 			imageButtonContainer = DOM.$('li.action-item');
@@ -103,8 +112,8 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 
 			imageButton.style(buttonStyle);
 
-			this._register(DOM.addDisposableListener(imageButtonContainer, DOM.EventType.CLICK, e => {
-				this.onInsertButtonClick(e, MarkdownButtonType.IMAGE_PREVIEW);
+			this._register(DOM.addDisposableListener(imageButtonContainer, DOM.EventType.CLICK, async e => {
+				await this.onInsertButtonClick(e, MarkdownButtonType.IMAGE_PREVIEW);
 			}));
 		} else {
 			linkButton = this._instantiationService.createInstance(TransformMarkdownAction, 'notebook.linkText', '', 'insert-link masked-icon', this.buttonLink, this.cellModel, MarkdownButtonType.LINK);
@@ -169,6 +178,7 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 			{ action: underlineButton },
 			{ action: highlightButton },
 			{ action: codeButton },
+			{ element: linkButtonContainer },
 			{ action: listButton },
 			{ action: orderedListButton },
 			{ element: buttonDropdownContainer },
@@ -203,15 +213,40 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 				this._actionBar.setContent(this._taskbarContent);
 			}
 		}
+		this._notebookEditor = this._notebookService.findNotebookEditor(this.cellModel?.notebookModel?.notebookUri);
 	}
 
-	public onInsertButtonClick(event: MouseEvent, type: MarkdownButtonType): void {
-		let go = new MarkdownTextTransformer(this._notebookService, this.cellModel, this._instantiationService);
-		let trigger = event.target as HTMLElement;
-		go.transformText(type, trigger);
+	public async onInsertButtonClick(event: MouseEvent, type: MarkdownButtonType): Promise<void> {
+		DOM.EventHelper.stop(event, true);
+		let triggerElement = event.target as HTMLElement;
+		let needsTransform = true;
+		let calloutResult: ILinkCalloutDialogOptions;
+		if (type === MarkdownButtonType.LINK_PREVIEW) {
+			calloutResult = await this.createCallout(type, triggerElement);
+			// If no URL is present, no-op
+			if (!calloutResult.insertUnescapedLinkUrl) {
+				return;
+			}
+			// If cell edit mode isn't WYSIWYG, use result from callout. No need for further transformation.
+			if (this.cellModel.currentMode !== CellEditModes.WYSIWYG) {
+				needsTransform = false;
+			} else {
+				// Otherwise, re-focus on the output element, and insert the link directly.
+				this.output?.nativeElement?.focus();
+				// Callout is responsible for returning escaped strings
+				document.execCommand('insertHTML', false, `<a href="${calloutResult?.insertUnescapedLinkUrl}">${calloutResult?.insertUnescapedLinkLabel}</a>`);
+				return;
+			}
+		}
+		const transformer = new MarkdownTextTransformer(this._notebookService, this.cellModel);
+		if (needsTransform) {
+			await transformer.transformText(type);
+		} else if (!needsTransform) {
+			await insertFormattedMarkdown(calloutResult?.insertEscapedMarkdown, this.getCellEditorControl());
+		}
 	}
 
-	public hideLinkAndImageButtons() {
+	public hideImageButton() {
 		this._actionBar.setContent(this._wysiwygTaskbarContent);
 	}
 
@@ -231,4 +266,52 @@ export class MarkdownToolbarComponent extends AngularDisposable {
 			}
 		}
 	}
+
+	/**
+	 * Instantiate modal for use as callout when inserting Link or Image into markdown.
+	 * @param calloutStyle Style of callout passed in to determine which callout is rendered.
+	 * Returns markup created after user enters values and submits the callout.
+	 */
+	private async createCallout(type: MarkdownButtonType, triggerElement: HTMLElement): Promise<ILinkCalloutDialogOptions> {
+		const triggerPosX = triggerElement.getBoundingClientRect().left;
+		const triggerPosY = triggerElement.getBoundingClientRect().top;
+		const triggerHeight = triggerElement.offsetHeight;
+		const triggerWidth = triggerElement.offsetWidth;
+		const dialogProperties = { xPos: triggerPosX, yPos: triggerPosY, width: triggerWidth, height: triggerHeight };
+		let calloutOptions;
+
+		if (type === MarkdownButtonType.LINK_PREVIEW) {
+			const defaultLabel = this.getCurrentSelectionText();
+			this._linkCallout = this._instantiationService.createInstance(LinkCalloutDialog, this.insertLinkHeading, dialogProperties, defaultLabel);
+			this._linkCallout.render();
+			calloutOptions = await this._linkCallout.open();
+		}
+		return calloutOptions;
+	}
+
+	private getCurrentSelectionText(): string {
+		if (this.cellModel.currentMode === CellEditModes.WYSIWYG) {
+			return document.getSelection()?.toString() || '';
+		} else {
+			const editorControl = this.getCellEditorControl();
+			const selection = editorControl?.getSelection();
+			if (selection && !selection.isEmpty()) {
+				const textModel = editorControl?.getModel() as TextModel;
+				const value = textModel?.getValueInRange(selection);
+				return value || '';
+			}
+			return '';
+		}
+	}
+
+	private getCellEditorControl(): IEditor | undefined {
+		// If control doesn't exist, editor may have been destroyed previously when switching edit modes
+		if (!this._cellEditor?.getEditor()?.getControl()) {
+			this._cellEditor = this._notebookEditor?.cellEditors?.find(e => e.cellGuid() === this.cellModel?.cellGuid);
+		}
+		if (this._cellEditor?.hasEditor) {
+			return this._cellEditor.getEditor()?.getControl();
+		}
+		return undefined;
+	}
 }

src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html

@@ -5,7 +5,7 @@
  *--------------------------------------------------------------------------------------------*/
 -->
 <div style="width: 100%; height: 100%; display: flex; flex-flow: column" (mouseover)="hover=true" (mouseleave)="hover=false">
-	<markdown-toolbar-component #markdownToolbar *ngIf="isEditMode" [cellModel]="cellModel"></markdown-toolbar-component>
+	<markdown-toolbar-component #markdownToolbar *ngIf="isEditMode" [cellModel]="cellModel" [output]="output"></markdown-toolbar-component>
 	<div class="notebook-text" [class.show-markdown]="markdownMode" [class.show-preview]="previewMode">
 		<code-component *ngIf="markdownMode" [cellModel]="cellModel" (onContentChanged)="handleContentChanged()" [model]="model" [activeCellId]="activeCellId">
 		</code-component>

src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts

@@ -5,6 +5,7 @@
 import 'vs/css!./textCell';
 import 'vs/css!./media/markdown';
 import 'vs/css!./media/highlight';
+import * as DOM from 'vs/base/browser/dom';
 
 import { OnInit, Component, Input, Inject, forwardRef, ElementRef, ChangeDetectorRef, ViewChild, OnChanges, SimpleChange, HostListener, ViewChildren, QueryList } from '@angular/core';
 import * as Mark from 'mark.js';
@@ -68,7 +69,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges {
 
 	@HostListener('document:keydown', ['$event'])
 	onkeydown(e: KeyboardEvent) {
-		if (this.isActive() && this.cellModel?.currentMode === CellEditModes.WYSIWYG) {
+		if (DOM.getActiveElement() === this.output?.nativeElement && this.isActive() && this.cellModel?.currentMode === CellEditModes.WYSIWYG) {
 			// select the active .
 			if ((e.ctrlKey || e.metaKey) && e.key === 'a') {
 				preventDefaultAndExecCommand(e, 'selectAll');

src/sql/workbench/contrib/notebook/browser/markdownToolbarActions.ts

@@ -4,7 +4,6 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { Action } from 'vs/base/common/actions';
-import { localize } from 'vs/nls';
 import { INotebookEditor, INotebookService } from 'sql/workbench/services/notebook/browser/notebookService';
 import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditorWidget';
 import { IRange, Range } from 'vs/editor/common/core/range';
@@ -16,10 +15,7 @@ import { Selection } from 'vs/editor/common/core/selection';
 import { EditOperation } from 'vs/editor/common/core/editOperation';
 import { Position } from 'vs/editor/common/core/position';
 import { MarkdownToolbarComponent } from 'sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component';
-import { ImageCalloutDialog } from 'sql/workbench/contrib/notebook/browser/calloutDialog/imageCalloutDialog';
-import { LinkCalloutDialog } from 'sql/workbench/contrib/notebook/browser/calloutDialog/linkCalloutDialog';
-import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
-import { DialogWidth } from 'sql/workbench/api/common/sqlExtHostTypes';
+import { IEditor } from 'vs/editor/common/editorCommon';
 
 export class TransformMarkdownAction extends Action {
 
@@ -30,8 +26,7 @@ export class TransformMarkdownAction extends Action {
 		tooltip: string,
 		private _cellModel: ICellModel,
 		private _type: MarkdownButtonType,
-		@INotebookService private _notebookService: INotebookService,
-		@IInstantiationService private _instantiationService: IInstantiationService
+		@INotebookService private _notebookService: INotebookService
 	) {
 		super(id, label, cssClass);
 		this._tooltip = tooltip;
@@ -40,7 +35,7 @@ export class TransformMarkdownAction extends Action {
 		if (!context?.cellModel?.showMarkdown && context?.cellModel?.showPreview) {
 			this.transformDocumentCommand();
 		} else {
-			let markdownTextTransformer = new MarkdownTextTransformer(this._notebookService, this._cellModel, this._instantiationService);
+			let markdownTextTransformer = new MarkdownTextTransformer(this._notebookService, this._cellModel);
 			await markdownTextTransformer.transformText(this._type);
 		}
 		return true;
@@ -131,22 +126,17 @@ export class TransformMarkdownAction extends Action {
 }
 
 export class MarkdownTextTransformer {
-	private _imageCallout: ImageCalloutDialog;
-	private _linkCallout: LinkCalloutDialog;
-	private readonly insertLinkHeading = localize('callout.insertLinkHeading', "Insert link");
-	private readonly insertImageHeading = localize('callout.insertImageHeading', "Insert image");
 
 	constructor(
 		private _notebookService: INotebookService,
 		private _cellModel: ICellModel,
-		private _instantiationService: IInstantiationService,
 		private _notebookEditor?: INotebookEditor) { }
 
 	public get notebookEditor(): INotebookEditor {
 		return this._notebookEditor;
 	}
 
-	public async transformText(type: MarkdownButtonType, triggerElement?: HTMLElement): Promise<void> {
+	public async transformText(type: MarkdownButtonType): Promise<void> {
 		let editorControl = this.getEditorControl();
 		if (editorControl) {
 			let selections = editorControl.getSelections();
@@ -160,15 +150,8 @@ export class MarkdownTextTransformer {
 				endLineNumber: selection.startLineNumber
 			};
 
-			let beginInsertedText: string;
-			let endInsertedText: string;
-
-			if (type === MarkdownButtonType.IMAGE_PREVIEW || type === MarkdownButtonType.LINK_PREVIEW) {
-				beginInsertedText = await this.createCallout(type, triggerElement);
-			} else {
-				beginInsertedText = getStartTextToInsert(type);
-				endInsertedText = getEndTextToInsert(type);
-			}
+			let beginInsertedText = getStartTextToInsert(type);
+			let endInsertedText = getEndTextToInsert(type);
 
 			let endRange: IRange = {
 				startColumn: selection.endColumn,
@@ -200,41 +183,6 @@ export class MarkdownTextTransformer {
 		}
 	}
 
-	/**
-	 * Instantiate modal for use as callout when inserting Link or Image into markdown.
-	 * @param calloutStyle Style of callout passed in to determine which callout is rendered.
-	 * Returns markup created after user enters values and submits the callout.
-	 */
-	private async createCallout(type: MarkdownButtonType, triggerElement: HTMLElement): Promise<string> {
-		const triggerPosX = triggerElement.getBoundingClientRect().left;
-		const triggerPosY = triggerElement.getBoundingClientRect().top;
-		const triggerHeight = triggerElement.offsetHeight;
-		const triggerWidth = triggerElement.offsetWidth;
-		const dialogProperties = { xPos: triggerPosX, yPos: triggerPosY, width: triggerWidth, height: triggerHeight };
-		let calloutOptions;
-		/**
-		 * Width value here reflects designs for Notebook callouts.
-		 */
-		const width: DialogWidth = 452;
-
-		if (type === MarkdownButtonType.IMAGE_PREVIEW) {
-			if (!this._imageCallout) {
-				this._imageCallout = this._instantiationService.createInstance(ImageCalloutDialog, this.insertImageHeading, width, dialogProperties);
-				this._imageCallout.render();
-				calloutOptions = await this._imageCallout.open();
-				calloutOptions.insertTitle = this.insertImageHeading;
-			}
-		} else {
-			if (!this._linkCallout) {
-				this._linkCallout = this._instantiationService.createInstance(LinkCalloutDialog, this.insertLinkHeading, width, dialogProperties);
-				this._linkCallout.render();
-				calloutOptions = await this._linkCallout.open();
-				calloutOptions.insertTitle = this.insertLinkHeading;
-			}
-		}
-		return calloutOptions.insertMarkup;
-	}
-
 	private getEditorControl(): CodeEditorWidget | undefined {
 		if (!this._notebookEditor) {
 			this._notebookEditor = this._notebookService.findNotebookEditor(this._cellModel?.notebookModel?.notebookUri);
@@ -616,6 +564,36 @@ function getColumnOffsetForSelection(type: MarkdownButtonType, nothingSelected:
 	}
 }
 
+/**
+ * When markdown is already formatted correctly and doesn't need transformed, insert markdown based on current editor selection
+ * @param markdownToInsert formatted markdown
+ * @param editorControl editor control for cell
+ */
+export async function insertFormattedMarkdown(markdownToInsert: string, editorControl?: IEditor): Promise<void> {
+	if (editorControl) {
+		let selections = editorControl.getSelections();
+		let selection = selections[0];
+		let startRange: IRange = {
+			startColumn: selection.startColumn,
+			endColumn: selection.startColumn,
+			startLineNumber: selection.startLineNumber,
+			endLineNumber: selection.startLineNumber
+		};
+
+		let editorModel = editorControl.getModel() as TextModel;
+
+		startRange = {
+			startColumn: selection.startColumn,
+			endColumn: selection.endColumn,
+			startLineNumber: selection.startLineNumber,
+			endLineNumber: selection.endLineNumber
+		};
+		editorModel.pushEditOperations(selections, [
+			{ range: startRange, text: markdownToInsert },
+		], undefined);
+	}
+}
+
 export class ToggleViewAction extends Action {
 	constructor(
 		id: string,
@@ -634,9 +612,9 @@ export class ToggleViewAction extends Action {
 		this.class += ' active';
 		context.cellModel.showPreview = this.showPreview;
 		context.cellModel.showMarkdown = this.showMarkdown;
-		// Hide link and image buttons in WYSIWYG mode
+		// Hide image button in WYSIWYG mode
 		if (this.showPreview && !this.showMarkdown) {
-			context.hideLinkAndImageButtons();
+			context.hideImageButton();
 		} else {
 			context.showLinkAndImageButtons();
 		}

src/sql/workbench/contrib/notebook/test/browser/markdownTextTransformer.test.ts

@@ -6,7 +6,7 @@
 import * as TypeMoq from 'typemoq';
 import * as assert from 'assert';
 
-import { MarkdownTextTransformer, MarkdownButtonType } from 'sql/workbench/contrib/notebook/browser/markdownToolbarActions';
+import { MarkdownTextTransformer, MarkdownButtonType, insertFormattedMarkdown } from 'sql/workbench/contrib/notebook/browser/markdownToolbarActions';
 import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl';
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { TestLifecycleService, TestEnvironmentService, TestAccessibilityService } from 'vs/workbench/test/browser/workbenchTestServices';
@@ -73,7 +73,7 @@ suite('MarkdownTextTransformer', () => {
 
 		cellModel = new CellModel(undefined, undefined, mockNotebookService.object);
 		notebookEditor = new NotebookEditorStub({ cellGuid: cellModel.cellGuid, instantiationService: instantiationService });
-		markdownTextTransformer = new MarkdownTextTransformer(mockNotebookService.object, cellModel, instantiationService, notebookEditor);
+		markdownTextTransformer = new MarkdownTextTransformer(mockNotebookService.object, cellModel, notebookEditor);
 		mockNotebookService.setup(s => s.findNotebookEditor(TypeMoq.It.isAny())).returns(() => notebookEditor);
 
 		let editor = notebookEditor.cellEditors[0].getEditor();
@@ -114,10 +114,12 @@ suite('MarkdownTextTransformer', () => {
 		await testWithNoSelection(MarkdownButtonType.HEADING2, '');
 		await testWithNoSelection(MarkdownButtonType.HEADING3, '### ', true);
 		await testWithNoSelection(MarkdownButtonType.HEADING3, '');
+		await testPreviouslyTransformedWithNoSelection(MarkdownButtonType.LINK_PREVIEW, '[test](./URL)', true);
 	});
 
 	test('Transform text with one word selected', async () => {
 		await testWithSingleWordSelected(MarkdownButtonType.CODE, '```\nWORD\n```');
+		await testPreviouslyTransformedWithSingleWordSelected(MarkdownButtonType.LINK_PREVIEW, '[SampleURL](https://aka.ms)');
 	});
 
 	test('Transform text with multiple words selected', async () => {
@@ -148,7 +150,7 @@ suite('MarkdownTextTransformer', () => {
 	test('Ensure notebook editor returns expected object', async () => {
 		assert.deepEqual(notebookEditor, markdownTextTransformer.notebookEditor, 'Notebook editor does not match expected value');
 		// Set markdown text transformer to not have a notebook editor passed in
-		markdownTextTransformer = new MarkdownTextTransformer(mockNotebookService.object, cellModel, instantiationService);
+		markdownTextTransformer = new MarkdownTextTransformer(mockNotebookService.object, cellModel);
 		assert.equal(markdownTextTransformer.notebookEditor, undefined, 'No notebook editor should be returned');
 		// Even after text is attempted to be transformed, there should be no editor, and therefore nothing on the text model
 		await markdownTextTransformer.transformText(MarkdownButtonType.BOLD);
@@ -164,6 +166,15 @@ suite('MarkdownTextTransformer', () => {
 		assert.equal(textModel.getValue(), expectedValue, `${MarkdownButtonType[type]} with no selection failed (setValue ${setValue})`);
 	}
 
+
+	async function testPreviouslyTransformedWithNoSelection(type: MarkdownButtonType, expectedValue: string, setValue = false): Promise<void> {
+		if (setValue) {
+			textModel.setValue('');
+		}
+		await insertFormattedMarkdown('[test](./URL)', widget);
+		assert.equal(textModel.getValue(), expectedValue, `${MarkdownButtonType[type]} with no selection and previously transformed md failed (setValue ${setValue})`);
+	}
+
 	async function testWithSingleWordSelected(type: MarkdownButtonType, expectedValue: string): Promise<void> {
 		let value = 'WORD';
 		textModel.setValue(value);
@@ -183,6 +194,18 @@ suite('MarkdownTextTransformer', () => {
 		assert.equal(textModel.getValue(), value, `Undo operation for ${MarkdownButtonType[type]} with single word selection failed`);
 	}
 
+	async function testPreviouslyTransformedWithSingleWordSelected(type: MarkdownButtonType, expectedValue: string): Promise<void> {
+		let value = 'WORD';
+		textModel.setValue(value);
+
+		// Test transformation (adding text)
+		widget.setSelection({ startColumn: 1, startLineNumber: 1, endColumn: value.length + 1, endLineNumber: 1 });
+		assert.equal(textModel.getValueInRange(widget.getSelection()), value, 'Expected selection is not found');
+		await insertFormattedMarkdown('[SampleURL](https://aka.ms)', widget);
+		const textModelValue = textModel.getValue();
+		assert.equal(textModelValue, expectedValue, `${MarkdownButtonType[type]} with single word selection and previously transformed md failed`);
+	}
+
 	async function testWithMultipleWordsSelected(type: MarkdownButtonType, expectedValue: string): Promise<void> {
 		let value = 'Multi Words';
 		textModel.setValue(value);

src/sql/workbench/contrib/notebook/test/calloutDialog/linkCalloutDialog.test.ts

@@ -0,0 +1,106 @@
+import * as assert from 'assert';
+
+import { ILinkCalloutDialogOptions, LinkCalloutDialog } from 'sql/workbench/contrib/notebook/browser/calloutDialog/linkCalloutDialog';
+import { TestLayoutService } from 'vs/workbench/test/browser/workbenchTestServices';
+import { ILayoutService } from 'vs/platform/layout/browser/layoutService';
+import { IThemeService } from 'vs/platform/theme/common/themeService';
+import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService';
+import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService';
+import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry';
+import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
+import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { Deferred } from 'sql/base/common/promise';
+import { escapeLabel, escapeUrl } from 'sql/workbench/contrib/notebook/browser/calloutDialog/common/utils';
+
+suite('Link Callout Dialog', function (): void {
+	let layoutService: ILayoutService;
+	let themeService: IThemeService;
+	let telemetryService: IAdsTelemetryService;
+	let contextKeyService: IContextKeyService;
+
+	setup(() => {
+		layoutService = new TestLayoutService();
+		themeService = new TestThemeService();
+		telemetryService = new NullAdsTelemetryService();
+		contextKeyService = new MockContextKeyService();
+	});
+
+	test('Should return empty markdown on cancel', async function (): Promise<void> {
+		let linkCalloutDialog = new LinkCalloutDialog('Title', undefined, 'defaultLabel',
+			undefined, themeService, layoutService, telemetryService, contextKeyService, undefined, undefined, undefined);
+		linkCalloutDialog.render();
+
+		let deferred = new Deferred<ILinkCalloutDialogOptions>();
+		// When I first open the callout dialog
+		linkCalloutDialog.open().then(value => {
+			deferred.resolve(value);
+		});
+		// And cancel the dialog
+		linkCalloutDialog.cancel();
+		let result = await deferred.promise;
+
+		assert.equal(result.insertUnescapedLinkLabel, 'defaultLabel', 'Label not returned correctly');
+		assert.equal(result.insertUnescapedLinkUrl, undefined, 'URL not returned correctly');
+		assert.equal(result.insertEscapedMarkdown, '', 'Markdown not returned correctly');
+	});
+
+	test('Should return expected values on insert', async function (): Promise<void> {
+		const defaultLabel = 'defaultLabel';
+		const sampleUrl = 'https://www.aka.ms/azuredatastudio';
+		let linkCalloutDialog = new LinkCalloutDialog('Title', undefined, defaultLabel,
+			undefined, themeService, layoutService, telemetryService, contextKeyService, undefined, undefined, undefined);
+		linkCalloutDialog.render();
+
+		let deferred = new Deferred<ILinkCalloutDialogOptions>();
+		// When I first open the callout dialog
+		linkCalloutDialog.open().then(value => {
+			deferred.resolve(value);
+		});
+
+		linkCalloutDialog.url = sampleUrl;
+
+		// And insert the dialog
+		linkCalloutDialog.insert();
+		let result = await deferred.promise;
+		assert.equal(result.insertUnescapedLinkLabel, defaultLabel, 'Label not returned correctly');
+		assert.equal(result.insertUnescapedLinkUrl, sampleUrl, 'URL not returned correctly');
+		assert.equal(result.insertEscapedMarkdown, `[${defaultLabel}](${sampleUrl})`, 'Markdown not returned correctly');
+	});
+
+	test('Should return expected values on insert when escape necessary', async function (): Promise<void> {
+		const defaultLabel = 'default[]Label';
+		const sampleUrl = 'https://www.aka.ms/azuredatastudio()';
+		let linkCalloutDialog = new LinkCalloutDialog('Title', undefined, defaultLabel,
+			undefined, themeService, layoutService, telemetryService, contextKeyService, undefined, undefined, undefined);
+		linkCalloutDialog.render();
+
+		let deferred = new Deferred<ILinkCalloutDialogOptions>();
+		// When I first open the callout dialog
+		linkCalloutDialog.open().then(value => {
+			deferred.resolve(value);
+		});
+
+		linkCalloutDialog.url = sampleUrl;
+
+		// And insert the dialog
+		linkCalloutDialog.insert();
+		let result = await deferred.promise;
+		assert.equal(result.insertUnescapedLinkLabel, defaultLabel, 'Label not returned correctly');
+		assert.equal(result.insertUnescapedLinkUrl, sampleUrl, 'URL not returned correctly');
+		assert.equal(result.insertEscapedMarkdown, '[default\[\]Label](https://www.aka.ms/azuredatastudio%28%29)', 'Markdown not returned correctly');
+	});
+
+	test('Label escape', function (): void {
+		assert.equal(escapeLabel('TestLabel'), 'TestLabel', 'Basic escape label test failed');
+		assert.equal(escapeLabel('Test[]Label'), 'Test\[\]Label', 'Label test square brackets failed');
+		assert.equal(escapeLabel('<>&[]'), '&lt;&gt;&amp;\[\]', 'Label test known escaped characters failed');
+		assert.equal(escapeLabel('<>&[]()'), '&lt;&gt;&amp;\[\]()', 'Label test all escaped characters failed');
+	});
+
+	test('URL escape', function (): void {
+		assert.equal(escapeUrl('TestURL'), 'TestURL', 'Basic escape URL test failed');
+		assert.equal(escapeUrl('Test()URL'), 'Test%28%29URL', 'URL test square brackets failed');
+		assert.equal(escapeUrl('<>&()'), '&lt;&gt;&amp;%28%29', 'URL test known escaped characters failed');
+		assert.equal(escapeUrl('<>&()[]'), '&lt;&gt;&amp;%28%29[]', 'URL test all escaped characters failed');
+	});
+});