From 604c1e786156cae938f79c805e44c0a44181b22d Mon Sep 17 00:00:00 2001
From: Aasim Khan <aasimkhan30@gmail.com>
Date: Fri, 9 Jun 2023 15:28:53 -0700
Subject: [PATCH] Adding aria-labels to checkbox cells in declarative table
 (#23204)

---
 .../wizard/pages/modifyColumnsPage.test.ts    | 37 ++++++++--
 .../src/wizard/pages/modifyColumnsPage.ts     | 43 +++++++----
 .../declarativeTable.component.html           | 71 ++++++++++---------
 3 files changed, 100 insertions(+), 51 deletions(-)

diff --git a/extensions/import/src/test/wizard/pages/modifyColumnsPage.test.ts b/extensions/import/src/test/wizard/pages/modifyColumnsPage.test.ts
index e7cf3471a5..fbc015c000 100644
--- a/extensions/import/src/test/wizard/pages/modifyColumnsPage.test.ts
+++ b/extensions/import/src/test/wizard/pages/modifyColumnsPage.test.ts
@@ -56,9 +56,7 @@ describe('import extension modify Column Page', function () {
 		should.notEqual(modifyColumnsPage.form, undefined, 'form should not be undefined');
 	});
 
-	it('handleImport updates table value correctly when import is successful', async function() {
-
-
+	it('handleImport updates table value correctly when import is successful', async function () {
 		let testProseColumns = [
 			{
 				columnName: 'column1',
@@ -75,8 +73,35 @@ describe('import extension modify Column Page', function () {
 		];
 
 		let testTableData = [
-			[ 'column1', 'nvarchar(50)', false, false],
-			[ 'column2', 'nvarchar(50)', false, false]
+			[
+				{
+					value: 'column1'
+				}, {
+					value: 'nvarchar(50)'
+				}, {
+					value: false,
+					ariaLabel: constants.primaryKeyText,
+					enabled: true
+				}, {
+					value: false,
+					ariaLabel: constants.allowNullsText,
+					enabled: true
+				}
+			], [
+				{
+					value: 'column2'
+				}, {
+					value: 'nvarchar(50)'
+				}, {
+					value: false,
+					ariaLabel: constants.primaryKeyText,
+					enabled: true
+				}, {
+					value: false,
+					ariaLabel: constants.allowNullsText,
+					enabled: true
+				}
+			]
 		];
 
 		mockImportModel.object.proseColumns = testProseColumns;
@@ -98,7 +123,7 @@ describe('import extension modify Column Page', function () {
 		await modifyColumnsPage.onPageEnter();
 
 		// checking if all the required components are correctly initialized
-		should.deepEqual(modifyColumnsPage.table.data, testTableData);
+		should.deepEqual(modifyColumnsPage.table.dataValues, testTableData);
 
 	});
 });
diff --git a/extensions/import/src/wizard/pages/modifyColumnsPage.ts b/extensions/import/src/wizard/pages/modifyColumnsPage.ts
index c6a49f3dd8..1ba1b01571 100644
--- a/extensions/import/src/wizard/pages/modifyColumnsPage.ts
+++ b/extensions/import/src/wizard/pages/modifyColumnsPage.ts
@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as azdata from 'azdata';
-import { ColumnMetadata, ColumnMetadataArray } from '../api/models';
+import { ColumnMetadata } from '../api/models';
 import { ImportPage } from '../api/importPage';
 import * as constants from '../../common/constants';
 
@@ -83,8 +83,25 @@ export class ModifyColumnsPage extends ImportPage {
 		this._form = form;
 	}
 
-	private static convertMetadata(column: ColumnMetadata): any[] {
-		return [column.columnName, column.dataType, false, column.nullable];
+	private static convertMetadata(column: ColumnMetadata): azdata.DeclarativeTableCellValue[] {
+		return [
+			{
+				value: column.columnName,
+			},
+			{
+				value: column.dataType,
+			},
+			{
+				value: false,
+				ariaLabel: constants.primaryKeyText,
+				enabled: true
+			},
+			{
+				value: column.nullable,
+				ariaLabel: constants.allowNullsText,
+				enabled: true
+			}
+		];
 	}
 
 	async start(): Promise<boolean> {
@@ -94,12 +111,12 @@ export class ModifyColumnsPage extends ImportPage {
 
 		this.table.onDataChanged((e) => {
 			this.model.proseColumns = [];
-			this.table.data.forEach((row) => {
+			this.table.dataValues.forEach((row) => {
 				this.model.proseColumns.push({
-					columnName: row[0].value,
-					dataType: row[1].value,
-					primaryKey: row[2].value,
-					nullable: row[3].value
+					columnName: <string>row[0].value,
+					dataType: <string>row[1].value,
+					primaryKey: <boolean>row[2].value,
+					nullable: <boolean>row[3].value
 				});
 			});
 		});
@@ -131,7 +148,7 @@ export class ModifyColumnsPage extends ImportPage {
 		this.instance.changeNextButtonLabel(constants.importDataText);
 		this.loading.loading = false;
 		this.instance.registerNavigationValidator((info) => {
-			return this.table.data && this.table.data.length > 0;
+			return this.table.dataValues && this.table.dataValues.length > 0;
 		});
 		return true;
 	}
@@ -147,7 +164,7 @@ export class ModifyColumnsPage extends ImportPage {
 
 	private emptyTable() {
 		this.table.updateProperties({
-			data: [],
+			dataValues: [],
 			columns: []
 		});
 	}
@@ -160,13 +177,13 @@ export class ModifyColumnsPage extends ImportPage {
 	}
 
 	private async populateTable() {
-		let data: ColumnMetadataArray[] = [];
+		let data: azdata.DeclarativeTableCellValue[][] = [];
 
 		this.model.proseColumns.forEach((column) => {
 			data.push(ModifyColumnsPage.convertMetadata(column));
 		});
 
-		this.table.updateProperties({
+		this.table.updateProperties(<azdata.DeclarativeTableProperties>{
 			columns: [{
 				displayName: constants.columnNameText,
 				valueType: azdata.DeclarativeDataType.string,
@@ -191,7 +208,7 @@ export class ModifyColumnsPage extends ImportPage {
 				width: '100px',
 				showCheckAll: true
 			}],
-			data: data
+			dataValues: data
 		});
 	}
 }
diff --git a/src/sql/workbench/browser/modelComponents/declarativeTable.component.html b/src/sql/workbench/browser/modelComponents/declarativeTable.component.html
index a8538059d8..92c4921163 100644
--- a/src/sql/workbench/browser/modelComponents/declarativeTable.component.html
+++ b/src/sql/workbench/browser/modelComponents/declarativeTable.component.html
@@ -4,9 +4,8 @@
 		<tr role="row">
 			<ng-container *ngFor="let column of columns; let c = index;">
 				<th class="declarative-table-header" aria-sort="none" [style.width]="getColumnWidth(column)"
-					[ngClass]="{'declarative-table-cell-checkbox' : isCheckBox(c)}"
-					[ngStyle]="column.headerCssStyles" [attr.aria-label]="getHeaderAriaLabel(c)"
-					*ngIf="showColumn(column)">
+					[ngClass]="{'declarative-table-cell-checkbox' : isCheckBox(c)}" [ngStyle]="column.headerCssStyles"
+					[attr.aria-label]="getHeaderAriaLabel(c)" *ngIf="showColumn(column)">
 					{{column.displayName}}
 					<checkbox *ngIf="headerCheckboxVisible(c)" [checked]="isHeaderChecked(c)"
 						[aria-label]="getCheckAllColumnAriaLabel(c)" (onChange)="onHeaderCheckBoxChanged($event,c)"
@@ -18,36 +17,44 @@
 	<tbody role="rowgroup">
 		<ng-container *ngIf="data.length > 0">
 			<ng-container *ngFor="let row of data;let r = index;">
-				<tr [style.display]="isFiltered(r) ? 'none' : ''" class="declarative-table-row" [ngStyle]="getRowStyle(r)" role="row" [attr.tabindex]="enableRowSelection ? 0 : null" (click)="onRowSelected(r)" (keydown)="onKey($event,r)">
+				<tr [style.display]="isFiltered(r) ? 'none' : ''" class="declarative-table-row"
+					[ngStyle]="getRowStyle(r)" role="row" [attr.tabindex]="enableRowSelection ? 0 : null"
+					(click)="onRowSelected(r)" (keydown)="onKey($event,r)">
 					<ng-container *ngFor="let cellData of row;let c = index;trackBy:trackByFnCols">
-						<td class="declarative-table-cell" [style.width]="getColumnWidth(c)"
-							[attr.aria-label]="getAriaLabel(r, c)"
-							[ngClass]="{'declarative-table-cell-checkbox' : isCheckBox(c)}"
-							[ngStyle]="mergeCss(columns[c].rowCssStyles, cellData.style)" role="gridcell"
-							*ngIf="showColumn(columns[c])">
-							<checkbox *ngIf="isCheckBox(c)" label="" (onChange)="onCheckBoxChanged($event,r,c)"
-								[enabled]="isControlEnabled(r, c)" [checked]="isChecked(r,c)"
-								[ngStyle]="mergeCss(columns[c].rowCssStyles, cellData.style)">
-							</checkbox>
-							<select-box *ngIf="isSelectBox(c)" [options]="getOptions(c)"
-								(onDidSelect)="onSelectBoxChanged($event,r,c)"
-								[selectedOption]="getSelectedOptionDisplayName(r,c)">
-							</select-box>
-							<editable-select-box *ngIf="isEditableSelectBox(c)" [options]="getOptions(c)"
-								(onDidSelect)="onSelectBoxChanged($event,r,c)"
-								[selectedOption]="getSelectedOptionDisplayName(r,c)">
-							</editable-select-box>
-							<input-box *ngIf="isInputBox(c)" [value]="cellData.value"
-								(onDidChange)="onInputBoxChanged($event,r,c)"></input-box>
-							<span *ngIf="isLabel(c)">
-								{{cellData.value}}
-							</span>
-							<model-component-wrapper *ngIf="isComponent(c) && getItemDescriptor(cellData.value)"
-								[descriptor]="getItemDescriptor(cellData.value)" [modelStore]="modelStore">
-							</model-component-wrapper>
-							<button *ngIf="isContextMenuColumn(c)" [title]="contextMenuButtonTitle" [attr.aria-label]="contextMenuButtonTitle" class="codicon toggle-more" (click)="onContextMenuButtonClick($event,r,c)" (keydown)="onContextMenuButtonKeyDown($event,r,c)">
-							</button>
-						</td>
+						<ng-container *ngIf="c<columns?.length">
+							<td class="declarative-table-cell" [style.width]="getColumnWidth(c)"
+								[attr.aria-label]="getAriaLabel(r, c)"
+								[ngClass]="{'declarative-table-cell-checkbox' : isCheckBox(c)}"
+								[ngStyle]="mergeCss(columns[c].rowCssStyles, cellData.style)" role="gridcell"
+								*ngIf="showColumn(columns[c])">
+								<checkbox *ngIf="isCheckBox(c)" label="" [aria-label]="getAriaLabel(r,c)"
+									(onChange)="onCheckBoxChanged($event,r,c)" [enabled]="isControlEnabled(r, c)"
+									[checked]="isChecked(r,c)"
+									[ngStyle]="mergeCss(columns[c].rowCssStyles, cellData.style)">
+								</checkbox>
+								<select-box *ngIf="isSelectBox(c)" [options]="getOptions(c)"
+									(onDidSelect)="onSelectBoxChanged($event,r,c)"
+									[selectedOption]="getSelectedOptionDisplayName(r,c)">
+								</select-box>
+								<editable-select-box *ngIf="isEditableSelectBox(c)" [options]="getOptions(c)"
+									(onDidSelect)="onSelectBoxChanged($event,r,c)"
+									[selectedOption]="getSelectedOptionDisplayName(r,c)">
+								</editable-select-box>
+								<input-box *ngIf="isInputBox(c)" [value]="cellData.value"
+									(onDidChange)="onInputBoxChanged($event,r,c)"></input-box>
+								<span *ngIf="isLabel(c)">
+									{{cellData.value}}
+								</span>
+								<model-component-wrapper *ngIf="isComponent(c) && getItemDescriptor(cellData.value)"
+									[descriptor]="getItemDescriptor(cellData.value)" [modelStore]="modelStore">
+								</model-component-wrapper>
+								<button *ngIf="isContextMenuColumn(c)" [title]="contextMenuButtonTitle"
+									[attr.aria-label]="contextMenuButtonTitle" class="codicon toggle-more"
+									(click)="onContextMenuButtonClick($event,r,c)"
+									(keydown)="onContextMenuButtonKeyDown($event,r,c)">
+								</button>
+							</td>
+						</ng-container>
 					</ng-container>
 				</tr>
 			</ng-container>