Fix Grid Links (#2393)

* fix grid links

* formatting

* remove commented code

* revert formatting functions

* fix build break

src/sql/base/browser/ui/table/table.ts

@@ -18,7 +18,7 @@ import { isArray, isBoolean } from 'vs/base/common/types';
 import { Event, Emitter } from 'vs/base/common/event';
 import { range } from 'vs/base/common/arrays';
 
-export interface ITableContextMenuEvent {
+export interface ITableMouseEvent {
 	anchor: HTMLElement | { x: number, y: number };
 	cell?: { row: number, cell: number };
 }
@@ -62,8 +62,11 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 
 	private _disposables: IDisposable[] = [];
 
-	private _onContextMenu = new Emitter<ITableContextMenuEvent>();
+	private _onContextMenu = new Emitter<ITableMouseEvent>();
-	public readonly onContextMenu: Event<ITableContextMenuEvent> = this._onContextMenu.event;
+	public readonly onContextMenu: Event<ITableMouseEvent> = this._onContextMenu.event;
+
+	private _onClick = new Emitter<ITableMouseEvent>();
+	public readonly onClick: Event<ITableMouseEvent> = this._onClick.event;
 
 	constructor(parent: HTMLElement, configuration?: ITableConfiguration<T>, options?: Slick.GridOptions<T>) {
 		super();
@@ -114,11 +117,16 @@ export class Table<T extends Slick.SlickData> extends Widget implements IThemabl
 			});
 		}
 
