Agent: UI/UX finishes and clean up (#1768)

* changed columnw widths

* fixed scrolling for prev run job history list

* fixed all resizing and scrolling issues in agent

* removed commented code

* code review comments

src/sql/parts/jobManagement/common/jobManagementService.ts

@@ -7,11 +7,8 @@
 
 import { localize } from 'vs/nls';
 import * as sqlops from 'sqlops';
-import * as vscode from 'vscode';
 import { TPromise } from 'vs/base/common/winjs.base';
-import { Injectable } from '@angular/core';
 import { IJobManagementService } from 'sql/parts/jobManagement/common/interfaces';
-import { ICapabilitiesService } from 'sql/services/capabilities/capabilitiesService';
 import { IConnectionManagementService } from 'sql/parts/connection/common/connectionManagement';
 
 
@@ -22,8 +19,7 @@ export class JobManagementService implements IJobManagementService {
 	private _jobCacheObject : {[server: string]: JobCacheObject; } = {};
 
 	constructor(
-		@IConnectionManagementService private _connectionService: IConnectionManagementService,
+		@IConnectionManagementService private _connectionService: IConnectionManagementService
-		@ICapabilitiesService private _capabilitiesService: ICapabilitiesService
 	) {
 	}
 
@@ -79,7 +75,8 @@ export class JobManagementService implements IJobManagementService {
 export class JobCacheObject {
 	_serviceBrand: any;
 	private _jobs: sqlops.AgentJobInfo[] = [];
-	private _jobHistories: { [jobId: string]: sqlops.AgentJobHistoryInfo[]; } = {};
+	private _jobHistories: { [jobID: string]: sqlops.AgentJobHistoryInfo[]; } = {};
+	private _runCharts: { [jobID: string]: string[]; } = {};
 	private _prevJobID: string;
 	private _serverName: string;
 	private _dataView: Slick.Data.DataView<any>;
@@ -89,7 +86,7 @@ export class JobCacheObject {
 			return this._jobs;
 		}
 
-		public get jobHistories(): { [jobId: string]: sqlops.AgentJobHistoryInfo[] } {
+		public get jobHistories(): { [jobID: string]: sqlops.AgentJobHistoryInfo[] } {
 			return this._jobHistories;
 		}
 
@@ -109,12 +106,16 @@ export class JobCacheObject {
 			return this._dataView;
 		}
 
+		public getRunChart(jobID: string): string[] {
+			return this._runCharts[jobID];
+		}
+
 		/* Setters */
 		public set jobs(value: sqlops.AgentJobInfo[]) {
 			this._jobs = value;
 		}
 
-		public set jobHistories(value: { [jobId: string]: sqlops.AgentJobHistoryInfo[]; }) {
+		public set jobHistories(value: { [jobID: string]: sqlops.AgentJobHistoryInfo[]; }) {
 			this._jobHistories = value;
 		}
 
@@ -126,6 +127,10 @@ export class JobCacheObject {
 			this._jobHistories[jobID] = value;
 		}
 
+		public setRunChart(jobID: string, value: string[]) {
+			this._runCharts[jobID] = value;
+		}
+
 		public set serverName(value: string) {
 			this._serverName = value;
 		}

src/sql/parts/jobManagement/common/media/jobs.css

@@ -97,7 +97,7 @@ jobhistory-component {
 
 #jobsDiv .jobview-grid .slick-cell.l1.r1 .jobview-jobnametext {
     text-overflow: ellipsis;
-    width: 200px;
+    width: 250px;
     overflow: hidden;
     white-space: nowrap;
     display: inline-block;
@@ -114,10 +114,6 @@ jobhistory-component {
     color: orangered;
 }
 
-#jobsDiv .jobview-grid > .monaco-table .slick-viewport > .grid-canvas > .ui-widget-content.slick-row .slick-cell.error-row {
-    opacity: 0;
-}
-
 #jobsDiv .jobview-grid > .monaco-table .slick-viewport > .grid-canvas > .ui-widget-content.slick-row .slick-cell._detail_selector.error-row {
     opacity: 1;
 }
@@ -219,8 +215,8 @@ agentview-component .jobview-grid .grid-canvas > .ui-widget-content.slick-row.od
     background: #444444 !important;
 }
 
-table.jobprevruns div.bar1, table.jobprevruns div.bar2, table.jobprevruns div.bar3,
+table.jobprevruns div.bar0, table.jobprevruns div.bar1, table.jobprevruns div.bar2,
-table.jobprevruns div.bar4, table.jobprevruns div.bar5 {
+table.jobprevruns div.bar3, table.jobprevruns div.bar4, table.jobprevruns div.bar5 {
     padding-top: 3px;
     padding-left: 5px;
 	width: 10px;
@@ -232,7 +228,6 @@ table.jobprevruns div.bar4, table.jobprevruns div.bar5 {
 }
 
 table.jobprevruns {
-    margin: auto;
     height: 100%;
 }
 
@@ -264,5 +259,4 @@ table.jobprevruns > tbody {
 
 .jobs-view-toolbar span{
     padding-left: 5px;
-}
+}
-

src/sql/parts/jobManagement/views/jobHistory.component.html

@@ -77,7 +77,7 @@
 <!-- Job History details -->
 <div class='history-details'>
 	<!-- Previous run list -->
-	<div style="min-width: 275px">
+	<div class="prev-run-list-container" style="min-width: 275px; height: 75vh">
 		<table *ngIf="_showPreviousRuns === true">
 			<tr>
 				<td class="date-column">

src/sql/parts/jobManagement/views/jobHistory.component.ts

@@ -60,6 +60,9 @@ export class JobHistoryComponent extends Disposable implements OnInit {
 	private _noJobsAvailable: boolean = false;
 	private _serverName: string;
 
+	private static readonly INITIAL_TREE_HEIGHT: number = 780;
+	private static readonly HEADING_HEIGHT: number = 24;
+
 	constructor(
 		@Inject(forwardRef(() => ElementRef)) el: ElementRef,
 		@Inject(forwardRef(() => ChangeDetectorRef)) private _cd: ChangeDetectorRef,
@@ -125,8 +128,17 @@ export class JobHistoryComponent extends Disposable implements OnInit {
 			renderer: this._treeRenderer
 		}, {verticalScrollMode: ScrollbarVisibility.Visible});
 		this._register(attachListStyler(this._tree, this.themeService));
-		this._tree.layout(1024);
+		this._tree.layout(JobHistoryComponent.INITIAL_TREE_HEIGHT);
 		this._initActionBar();
+		$(window).resize(() => {
+			let historyDetails = $('.overview-container').get(0);
+			let statusBar = $('.part.statusbar').get(0);
+			if (historyDetails && statusBar) {
+				let historyBottom = historyDetails.getBoundingClientRect().bottom;
+				let statusTop = statusBar.getBoundingClientRect().top;
+				this._tree.layout(statusTop - historyBottom - JobHistoryComponent.HEADING_HEIGHT);
+			}
+		});
 	}
 
 	ngAfterContentChecked() {

src/sql/parts/jobManagement/views/jobHistory.css

@@ -230,10 +230,6 @@ table.step-list tr.step-row td {
 	min-height: 40px !important;
 }
 
-jobhistory-component .history-details .step-table.prev-run-list .monaco-scrollable-element {
-	overflow-y: scroll !important;
-}
-
 jobhistory-component .jobhistory-heading-container {
 	display: -webkit-box;
 }

src/sql/parts/jobManagement/views/jobsView.component.ts

@@ -44,16 +44,28 @@ export class JobsViewComponent implements AfterContentChecked {
 	private _disposables = new Array<vscode.Disposable>();
 
 	private columns: Array<Slick.Column<any>> = [
-		{ name: nls.localize('jobColumns.name', 'Name'), field: 'name', formatter: (row, cell, value, columnDef, dataContext) => this.renderName(row, cell, value, columnDef, dataContext), width: 200, id: 'name' },
+		{
-		{ name: nls.localize('jobColumns.lastRun', 'Last Run'), field: 'lastRun', width: 120, id: 'lastRun' },
+			name: nls.localize('jobColumns.name', 'Name'),
-		{ name: nls.localize('jobColumns.nextRun', 'Next Run'), field: 'nextRun', width: 120, id: 'nextRun' },
+			field: 'name',
-		{ name: nls.localize('jobColumns.enabled', 'Enabled'), field: 'enabled', width: 50, id: 'enabled' },
+			formatter: (row, cell, value, columnDef, dataContext) => this.renderName(row, cell, value, columnDef, dataContext),
-		{ name: nls.localize('jobColumns.status', 'Status'), field: 'currentExecutionStatus', width: 60, id: 'currentExecutionStatus' },
+			width: 150,
-		{ name: nls.localize('jobColumns.category', 'Category'), field: 'category', width: 120, id: 'category' },
+			id: 'name'
+		},
+		{ name: nls.localize('jobColumns.lastRun', 'Last Run'), field: 'lastRun', width: 80, id: 'lastRun' },
+		{ name: nls.localize('jobColumns.nextRun', 'Next Run'), field: 'nextRun', width: 80, id: 'nextRun' },
+		{ name: nls.localize('jobColumns.enabled', 'Enabled'), field: 'enabled', width: 60, id: 'enabled' },
+		{ name: nls.localize('jobColumns.status', 'Status'), field: 'currentExecutionStatus', width: 50, id: 'currentExecutionStatus' },
+		{ name: nls.localize('jobColumns.category', 'Category'), field: 'category', width: 100, id: 'category' },
 		{ name: nls.localize('jobColumns.runnable', 'Runnable'), field: 'runnable', width: 70, id: 'runnable' },
 		{ name: nls.localize('jobColumns.schedule', 'Schedule'), field: 'hasSchedule', width: 60, id: 'hasSchedule' },
-		{ name: nls.localize('jobColumns.lastRunOutcome', 'Last Run Outcome'), field: 'lastRunOutcome', width: 120, id: 'lastRunOutcome' },
+		{ name: nls.localize('jobColumns.lastRunOutcome', 'Last Run Outcome'), field: 'lastRunOutcome', width: 100, id: 'lastRunOutcome' },
-		{ name: nls.localize('jobColumns.previousRuns', 'Previous Runs'), formatter: this.renderChartsPostHistory, field: 'previousRuns', width: 80, id: 'previousRuns' }
+		{
+			name: nls.localize('jobColumns.previousRuns', 'Previous Runs'),
+			formatter: (row, cell, value, columnDef, dataContext) => this.renderChartsPostHistory(row, cell, value, columnDef, dataContext),
+			field: 'previousRuns',
+			width: 100,
+			id: 'previousRuns'
+		}
 	];
 
 
@@ -79,7 +91,6 @@ export class JobsViewComponent implements AfterContentChecked {
 	private _serverName: string;
 	private _isCloud: boolean;
 	private _showProgressWheel: boolean;
-	private _tabHeight: number;
 	private filterStylingMap: { [columnName: string]: [any]; } = {};
 	private filterStack = ['start'];
 	private filterValueMap: { [columnName: string]: string[]; } = {};
@@ -337,19 +348,14 @@ export class JobsViewComponent implements AfterContentChecked {
 		this._showProgressWheel = false;
 		this._cd.detectChanges();
 		const self = this;
-		this._tabHeight = $('agentview-component #jobsDiv .jobview-grid').get(0).clientHeight;
 		$(window).resize(() => {
-			let currentTab = $('agentview-component #jobsDiv .jobview-grid').get(0);
+			let jobsViewToolbar = $('jobsview-component .jobs-view-toolbar').get(0);
-			if (currentTab) {
+			let statusBar = $('.part.statusbar').get(0);
-				let currentTabHeight = currentTab.clientHeight;
+			if (jobsViewToolbar && statusBar) {
-				if (currentTabHeight < self._tabHeight) {
+				let toolbarBottom = jobsViewToolbar.getBoundingClientRect().bottom;
-					$('agentview-component #jobsDiv div.ui-widget').css('height', `${currentTabHeight - 22}px`);
+				let statusTop = statusBar.getBoundingClientRect().top;
-					self._table.resizeCanvas();
+				$('agentview-component #jobsDiv .jobview-grid').css('height', statusTop - toolbarBottom);
-				} else {
+				self._table.resizeCanvas();
-					$('agentview-component #jobsDiv div.ui-widget').css('height', `${currentTabHeight}px`);
-					self._table.resizeCanvas();
-				}
-				self._tabHeight = currentTabHeight;
 			}
 		});
 		this._table.grid.onColumnsResized.subscribe((e, data: any) => {
@@ -361,49 +367,17 @@ export class JobsViewComponent implements AfterContentChecked {
 
 			// generate job charts again
 			self.jobs.forEach(job => {
-				let jobId = job.jobId;
 				let jobHistories = self._jobCacheObject.getJobHistory(job.jobId);
 				let previousRuns = jobHistories.slice(jobHistories.length - 5, jobHistories.length);
 				self.createJobChart(job.jobId, previousRuns);
 			});
 		});
-		$('#jobsDiv .jobview-grid .monaco-table .slick-viewport .grid-canvas .ui-widget-content.slick-row').hover((e) => {
+		$('#jobsDiv .jobview-grid .monaco-table .slick-viewport .grid-canvas .ui-widget-content.slick-row').hover((e1) =>
-			// highlight the error row as well if a failing job row is hovered
+			this.highlightErrorRows(e1), (e2) => this.hightlightNonErrorRows(e2));
-			if (e.currentTarget.children.item(0).classList.contains('job-with-error')) {
+
-				let target = $(e.currentTarget);
+		this._table.grid.onScroll.subscribe((e) => {
-				let targetChildren = $(e.currentTarget.children);
+			$('#jobsDiv .jobview-grid .monaco-table .slick-viewport .grid-canvas .ui-widget-content.slick-row').hover((e1) =>
-				let siblings = target.nextAll().toArray();
+				this.highlightErrorRows(e1), (e2) => this.hightlightNonErrorRows(e2));
-				let top = parseInt(target.css('top'), 10);
-				for (let i = 0; i < siblings.length; i++) {
-					let sibling = siblings[i];
-					let siblingTop = parseInt($(sibling).css('top'), 10);
-					if (siblingTop === top + ROW_HEIGHT) {
-						$(sibling.children).addClass('hovered');
-						sibling.onmouseenter = (e) => {
-							targetChildren.addClass('hovered');
-						};
-						sibling.onmouseleave = (e) => {
-							targetChildren.removeClass('hovered');
-						}
-						break;
-					}
-				}
-			}
-		}, (e) => {
-			// switch back to original background
-			if (e.currentTarget.children.item(0).classList.contains('job-with-error')) {
-				let target = $(e.currentTarget);
-				let siblings = target.nextAll().toArray();
-				let top = parseInt(target.css('top'), 10);
-				for (let i = 0; i < siblings.length; i++) {
-					let sibling = siblings[i];
-					let siblingTop = parseInt($(sibling).css('top'), 10);
-					if (siblingTop === top + ROW_HEIGHT) {
-						$(sibling.children).removeClass('hovered');
-						break;
-					}
-				}
-			}
 		});
 		// cache the dataview for future use
 		this._jobCacheObject.dataView = this.dataView;
@@ -411,6 +385,47 @@ export class JobsViewComponent implements AfterContentChecked {
 		this.loadJobHistories();
 	}
 
+	private highlightErrorRows(e) {
+		// highlight the error row as well if a failing job row is hovered
+		if (e.currentTarget.children.item(0).classList.contains('job-with-error')) {
+			let target = $(e.currentTarget);
+			let targetChildren = $(e.currentTarget.children);
+			let siblings = target.nextAll().toArray();
+			let top = parseInt(target.css('top'), 10);
+			for (let i = 0; i < siblings.length; i++) {
+				let sibling = siblings[i];
+				let siblingTop = parseInt($(sibling).css('top'), 10);
+				if (siblingTop === top + ROW_HEIGHT) {
+					$(sibling.children).addClass('hovered');
+					sibling.onmouseenter = (e) => {
+						targetChildren.addClass('hovered');
+					};
+					sibling.onmouseleave = (e) => {
+						targetChildren.removeClass('hovered');
+					}
+					break;
+				}
+			}
+		}
+	}
+
+	private hightlightNonErrorRows(e) {
+		// switch back to original background
+		if (e.currentTarget.children.item(0).classList.contains('job-with-error')) {
+			let target = $(e.currentTarget);
+			let siblings = target.nextAll().toArray();
+			let top = parseInt(target.css('top'), 10);
+			for (let i = 0; i < siblings.length; i++) {
+				let sibling = siblings[i];
+				let siblingTop = parseInt($(sibling).css('top'), 10);
+				if (siblingTop === top + ROW_HEIGHT) {
+					$(sibling.children).removeClass('hovered');
+					break;
+				}
+			}
+		}
+	}
+
 	private setRowWithErrorClass(hash: { [index: number]: { [id: string]: string; } }, row: number, errorClass: string) {
 		hash[row] = {
 			'_detail_selector': errorClass,
@@ -481,14 +496,28 @@ export class JobsViewComponent implements AfterContentChecked {
 	}
 
 	private renderChartsPostHistory(row, cell, value, columnDef, dataContext) {
-		return `<table class="jobprevruns" id="${dataContext.id}">
+		let runChart = this._jobCacheObject.getRunChart(dataContext.id);
+		if (runChart && runChart.length > 0) {
+			return `<table class="jobprevruns" id="${dataContext.id}">
 				<tr>
-					<td><div class="bar1"></div></td>
+					<td>${runChart[0] ? runChart[0] : '<div></div>' }</td>
-					<td><div class="bar2"></div></td>
+					<td>${runChart[1] ? runChart[1] : '<div></div>' }</td>
-					<td><div class="bar3"></div></td>
+					<td>${runChart[2] ? runChart[2] : '<div></div>' }</td>
-					<td><div class="bar4"></div></td>
+					<td>${runChart[3] ? runChart[3] : '<div></div>' }</td>
+					<td>${runChart[4] ? runChart[4] : '<div></div>' }</td>
 				</tr>
-				</table>`;
+			</table>`;
+		} else {
+			return `<table class="jobprevruns" id="${dataContext.id}">
+			<tr>
+				<td><div class="bar0"></div></td>
+				<td><div class="bar1"></div></td>
+				<td><div class="bar2"></div></td>
+				<td><div class="bar3"></div></td>
+				<td><div class="bar4"></div></td>
+			</tr>
+			</table>`;
+		}
 	}
 
 	private expandJobRowDetails(rowIdx: number, message?: string): void {
@@ -579,8 +608,9 @@ export class JobsViewComponent implements AfterContentChecked {
 
 	private createJobChart(jobId: string, jobHistories: sqlops.AgentJobHistoryInfo[]): void {
 		let chartHeights = this.getChartHeights(jobHistories);
+		let runCharts = [];
 		for (let i = 0; i < jobHistories.length; i++) {
-			let runGraph = $(`table#${jobId}.jobprevruns > tbody > tr > td > div.bar${i + 1}`);
+			let runGraph = $(`table#${jobId}.jobprevruns > tbody > tr > td > div.bar${i}`);
 			if (jobHistories && jobHistories.length > 0) {
 				runGraph.css('height', chartHeights[i]);
 				let bgColor = jobHistories[i].runStatus === 0 ? 'red' : 'green';
@@ -589,6 +619,9 @@ export class JobsViewComponent implements AfterContentChecked {
 					let currentTarget = e.currentTarget;
 					currentTarget.title = jobHistories[i].runDuration;
 				});
+				if (runGraph.get(0)) {
+					runCharts.push(runGraph.get(0).outerHTML);
+				}
 			} else {
 				runGraph.css('height', '5px');
 				runGraph.css('background', 'red');
@@ -598,6 +631,9 @@ export class JobsViewComponent implements AfterContentChecked {
 				});
 			}
 		}
+		if (runCharts.length > 0) {
+			this._jobCacheObject.setRunChart(jobId, runCharts);
+		}
 	}
 
 	// chart height normalization logic

src/sqltest/parts/jobManagement/jobManagementService.test.ts

@@ -16,6 +16,6 @@ suite('Job Management service tests', () => {
 
 	test('Construction - Job Service Initialization', () => {
 		// ... Create instance of the service and reder account picker
-		let service = new JobManagementService(undefined, undefined);
+		let service = new JobManagementService(undefined);
 	});
 });