Improve notebook link handling (#6087)

* Improve notebook link handling
- Single click now works for links inside Output areas
- Command links in untrusted notebooks have link color
- Refactored to use directive so code is in 1 place and can be easily
  added elsewhere if needed

* Removed unneeded service from constructor
This commit is contained in:
Kevin Cunnane
2019-06-20 11:40:12 -07:00
committed by GitHub
parent 578ac6cae5
commit b37b14eabd
7 changed files with 82 additions and 62 deletions

View File

@@ -0,0 +1,65 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the Source EULA. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Directive, Inject, HostListener, Input } from '@angular/core';
import { URI } from 'vs/base/common/uri';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { onUnexpectedError } from 'vs/base/common/errors';
import product from 'vs/platform/product/node/product';
const knownSchemes = new Set(['http', 'https', 'file', 'mailto', 'data', `${product.urlProtocol}`, 'azuredatastudio', 'azuredatastudio-insiders', 'vscode', 'vscode-insiders', 'vscode-resource']);
@Directive({
selector: '[link-handler]',
})
export class LinkHandlerDirective {
@Input() isTrusted: boolean;
constructor(@Inject(IOpenerService) private readonly openerService: IOpenerService) {
}
@HostListener('click', ['$event'])
onclick(event: MouseEvent): void {
// Note: this logic is taken from the VSCode handling of links in markdown
// Untrusted cells will not support commands or raw HTML tags
// Finally, we should consider supporting relative paths - created #5238 to track
let target: HTMLElement = event.target as HTMLElement;
if (target.tagName !== 'A') {
target = target.parentElement;
if (!target || target.tagName !== 'A') {
return;
}
}
try {
const href = target['href'];
if (href) {
this.handleLink(href);
}
} catch (err) {
onUnexpectedError(err);
} finally {
event.preventDefault();
}
}
private handleLink(content: string): void {
let uri: URI | undefined;
try {
uri = URI.parse(content);
} catch {
// ignore
}
if (uri && this.openerService && this.isSupportedLink(uri)) {
this.openerService.open(uri).catch(onUnexpectedError);
}
}
private isSupportedLink(link: URI): boolean {
if (knownSchemes.has(link.scheme)) {
return true;
}
return !!this.isTrusted && link.scheme === 'command';
}
}

View File

@@ -5,7 +5,7 @@
*--------------------------------------------------------------------------------------------*/ *--------------------------------------------------------------------------------------------*/
--> -->
<div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: column"> <div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: column">
<div #outputarea class="notebook-output" style="flex: 0 0 auto;"> <div #outputarea link-handler [isTrusted]="isTrusted" class="notebook-output" style="flex: 0 0 auto;">
<output-component *ngFor="let output of cellModel.outputs" [cellOutput]="output" [trustedMode] = "cellModel.trustedMode" [cellModel]="cellModel" [activeCellId]="activeCellId"> <output-component *ngFor="let output of cellModel.outputs" [cellOutput]="output" [trustedMode] = "cellModel.trustedMode" [cellModel]="cellModel" [activeCellId]="activeCellId">
</output-component> </output-component>
</div> </div>

View File

