diff --git a/src/sql/workbench/contrib/charts/browser/chartView.ts b/src/sql/workbench/contrib/charts/browser/chartView.ts index c5d812b375..173c6a8760 100644 --- a/src/sql/workbench/contrib/charts/browser/chartView.ts +++ b/src/sql/workbench/contrib/charts/browser/chartView.ts @@ -27,6 +27,7 @@ import { Checkbox } from 'sql/base/browser/ui/checkbox/checkbox'; import { ChartState, IInsightOptions, ChartType } from 'sql/workbench/contrib/charts/common/interfaces'; import * as nls from 'vs/nls'; import { find } from 'vs/base/common/arrays'; +import { INotificationService } from 'vs/platform/notification/common/notification'; const insightRegistry = Registry.as(Extensions.InsightContribution); @@ -83,6 +84,7 @@ export class ChartView extends Disposable implements IPanelView { @IContextViewService private _contextViewService: IContextViewService, @IThemeService private _themeService: IThemeService, @IInstantiationService private _instantiationService: IInstantiationService, + @INotificationService private readonly _notificationService: INotificationService ) { super(); this.taskbarContainer = DOM.$('div.taskbar-container'); @@ -204,11 +206,16 @@ export class ChartView extends Disposable implements IPanelView { if (batch) { let summary = batch.resultSetSummaries[this._currentData.resultId]; if (summary) { - this._queryRunner.getQueryRows(0, summary.rowCount, 0, 0).then(d => { - this._data = { - columns: summary.columnInfo.map(c => c.columnName), - rows: d.resultSubset.rows.map(r => r.map(c => c.displayValue)) - }; + this._queryRunner.getQueryRows(0, summary.rowCount, this._currentData.batchId, this._currentData.resultId).then(d => { + if (!d.resultSubset.rows) { // be defensive against this + this._data = { columns: [], rows: [] }; + this._notificationService.error(nls.localize('charting.failedToGetRows', "Failed to get rows for the dataset to chart.")); + } else { + this._data = { + columns: summary.columnInfo.map(c => c.columnName), + rows: d.resultSubset.rows.map(r => r.map(c => c.displayValue)) + }; + } if (this.insight) { this.insight.data = this._data; } @@ -273,15 +280,17 @@ export class ChartView extends Disposable implements IPanelView { } private createOption(option: IChartOption, container: HTMLElement) { - let label = DOM.$('div'); + const label = DOM.$('div.option-label'); label.innerText = option.label; - let optionContainer = DOM.$('div.option-container'); + const optionContainer = DOM.$('div.option-container'); + const optionInput = DOM.$('div.option-input'); optionContainer.appendChild(label); + optionContainer.appendChild(optionInput); let setFunc: (val) => void; let value = this.state ? this.state.options[option.configEntry] || option.default : option.default; switch (option.type) { case ControlType.checkbox: - let checkbox = new Checkbox(optionContainer, { + let checkbox = new Checkbox(optionInput, { label: '', ariaLabel: option.label, checked: value, @@ -302,7 +311,7 @@ export class ChartView extends Disposable implements IPanelView { //pass options into changeAltNames in order for SelectBox to show user-friendly names. let dropdown = new SelectBox(option.displayableOptions || this.changeToAltNames(option.options), undefined, this._contextViewService); dropdown.select(option.options.indexOf(value)); - dropdown.render(optionContainer); + dropdown.render(optionInput); dropdown.onDidSelect(e => { if (this.options[option.configEntry] !== option.options[e.index]) { this.options[option.configEntry] = option.options[e.index]; @@ -319,7 +328,7 @@ export class ChartView extends Disposable implements IPanelView { this.optionDisposables.push(attachSelectBoxStyler(dropdown, this._themeService)); break; case ControlType.input: - let input = new InputBox(optionContainer, this._contextViewService); + let input = new InputBox(optionInput, this._contextViewService); input.value = value || ''; input.onDidChange(e => { if (this.options[option.configEntry] !== e) { @@ -337,7 +346,7 @@ export class ChartView extends Disposable implements IPanelView { this.optionDisposables.push(attachInputBoxStyler(input, this._themeService)); break; case ControlType.numberInput: - let numberInput = new InputBox(optionContainer, this._contextViewService, { type: 'number' }); + let numberInput = new InputBox(optionInput, this._contextViewService, { type: 'number' }); numberInput.value = value || ''; numberInput.onDidChange(e => { if (this.options[option.configEntry] !== Number(e)) { @@ -355,7 +364,7 @@ export class ChartView extends Disposable implements IPanelView { this.optionDisposables.push(attachInputBoxStyler(numberInput, this._themeService)); break; case ControlType.dateInput: - let dateInput = new InputBox(optionContainer, this._contextViewService, { type: 'datetime-local' }); + let dateInput = new InputBox(optionInput, this._contextViewService, { type: 'datetime-local' }); dateInput.value = value || ''; dateInput.onDidChange(e => { if (this.options[option.configEntry] !== e) { diff --git a/src/sql/workbench/contrib/charts/test/browser/chartView.test.ts b/src/sql/workbench/contrib/charts/test/browser/chartView.test.ts index daefcbe1cb..400a29272f 100644 --- a/src/sql/workbench/contrib/charts/test/browser/chartView.test.ts +++ b/src/sql/workbench/contrib/charts/test/browser/chartView.test.ts @@ -10,6 +10,7 @@ import { TestLayoutService } from 'vs/workbench/test/workbenchTestServices'; import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { IThemeService } from 'vs/platform/theme/common/themeService'; +import { SimpleNotificationService } from 'vs/editor/standalone/browser/simpleServices'; suite('Chart View', () => { test('initializes without error', () => { @@ -28,6 +29,7 @@ function createChartView(): ChartView { const contextViewService = new ContextViewService(layoutService); const themeService = new TestThemeService(); const instantiationService = new TestInstantiationService(); + const notificationService = new SimpleNotificationService(); instantiationService.stub(IThemeService, themeService); - return new ChartView(contextViewService, themeService, instantiationService); + return new ChartView(contextViewService, themeService, instantiationService, notificationService); }