From 010fe919217c2122038bf7d013ac4486574c360d Mon Sep 17 00:00:00 2001 From: Aasim Khan Date: Wed, 31 Aug 2022 09:02:40 -0700 Subject: [PATCH] Adding image support to list view (#20449) --- .../src/ui/resourceTypePickerDialog.ts | 4 +- .../sql-migration/src/constants/strings.ts | 3 + .../assessmentResultsDialog.ts | 7 +- .../assessmentResults/sqlDatabasesTree.ts | 94 +++++-------------- src/sql/azdata.d.ts | 4 +- src/sql/azdata.proposed.d.ts | 11 +++ src/sql/base/browser/dom.ts | 20 ++++ src/sql/base/test/browser/dom.test.ts | 32 ++++++- .../modelComponents/listView.component.html | 2 +- .../modelComponents/listView.component.ts | 35 ++++++- .../modelComponents/media/listView.css | 21 ++++- 11 files changed, 152 insertions(+), 81 deletions(-) diff --git a/extensions/resource-deployment/src/ui/resourceTypePickerDialog.ts b/extensions/resource-deployment/src/ui/resourceTypePickerDialog.ts index a1db696c93..88f87fe860 100644 --- a/extensions/resource-deployment/src/ui/resourceTypePickerDialog.ts +++ b/extensions/resource-deployment/src/ui/resourceTypePickerDialog.ts @@ -138,8 +138,8 @@ export class ResourceTypePickerDialog extends DialogBase { text: loc.resourceTypeCategoryListViewTitle }, CSSStyles: { - 'width': '140px', - 'margin-top': '35px' + 'margin-top': '35px', + 'width': '140px' }, options: items, selectedOptionId: items[0].id, diff --git a/extensions/sql-migration/src/constants/strings.ts b/extensions/sql-migration/src/constants/strings.ts index ec945d99f4..fb37e26794 100644 --- a/extensions/sql-migration/src/constants/strings.ts +++ b/extensions/sql-migration/src/constants/strings.ts @@ -950,6 +950,9 @@ export const NO_ISSUES_FOUND_MI = localize('sql.migration.no.issues.mi', "No iss export const NO_ISSUES_FOUND_SQLDB = localize('sql.migration.no.issues.sqldb', "No issues found for migrating to Azure SQL Database."); export const NO_RESULTS_AVAILABLE = localize('sql.migration.no.results', 'Assessment results are unavailable.'); +export function BLOCKING_ISSUE_ARIA_LABEL(issue: string): string { + return localize('sql.migration.issue.aria.label', "Blocking Issue: {0}", issue); +} export function IMPACT_OBJECT_TYPE(objectType?: string): string { return objectType ? localize('sql.migration.impact.object.type', "Type: {0}", objectType) : ''; } diff --git a/extensions/sql-migration/src/dialog/assessmentResults/assessmentResultsDialog.ts b/extensions/sql-migration/src/dialog/assessmentResults/assessmentResultsDialog.ts index c4fa3be8c7..b84c7ea40c 100644 --- a/extensions/sql-migration/src/dialog/assessmentResults/assessmentResultsDialog.ts +++ b/extensions/sql-migration/src/dialog/assessmentResults/assessmentResultsDialog.ts @@ -45,9 +45,14 @@ export class AssessmentResultsDialog { return new Promise((resolve, reject) => { dialog.registerContent(async (view) => { try { + /** + * When using 100% height in the dialog, the container extends beyond the screen. + * This causes a vertical scrollbar to appear. To fix that, 33px needs to be + * subtracted from 100%. + */ const flex = view.modelBuilder.flexContainer().withLayout({ flexFlow: 'row', - height: '100%', + height: 'calc( 100% - 33px )', width: '100%' }).component(); flex.addItem(await this._tree.createRootContainer(dialog, view), { flex: '1 1 auto' }); diff --git a/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts b/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts index bb0045de59..a9d6f81cf2 100644 --- a/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts +++ b/extensions/sql-migration/src/dialog/assessmentResults/sqlDatabasesTree.ts @@ -50,7 +50,7 @@ export class SqlDatabaseTree { private _dialog!: azdata.window.Dialog; private _instanceTable!: azdata.DeclarativeTableComponent; private _databaseTable!: azdata.DeclarativeTableComponent; - private _assessmentResultsTable!: azdata.DeclarativeTableComponent; + private _assessmentResultsList!: azdata.ListViewComponent; private _impactedObjectsTable!: azdata.DeclarativeTableComponent; private _assessmentContainer!: azdata.FlexContainer; private _assessmentsTable!: azdata.FlexContainer; @@ -202,7 +202,7 @@ export class SqlDatabaseTree { this._disposables.push(this._databaseTable.onRowSelected(async (e) => { if (this._targetType === MigrationTargetType.SQLMI || this._targetType === MigrationTargetType.SQLDB) { - this._activeIssues = this._model._assessmentResults?.databaseAssessments[e.row].issues; + this._activeIssues = this._model._assessmentResults?.databaseAssessments[e.row].issues.filter(i => i.appliesToMigrationTargetPlatform === this._targetType); } else { this._activeIssues = []; } @@ -727,52 +727,19 @@ export class SqlDatabaseTree { } private createImpactedObjectsTable(): azdata.FlexContainer { - const headerStyle: azdata.CssStyles = { - 'border': 'none', - 'text-align': 'left' - }; - const rowStyle: azdata.CssStyles = { - 'border': 'none', - 'text-align': 'left', - 'white-space': 'nowrap', - 'text-overflow': 'ellipsis', - 'width': '200px', - 'overflow': 'hidden', - }; - this._assessmentResultsTable = this._view.modelBuilder.declarativeTable() - .withProps({ - enableRowSelection: true, - width: '200px', - CSSStyles: { 'table-layout': 'fixed' }, - columns: [ - { - displayName: '', - valueType: azdata.DeclarativeDataType.component, - width: '16px', - isReadOnly: true, - headerCssStyles: headerStyle, - rowCssStyles: rowStyle - }, - { - displayName: '', - valueType: azdata.DeclarativeDataType.string, - width: '184px', - isReadOnly: true, - headerCssStyles: headerStyle, - rowCssStyles: rowStyle - } - ] - } - ).component(); + this._assessmentResultsList = this._view.modelBuilder.listView().withProps({ + width: '200px', + options: [] + }).component(); - this._disposables.push(this._assessmentResultsTable.onRowSelected(async (e) => { - const selectedIssue = e.row > -1 ? this._activeIssues[e.row] : undefined; + this._disposables.push(this._assessmentResultsList.onDidClick(async (e: azdata.ListViewClickEvent) => { + const selectedIssue = this._activeIssues[parseInt(this._assessmentResultsList.selectedOptionId!)]; await this.refreshAssessmentDetails(selectedIssue); })); const container = this._view.modelBuilder.flexContainer() - .withItems([this._assessmentResultsTable]) + .withItems([this._assessmentResultsList]) .withLayout({ flexFlow: 'column', height: '100%' @@ -814,36 +781,27 @@ export class SqlDatabaseTree { this._recommendationTitle.value = constants.ASSESSMENT_RESULTS; this._recommendation.value = ''; } - - const assessmentResults: azdata.DeclarativeTableCellValue[][] = this._activeIssues + let assessmentResults: azdata.ListViewOption[] = this._activeIssues .sort((e1, e2) => { if (e1.databaseRestoreFails) { return -1; } if (e2.databaseRestoreFails) { return 1; } - return e1.checkId.localeCompare(e2.checkId); - }).map((v) => [ - { - value: this._view.modelBuilder - .image() - .withProps({ - iconPath: v.databaseRestoreFails - ? IconPathHelper.error - : undefined, - iconHeight: 16, - iconWidth: 16, - height: 16, - width: 16, - title: v.databaseRestoreFails - ? constants.ASSESSMENT_BLOCKING_ISSUE_TITLE - : '', - }) - .component() - }, - { value: v.checkId }]) - || []; + }).filter((v) => { + return v.appliesToMigrationTargetPlatform === this._targetType; + }).map((v, index) => { + return { + id: index.toString(), + label: v.checkId, + icon: v.databaseRestoreFails ? IconPathHelper.error : undefined, + ariaLabel: v.databaseRestoreFails ? constants.BLOCKING_ISSUE_ARIA_LABEL(v.checkId) : v.checkId, + }; + }); - await this._assessmentResultsTable.setDataValues(assessmentResults); - this._assessmentResultsTable.selectedRow = assessmentResults?.length > 0 ? 0 : -1; + this._assessmentResultsList.options = assessmentResults; + if (this._assessmentResultsList.options.length) { + this._assessmentResultsList.selectedOptionId = '0'; + + } } public async refreshAssessmentDetails(selectedIssue?: SqlMigrationAssessmentResultItem): Promise { @@ -939,7 +897,7 @@ export class SqlDatabaseTree { style: styleLeft }, { - value: db.issues?.length, + value: db.issues.filter(v => v.appliesToMigrationTargetPlatform === this._targetType)?.length, style: styleRight } ]); diff --git a/src/sql/azdata.d.ts b/src/sql/azdata.d.ts index 957ca9f1b6..18205c4769 100644 --- a/src/sql/azdata.d.ts +++ b/src/sql/azdata.d.ts @@ -3198,12 +3198,12 @@ declare module 'azdata' { */ flexWrap?: FlexWrapType | undefined; /** - * Container Height + * Container Height. Accepted values are px, %, auto and calc expressions. */ height?: number | string | undefined; /** - * Container Width + * Container Width. Accepted values are px, %, auto and calc expressions. */ width?: number | string | undefined; diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index e46538624a..6f36b3afe8 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -539,6 +539,17 @@ declare module 'azdata' { appendData(data: any[][]): Thenable; } + export interface ListViewOption { + /** + * The optional accessibility label for the column. Default is the label for the list view option. + */ + ariaLabel?: string; + /** + * Specify the icon for the option. The value could the path to the icon or and ADS icon defined in {@link SqlThemeIcon}. + */ + icon?: IconPath; + } + export interface IconColumnCellValue { /** * The icon to be displayed. diff --git a/src/sql/base/browser/dom.ts b/src/sql/base/browser/dom.ts index da8636807a..6b39b0e615 100644 --- a/src/sql/base/browser/dom.ts +++ b/src/sql/base/browser/dom.ts @@ -13,6 +13,21 @@ export function isHidden(element: HTMLElement): boolean { return element.style.display === 'none'; } +/** + * Checks if the CSS calc expression is valid or not. + * @param expression string to be tested. + * @returns true if the expression is a valid calc expression else false. + */ +export function validateCalcExpression(expression: string): boolean { + /** + * Regex that checks if a size string is a calc expression. Source: https://codepen.io/benfoster/pen/VPjLdQ + * If the size is a valid calc expression, we want to leave it as it is. + */ + const calcRegex = /calc\(( )?([\d\.]+(%|vh|vw|vmin|vmax|em|rem|px|cm|ex|in|mm|pc|pt|ch|q|deg|rad|grad|turn|s|ms|hz|khz))( )+[+\-\*\/]( )+(\-)?([\d\.]+(%|vh|vw|vmin|vmax|em|rem|px|cm|ex|in|mm|pc|pt|ch|q|deg|rad|grad|turn|s|ms|hz|khz))( )?\)/i; + + return calcRegex.test(expression); +} + /** * Converts a size value into its string representation. This will add px to the end unless * it already ends with %. If the size value is undefined it will return the defaultValue as-is. @@ -20,6 +35,11 @@ export function isHidden(element: HTMLElement): boolean { * @param defaultValue The default value to use if the size is undefined */ export function convertSize(size: number | string | undefined, defaultValue?: string): string { + + if (types.isString(size) && validateCalcExpression(size)) { + return size; + } + defaultValue = defaultValue || ''; if (types.isUndefinedOrNull(size)) { return defaultValue; diff --git a/src/sql/base/test/browser/dom.test.ts b/src/sql/base/test/browser/dom.test.ts index 890b34ac18..e0a1000a7d 100644 --- a/src/sql/base/test/browser/dom.test.ts +++ b/src/sql/base/test/browser/dom.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { convertSize, convertSizeToNumber } from 'sql/base/browser/dom'; +import { convertSize, convertSizeToNumber, validateCalcExpression } from 'sql/base/browser/dom'; suite('DOM Tests', () => { @@ -55,4 +55,34 @@ suite('DOM Tests', () => { const actual = convertSizeToNumber(undefined); assert.strictEqual(expected, actual); }); + + test('Validating different calc expressions', () => { + const calcExpressionsTestInputs = [ + { input: 'calc(10px+10px)', expected: false }, + { input: 'calc(76.8px--50%)', expected: false }, + { input: 'calc(10px +10px)', expected: false }, + { input: 'calc(10px- -50%)', expected: false }, + { input: 'calc(10vmin + 10px)', expected: true }, + { input: 'calc(10% - -50.7%)', expected: true }, + { input: 'calc(103px - -50%)', expected: true }, + { input: 'calc(10px +10px)', expected: false }, + { input: 'calc(10px --50%)', expected: false }, + { input: 'calc(10vmin + 10px )', expected: true }, + { input: 'calc( 10% - -50.7%)', expected: true }, + { input: 'calc( 103px - -50%)', expected: true }, + { input: 'calc( 10% - -50.7%)', expected: true }, + { input: 'calc( 10% --50.7% )', expected: false }, + { input: 'calc( 10.89% - -50.7% )', expected: true }, + { input: 'calc( 103px - -50%)', expected: true }, + { input: 'calc', expected: false }, + { input: 'calc(sdfs - sdf)', expected: false }, + { input: 'calc(15sdfs - 456svbdf)', expected: false }, + { input: 'calc( bpx45 - 45px)', expected: false }, + { input: 'calc( 34px - 45g)', expected: false } + ]; + + calcExpressionsTestInputs.forEach((run) => { + assert.strictEqual(run.expected, validateCalcExpression(run.input), `error validating calc expression: ${run.input}`); + }); + }); }); diff --git a/src/sql/workbench/browser/modelComponents/listView.component.html b/src/sql/workbench/browser/modelComponents/listView.component.html index 9d235d6261..1862550dae 100644 --- a/src/sql/workbench/browser/modelComponents/listView.component.html +++ b/src/sql/workbench/browser/modelComponents/listView.component.html @@ -1,4 +1,4 @@ -
+
{{title.text}}
diff --git a/src/sql/workbench/browser/modelComponents/listView.component.ts b/src/sql/workbench/browser/modelComponents/listView.component.ts index 4dd6fa4569..10c94ea4aa 100644 --- a/src/sql/workbench/browser/modelComponents/listView.component.ts +++ b/src/sql/workbench/browser/modelComponents/listView.component.ts @@ -2,6 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ + import { ChangeDetectorRef, Component, ElementRef, forwardRef, Inject, Input, OnDestroy, ViewChild } from '@angular/core'; import * as azdata from 'azdata'; import { ComponentBase } from 'sql/workbench/browser/modelComponents/componentBase'; @@ -18,6 +19,7 @@ import { IListRenderer, IListVirtualDelegate } from 'vs/base/browser/ui/list/lis import { attachListStyler } from 'vs/platform/theme/common/styler'; import { ScrollbarVisibility } from 'vs/base/common/scrollable'; import { ILogService } from 'vs/platform/log/common/log'; +import { createIconCssClass } from 'sql/workbench/browser/modelComponents/iconUtils'; @Component({ templateUrl: decodeURI(require.toUrl('./listView.component.html')) @@ -28,6 +30,7 @@ export default class ListViewComponent extends ComponentBase; + private _optionsListRenderer: OptionsListRenderer; private _selectedElementIdx!: number; static ROW_HEIGHT = 26; @@ -50,9 +53,11 @@ export default class ListViewComponent extends ComponentBase('ModelViewListView', this._vscodeList.nativeElement, - new OptionListDelegate(ListViewComponent.ROW_HEIGHT), [new OptionsListRenderer()], + new OptionListDelegate(ListViewComponent.ROW_HEIGHT), [this._optionsListRenderer], vscodelistOption); this._register(attachListStyler(this._optionsList, this.themeService)); @@ -109,6 +114,11 @@ export default class ListViewComponent extends ComponentBasethis.height) ?? (this.options.length * ListViewComponent.ROW_HEIGHT); this._optionsList.layout(height); + if (!this.options.find(o => o.icon !== undefined)) { + this._vscodeList.nativeElement.classList.add('hide-icon'); + } else { + this._vscodeList.nativeElement.classList.remove('hide-icon'); + } } // This is the entry point for the extension to set the selectedOptionId @@ -144,8 +154,8 @@ export default class ListViewComponent extends ComponentBase } interface ExtensionListTemplate { + parent: HTMLElement; root: HTMLElement; + labelContainer: HTMLElement; + iconContainer: HTMLElement; } class OptionsListRenderer implements IListRenderer { @@ -178,12 +191,26 @@ class OptionsListRenderer implements IListRenderer