From 69a35b38b2f50bad64f260bb23cfabb4f8eb46f9 Mon Sep 17 00:00:00 2001 From: Chris LaFreniere <40371649+chlafreniere@users.noreply.github.com> Date: Thu, 4 Mar 2021 12:51:13 -0800 Subject: [PATCH] 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 --- .../browser/calloutDialog/common/utils.ts | 36 ++++++ .../calloutDialog/imageCalloutDialog.ts | 8 +- .../calloutDialog/linkCalloutDialog.ts | 58 ++++++++-- .../cellViews/markdownToolbar.component.ts | 107 ++++++++++++++++-- .../browser/cellViews/textCell.component.html | 2 +- .../browser/cellViews/textCell.component.ts | 3 +- .../browser/markdownToolbarActions.ts | 98 +++++++--------- .../browser/markdownTextTransformer.test.ts | 29 ++++- .../calloutDialog/linkCalloutDialog.test.ts | 106 +++++++++++++++++ 9 files changed, 356 insertions(+), 91 deletions(-) create mode 100644 src/sql/workbench/contrib/notebook/browser/calloutDialog/common/utils.ts create mode 100644 src/sql/workbench/contrib/notebook/test/calloutDialog/linkCalloutDialog.test.ts diff --git a/src/sql/workbench/contrib/notebook/browser/calloutDialog/common/utils.ts b/src/sql/workbench/contrib/notebook/browser/calloutDialog/common/utils.ts new file mode 100644 index 0000000000..b602365945 --- /dev/null +++ b/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; + } + }); +} diff --git a/src/sql/workbench/contrib/notebook/browser/calloutDialog/imageCalloutDialog.ts b/src/sql/workbench/contrib/notebook/browser/calloutDialog/imageCalloutDialog.ts index 09aca869d3..6b3a1cc845 100644 --- a/src/sql/workbench/contrib/notebook/browser/calloutDialog/imageCalloutDialog.ts +++ b/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`, + insertEscapedMarkdown: `![](${escapeUrl(this._imageUrlInputBox.value)})`, imagePath: this._imageUrlInputBox.value, embedImage: this._imageEmbedCheckbox.checked }); @@ -196,7 +196,7 @@ export class ImageCalloutDialog extends CalloutDialog { @@ -34,11 +38,12 @@ export class LinkCalloutDialog extends CalloutDialog 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 ) { super( title, - width, + DEFAULT_DIALOG_WIDTH, dialogProperties, themeService, layoutService, @@ -60,12 +65,17 @@ export class LinkCalloutDialog extends CalloutDialog 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 { + this._selectionComplete = new Deferred(); this.show(); return this._selectionComplete.promise; } @@ -97,6 +107,7 @@ export class LinkCalloutDialog extends CalloutDialog 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 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: `${strings.escape(this._linkTextInputBox.value)}`, - }); - 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; + } } diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component.ts index 778ca22420..0a691d8649 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/markdownToolbar.component.ts +++ b/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; private _wysiwygTaskbarContent: Array; private _previewModeTaskbarContent: Array; + 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 { + 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, `${calloutResult?.insertUnescapedLinkLabel}`); + 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 { + 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; + } } diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html index 2993db0106..a92834fb93 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.html @@ -5,7 +5,7 @@ *--------------------------------------------------------------------------------------------*/ -->
- +
diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts index 79221695cc..8c402db774 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/textCell.component.ts +++ b/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'); diff --git a/src/sql/workbench/contrib/notebook/browser/markdownToolbarActions.ts b/src/sql/workbench/contrib/notebook/browser/markdownToolbarActions.ts index a1dd9dcdb7..68b1ea788e 100644 --- a/src/sql/workbench/contrib/notebook/browser/markdownToolbarActions.ts +++ b/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 { + public async transformText(type: MarkdownButtonType): Promise { 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 { - 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 { + 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(); } diff --git a/src/sql/workbench/contrib/notebook/test/browser/markdownTextTransformer.test.ts b/src/sql/workbench/contrib/notebook/test/browser/markdownTextTransformer.test.ts index 5c51b682ab..8f3c310cb9 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/markdownTextTransformer.test.ts +++ b/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 { + 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 { 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 { + 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 { let value = 'Multi Words'; textModel.setValue(value); diff --git a/src/sql/workbench/contrib/notebook/test/calloutDialog/linkCalloutDialog.test.ts b/src/sql/workbench/contrib/notebook/test/calloutDialog/linkCalloutDialog.test.ts new file mode 100644 index 0000000000..3cdf72661f --- /dev/null +++ b/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 { + let linkCalloutDialog = new LinkCalloutDialog('Title', undefined, 'defaultLabel', + undefined, themeService, layoutService, telemetryService, contextKeyService, undefined, undefined, undefined); + linkCalloutDialog.render(); + + let deferred = new Deferred(); + // 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 { + 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(); + // 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 { + 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(); + // 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('<>&[]'), '<>&\[\]', 'Label test known escaped characters failed'); + assert.equal(escapeLabel('<>&[]()'), '<>&\[\]()', '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('<>&()'), '<>&%28%29', 'URL test known escaped characters failed'); + assert.equal(escapeUrl('<>&()[]'), '<>&%28%29[]', 'URL test all escaped characters failed'); + }); +});