-		this._grid.onContextMenu.subscribe((e: JQuery.Event) => {
+		this.mapMouseEvent(this._grid.onContextMenu, this._onContextMenu);
+		this.mapMouseEvent(this._grid.onClick, this._onClick);
+	}
+
+	private mapMouseEvent(slickEvent: Slick.Event<any>, emitter: Emitter<ITableMouseEvent>) {
+		slickEvent.subscribe((e: JQuery.Event) => {
 			const originalEvent = e.originalEvent;
 			const cell = this._grid.getCellFromEvent(originalEvent);
 			const anchor = originalEvent instanceof MouseEvent ? { x: originalEvent.x, y: originalEvent.y } : originalEvent.srcElement as HTMLElement;
-			this._onContextMenu.fire({ anchor, cell });
+			emitter.fire({ anchor, cell });
 		});
 	}
 

src/sql/parts/grid/services/sharedServices.ts

@@ -3,6 +3,7 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { $ } from 'vs/base/browser/dom';
 import { escape } from 'sql/base/common/strings';
 
 export class DBCellValue {
@@ -14,6 +15,7 @@ export class DBCellValue {
 	}
 }
 
+
 /**
  * Format xml field into a hyperlink and performs HTML entity encoding
  */
@@ -54,3 +56,54 @@ export function textFormatter(row: number, cell: any, value: any, columnDef: any
 
 	return `<span title="${valueToDisplay}" class="${cellClasses}">${valueToDisplay}</span>`;
 }
+
+/** The following code is a rewrite over the both formatter function using dom builder
+ * rather than string manipulation, which is a safer and easier method of achieving the same goal.
+ * However, when electron is in "Run as node" mode, dom creation acts differently than normal and therefore
+ * the tests to test for html escaping fail. I'm keeping this code around as we should migrate to it if we ever
+ * integrate into actual DOM testing (electron running in normal mode) later on.
+
+export const hyperLinkFormatter: Slick.Formatter<any> = (row, cell, value, columnDef, dataContext): string => {
+	let classes: Array<string> = ['grid-cell-value-container'];
+	let displayValue = '';
+
+	if (DBCellValue.isDBCellValue(value)) {
+		if (!value.isNull) {
+			displayValue = value.displayValue;
+			classes.push('queryLink');
+			let linkContainer = $('a', {
+				class: classes.join(' '),
+				title: displayValue
+			});
+			linkContainer.innerText = displayValue;
+			return linkContainer.outerHTML;
+		} else {
+			classes.push('missing-value');
+		}
+	}
+
+	let cellContainer = $('span', { class: classes.join(' '), title: displayValue });
+	cellContainer.innerText = displayValue;
+	return cellContainer.outerHTML;
+};
+
+export const textFormatter: Slick.Formatter<any> = (row, cell, value, columnDef, dataContext): string => {
+	let displayValue = '';
+	let classes: Array<string> = ['grid-cell-value-container'];
+
+	if (DBCellValue.isDBCellValue(value)) {
+		if (!value.isNull) {
+			displayValue = value.displayValue.replace(/(\r\n|\n|\r)/g, ' ');
+		} else {
+			classes.push('missing-value');
+			displayValue = 'NULL';
+		}
+	}
+
+	let cellContainer = $('span', { class: classes.join(' '), title: displayValue });
+	cellContainer.innerText = displayValue;
+
+	return cellContainer.outerHTML;
+};
+
+*/

src/sql/parts/query/editor/gridPanel.ts

@@ -7,7 +7,7 @@
 import { attachTableStyler } from 'sql/common/theme/styler';
 import QueryRunner from 'sql/parts/query/execution/queryRunner';
 import { VirtualizedCollection, AsyncDataProvider } from 'sql/base/browser/ui/table/asyncDataView';
-import { Table, ITableStyles, ITableContextMenuEvent } from 'sql/base/browser/ui/table/table';
+import { Table, ITableStyles, ITableMouseEvent } from 'sql/base/browser/ui/table/table';
 import { ScrollableSplitView } from 'sql/base/browser/ui/scrollableSplitview/scrollableSplitview';
 import { MouseWheelSupport } from 'sql/base/browser/ui/table/plugins/mousewheelTableScroll.plugin';
 import { AutoColumnSize } from 'sql/base/browser/ui/table/plugins/autoSizeColumns.plugin';
@@ -15,6 +15,8 @@ import { SaveFormat } from 'sql/parts/grid/common/interfaces';
 import { IGridActionContext, SaveResultAction, CopyResultAction, SelectAllGridAction, MaximizeTableAction, MinimizeTableAction, ChartDataAction } from 'sql/parts/query/editor/actions';
 import { CellSelectionModel } from 'sql/base/browser/ui/table/plugins/cellSelectionModel.plugin';
 import { RowNumberColumn } from 'sql/base/browser/ui/table/plugins/rowNumberColumn.plugin';
+import { escape } from 'sql/base/common/strings';
+import { hyperLinkFormatter, textFormatter } from 'sql/parts/grid/services/sharedServices';
 
 import * as sqlops from 'sqlops';
 
@@ -34,6 +36,8 @@ import { TPromise } from 'vs/base/common/winjs.base';
 import { Separator, ActionBar, ActionsOrientation } from 'vs/base/browser/ui/actionbar/actionbar';
 import { Dimension, getContentWidth } from 'vs/base/browser/dom';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
+import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
+import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
 
 const rowHeight = 29;
 const columnHeight = 26;
@@ -145,7 +149,7 @@ export class GridPanel extends ViewletPanel {
 
 		for (let set of resultsToAdd) {
 			let tableState = new GridTableState();
-			let table = new GridTable(this.runner, tableState, set, this.contextMenuService, this.instantiationService);
+			let table = this.instantiationService.createInstance(GridTable, this.runner, tableState, set);
 			tableState.onMaximizedChange(e => {
 				if (e) {
 					this.maximizeTable(table.id);
@@ -229,8 +233,10 @@ class GridTable<T> extends Disposable implements IView {
 		private runner: QueryRunner,
 		public state: GridTableState,
 		private resultSet: sqlops.ResultSetSummary,
-		private contextMenuService: IContextMenuService,
+		@IContextMenuService private contextMenuService: IContextMenuService,
-		private instantiationService: IInstantiationService
+		@IInstantiationService private instantiationService: IInstantiationService,
+		@IEditorService private editorService: IEditorService,
+		@IUntitledEditorService private untitledEditorService: IUntitledEditorService
 	) {
 		super();
 		this.container.style.width = '100%';
@@ -239,11 +245,15 @@ class GridTable<T> extends Disposable implements IView {
 		this.container.className = 'grid-panel';
 
 		this.columns = this.resultSet.columnInfo.map((c, i) => {
+			let isLinked = c.isXml || c.isJson;
+
 			return <Slick.Column<T>>{
 				id: i.toString(),
-				name: c.columnName,
+				name: c.columnName === 'Microsoft SQL Server 2005 XML Showplan'
+					? 'XML Showplan'
+					: escape(c.columnName),
 				field: i.toString(),
-				width: 100
+				formatter: isLinked ? hyperLinkFormatter : textFormatter
 			};
 		});
 	}
@@ -267,12 +277,16 @@ class GridTable<T> extends Disposable implements IView {
 		});
 		let numberColumn = new RowNumberColumn({ numberOfRows: this.resultSet.rowCount });
 		this.columns.unshift(numberColumn.getColumnDefinition());
-		this.table = this._register(new Table(tableContainer, { dataProvider: new AsyncDataProvider(collection), columns: this.columns }, { rowHeight, showRowNumber: true }));
+		this.table = this._register(new Table(tableContainer, {
+			dataProvider: new AsyncDataProvider(collection),
+			columns: this.columns
+		}, { rowHeight, showRowNumber: true }));
 		this.table.setSelectionModel(this.selectionModel);
 		this.table.registerPlugin(new MouseWheelSupport());
 		this.table.registerPlugin(new AutoColumnSize());
 		this.table.registerPlugin(numberColumn);
 		this._register(this.table.onContextMenu(this.contextMenu, this));
+		this._register(this.table.onClick(this.onTableClick, this));
 
 		if (this.styles) {
 			this.table.style(this.styles);
@@ -313,6 +327,19 @@ class GridTable<T> extends Disposable implements IView {
 		this.actionBar.push(actions, { icon: true, label: false });
 	}
 
+	private onTableClick(event: ITableMouseEvent) {
+		// account for not having the number column
+		let column = this.resultSet.columnInfo[event.cell.cell - 1];
+		// handle if a showplan link was clicked
+		if (column && (column.isXml || column.isJson)) {
+			this.runner.getQueryRows(event.cell.row, 1, this.resultSet.batchId, this.resultSet.id).then(d => {
+				let value = d.resultSubset.rows[0][event.cell.cell - 1];
+				let input = this.untitledEditorService.createOrGet(undefined, column.isXml ? 'xml' : 'json', value.displayValue);
+				this.editorService.openEditor(input);
+			});
+		}
+	}
+
 	public layout(size: number): void {
 		if (!this.table) {
 			this.build();
@@ -341,14 +368,18 @@ class GridTable<T> extends Disposable implements IView {
 				let dataWithSchema = {};
 				// skip the first column since its a number column
 				for (let i = 1; i < this.columns.length; i++) {
-					dataWithSchema[this.columns[i].field] = r[i - 1].displayValue;
+					dataWithSchema[this.columns[i].field] = {
+						displayValue: r[i - 1].displayValue,
+						ariaLabel: escape(r[i - 1].displayValue),
+						isNull: r[i - 1].isNull
+					};
 				}
 				return dataWithSchema as T;
 			});
 		});
 	}
 
-	private contextMenu(e: ITableContextMenuEvent): void {
+	private contextMenu(e: ITableMouseEvent): void {
 		const selection = this.selectionModel.getSelectedRanges();
 		const { cell } = e;
 		this.contextMenuService.showContextMenu({
@@ -390,7 +421,7 @@ class GridTable<T> extends Disposable implements IView {
 	}
 
 	private placeholdGenerator(index: number): any {
-		return { values: [] };
+		return {};
 	}
 
 	private renderGridDataRowsRange(startIndex: number, count: number): void {

src/sqltest/parts/grid/services/sharedServices.test.ts

@@ -17,6 +17,7 @@ suite('Grid shared services tests', () => {
 		cellValue.displayValue = testText;
 		cellValue.isNull = false;
 		let formattedHtml = SharedServices.textFormatter(undefined, undefined, cellValue, undefined, undefined);
+		let hyperlink = SharedServices.hyperLinkFormatter(undefined, undefined, cellValue, undefined, undefined);
 
 		// Then the result is HTML for a span element containing the cell value's display value as plain text
 		verifyFormattedHtml(formattedHtml, testText);