@@ -44,6 +44,10 @@ export class OutputAreaComponent extends AngularDisposable implements OnInit {
} }
} }
public get isTrusted(): boolean {
return this.cellModel.trustedMode;
}
private setFocusAndScroll(node: HTMLElement): void { private setFocusAndScroll(node: HTMLElement): void {
if (node) { if (node) {
node.focus(); node.focus();

View File

@@ -10,7 +10,7 @@
</code-component> </code-component>
</div> </div>
<div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: row"> <div style="overflow: hidden; width: 100%; height: 100%; display: flex; flex-flow: row">
<div #preview class ="notebook-preview" style="flex: 1 1 auto" (dblclick)="toggleEditMode()"> <div #preview link-handler [isTrusted]="isTrusted" class="notebook-preview" style="flex: 1 1 auto" (dblclick)="toggleEditMode()">
</div> </div>
<div #moreactions class="moreActions" style="flex: 0 0 auto; display: flex; flex-flow:column;width: 20px; min-height: 20px; max-height: 20px; padding-top: 0px; orientation: portrait"> <div #moreactions class="moreActions" style="flex: 0 0 auto; display: flex; flex-flow:column;width: 20px; min-height: 20px; max-height: 20px; padding-top: 0px; orientation: portrait">
</div> </div>

View File

@@ -6,7 +6,7 @@ import 'vs/css!./textCell';
import 'vs/css!./media/markdown'; import 'vs/css!./media/markdown';
import 'vs/css!./media/highlight'; import 'vs/css!./media/highlight';
import { OnInit, Component, Input, Inject, forwardRef, ElementRef, ChangeDetectorRef, OnDestroy, ViewChild, OnChanges, SimpleChange, HostListener, AfterContentInit } from '@angular/core'; import { OnInit, Component, Input, Inject, forwardRef, ElementRef, ChangeDetectorRef, ViewChild, OnChanges, SimpleChange, HostListener, AfterContentInit } from '@angular/core';
import * as path from 'path'; import * as path from 'path';
import { localize } from 'vs/nls'; import { localize } from 'vs/nls';
@@ -16,11 +16,7 @@ import { ICommandService } from 'vs/platform/commands/common/commands';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { Emitter } from 'vs/base/common/event'; import { Emitter } from 'vs/base/common/event';
import { URI } from 'vs/base/common/uri'; import { URI } from 'vs/base/common/uri';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import * as DOM from 'vs/base/browser/dom'; import * as DOM from 'vs/base/browser/dom';
import { onUnexpectedError } from 'vs/base/common/errors';
import { IMouseEvent } from 'vs/base/browser/mouseEvent';
import product from 'vs/platform/product/node/product';
import { CommonServiceInterface } from 'sql/platform/bootstrap/node/commonServiceInterface.service'; import { CommonServiceInterface } from 'sql/platform/bootstrap/node/commonServiceInterface.service';
import { CellView } from 'sql/workbench/parts/notebook/cellViews/interfaces'; import { CellView } from 'sql/workbench/parts/notebook/cellViews/interfaces';
@@ -31,13 +27,12 @@ import { CellToggleMoreActions } from 'sql/workbench/parts/notebook/cellToggleMo
export const TEXT_SELECTOR: string = 'text-cell-component'; export const TEXT_SELECTOR: string = 'text-cell-component';
const USER_SELECT_CLASS = 'actionselect'; const USER_SELECT_CLASS = 'actionselect';
const knownSchemes = new Set(['http', 'https', 'file', 'mailto', 'data', `${product.urlProtocol}`, 'azuredatastudio', 'azuredatastudio-insiders', 'vscode', 'vscode-insiders', 'vscode-resource']);
@Component({ @Component({
selector: TEXT_SELECTOR, selector: TEXT_SELECTOR,
templateUrl: decodeURI(require.toUrl('./textCell.component.html')) templateUrl: decodeURI(require.toUrl('./textCell.component.html'))
}) })
export class TextCellComponent extends CellView implements OnInit, AfterContentInit, OnChanges { export class TextCellComponent extends CellView implements OnInit, OnChanges {
@ViewChild('preview', { read: ElementRef }) private output: ElementRef; @ViewChild('preview', { read: ElementRef }) private output: ElementRef;
@ViewChild('moreactions', { read: ElementRef }) private moreActionsElementRef: ElementRef; @ViewChild('moreactions', { read: ElementRef }) private moreActionsElementRef: ElementRef;
@Input() cellModel: ICellModel; @Input() cellModel: ICellModel;
@@ -83,8 +78,7 @@ export class TextCellComponent extends CellView implements OnInit, AfterContentI
@Inject(forwardRef(() => ChangeDetectorRef)) private _changeRef: ChangeDetectorRef, @Inject(forwardRef(() => ChangeDetectorRef)) private _changeRef: ChangeDetectorRef,
@Inject(IInstantiationService) private _instantiationService: IInstantiationService, @Inject(IInstantiationService) private _instantiationService: IInstantiationService,
@Inject(IWorkbenchThemeService) private themeService: IWorkbenchThemeService, @Inject(IWorkbenchThemeService) private themeService: IWorkbenchThemeService,
@Inject(ICommandService) private _commandService: ICommandService, @Inject(ICommandService) private _commandService: ICommandService
@Inject(IOpenerService) private readonly openerService: IOpenerService,
) { ) {
super(); super();
this.isEditMode = true; this.isEditMode = true;
@@ -122,54 +116,6 @@ export class TextCellComponent extends CellView implements OnInit, AfterContentI
})); }));
} }
ngAfterContentInit(): void {
if (this.output) {
let element: HTMLElement = this.output.nativeElement;
this._register(DOM.addStandardDisposableListener(element, 'click', event => {
// Note: this logic is taken from the VSCode handling of links in markdown
// Untrusted cells will not support commands or raw HTML tags
// Finally, we should consider supporting relative paths - created #5238 to track
let target: HTMLElement | null = event.target;
if (target.tagName !== 'A') {
target = target.parentElement;
if (!target || target.tagName !== 'A') {
return;
}
}
try {
const href = target['href'];
if (href) {
this.handleLink(href, event);
}
} catch (err) {
onUnexpectedError(err);
} finally {
event.preventDefault();
}
}));
}
}
private handleLink(content: string, event?: IMouseEvent): void {
let uri: URI | undefined;
try {
uri = URI.parse(content);
} catch {
// ignore
}
if (uri && this.openerService && this.isSupportedLink(uri)) {
this.openerService.open(uri).catch(onUnexpectedError);
}
}
private isSupportedLink(link: URI): boolean {
if (knownSchemes.has(link.scheme)) {
return true;
}
return !!this.model.trustedMode && link.scheme === 'command';
}
ngOnChanges(changes: { [propKey: string]: SimpleChange }) { ngOnChanges(changes: { [propKey: string]: SimpleChange }) {
for (let propName in changes) { for (let propName in changes) {
if (propName === 'activeCellId') { if (propName === 'activeCellId') {
@@ -186,6 +132,10 @@ export class TextCellComponent extends CellView implements OnInit, AfterContentI
} }
} }
public get isTrusted(): boolean {
return this.model.trustedMode;
}
/** /**
* Updates the preview of markdown component with latest changes * Updates the preview of markdown component with latest changes
* If content is empty and in non-edit mode, default it to 'Double-click to edit' * If content is empty and in non-edit mode, default it to 'Double-click to edit'

View File

@@ -13,7 +13,6 @@ import { IBootstrapParams, ISelector, providerIterator } from 'sql/platform/boot
import { CommonServiceInterface } from 'sql/platform/bootstrap/node/commonServiceInterface.service'; import { CommonServiceInterface } from 'sql/platform/bootstrap/node/commonServiceInterface.service';
import { EditableDropDown } from 'sql/platform/electron-browser/editableDropdown/editableDropdown.component'; import { EditableDropDown } from 'sql/platform/electron-browser/editableDropdown/editableDropdown.component';
import { NotebookComponent } from 'sql/workbench/parts/notebook/notebook.component'; import { NotebookComponent } from 'sql/workbench/parts/notebook/notebook.component';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { CodeComponent } from 'sql/workbench/parts/notebook/cellViews/code.component'; import { CodeComponent } from 'sql/workbench/parts/notebook/cellViews/code.component';
import { CodeCellComponent } from 'sql/workbench/parts/notebook/cellViews/codeCell.component'; import { CodeCellComponent } from 'sql/workbench/parts/notebook/cellViews/codeCell.component';
@@ -26,6 +25,7 @@ import LoadingSpinner from 'sql/workbench/electron-browser/modelComponents/loadi
import { Checkbox } from 'sql/base/electron-browser/ui/checkbox/checkbox.component'; import { Checkbox } from 'sql/base/electron-browser/ui/checkbox/checkbox.component';
import { SelectBox } from 'sql/platform/ui/electron-browser/selectBox/selectBox.component'; import { SelectBox } from 'sql/platform/ui/electron-browser/selectBox/selectBox.component';
import { InputBox } from 'sql/base/electron-browser/ui/inputBox/inputBox.component'; import { InputBox } from 'sql/base/electron-browser/ui/inputBox/inputBox.component';
import { LinkHandlerDirective } from 'sql/workbench/parts/notebook/cellViews/linkHandler.directive';
export const NotebookModule = (params, selector: string, instantiationService: IInstantiationService): any => { export const NotebookModule = (params, selector: string, instantiationService: IInstantiationService): any => {
@NgModule({ @NgModule({
@@ -43,7 +43,8 @@ export const NotebookModule = (params, selector: string, instantiationService: I
ComponentHostDirective, ComponentHostDirective,
OutputAreaComponent, OutputAreaComponent,
OutputComponent, OutputComponent,
StdInComponent StdInComponent,
LinkHandlerDirective
], ],
entryComponents: [NotebookComponent], entryComponents: [NotebookComponent],
imports: [ imports: [

View File

@@ -178,7 +178,7 @@ export function registerNotebookThemes(overrideEditorThemeSetting: boolean): IDi
const linkForeground = theme.getColor(textLinkForeground); const linkForeground = theme.getColor(textLinkForeground);
if (linkForeground) { if (linkForeground) {
collector.addRule(` collector.addRule(`
.notebookEditor a:link { .notebookEditor a {
color: ${linkForeground}; color: ${linkForeground};
} }
`); `);