Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add required indicators to fields #25992

Merged
merged 6 commits into from
Nov 20, 2024
Merged

Add required indicators to fields #25992

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe98d58
Add CodeQL Analysis workflow (#10195)
jhutchings1 Jun 17, 2020
438ea52
Merge remote-tracking branch 'upstream/master'
Oct 23, 2024
b49ffc5
Add required indicators for fields
Oct 23, 2024
80c5df0
Revert code ql changes
Oct 23, 2024
aecf376
Update migration list table column width
Nov 19, 2024
175ca70
Add TODO
Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 66 additions & 17 deletions extensions/sql-migration/src/wizard/databaseBackupPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,6 @@ export class DatabaseBackupPage extends MigrationWizardPage {
const headerCssStyles: azdata.CssStyles = {
...styles.LABEL_CSS,
'border': 'none',
'text-align': 'left',
'box-shadow': 'inset 0px -1px 0px #F3F2F1',
};
const rowCssStyle: azdata.CssStyles = {
Expand All @@ -384,23 +383,23 @@ export class DatabaseBackupPage extends MigrationWizardPage {
.withProps({
columns: [
{
displayName: constants.SOURCE_DATABASE,
valueType: azdata.DeclarativeDataType.string,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: '250px'
},
{
displayName: constants.TARGET_DATABASE_NAME,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: WIZARD_TABLE_COLUMN_WIDTH_SMALL
},
{
displayName: constants.NETWORK_SHARE_PATH,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
Expand All @@ -414,47 +413,47 @@ export class DatabaseBackupPage extends MigrationWizardPage {
.withProps({
columns: [
{
displayName: constants.SOURCE_DATABASE,
valueType: azdata.DeclarativeDataType.string,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: WIZARD_TABLE_COLUMN_WIDTH_SMALL,
},
{
displayName: constants.TARGET_DATABASE_NAME,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: WIZARD_TABLE_COLUMN_WIDTH_SMALL
},
{
displayName: constants.RESOURCE_GROUP,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: WIZARD_TABLE_COLUMN_WIDTH_SMALL
},
{
displayName: constants.STORAGE_ACCOUNT,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: WIZARD_TABLE_COLUMN_WIDTH_SMALL
},
{
displayName: constants.BLOB_CONTAINER,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
isReadOnly: true,
width: WIZARD_TABLE_COLUMN_WIDTH_SMALL
},
{
displayName: constants.BLOB_CONTAINER_LAST_BACKUP_FILE,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
Expand All @@ -463,7 +462,7 @@ export class DatabaseBackupPage extends MigrationWizardPage {
hidden: true
},
{
displayName: constants.BLOB_CONTAINER_FOLDER,
displayName: '',
valueType: azdata.DeclarativeDataType.component,
rowCssStyles: rowCssStyle,
headerCssStyles: headerCssStyles,
Expand Down Expand Up @@ -1299,24 +1298,74 @@ export class DatabaseBackupPage extends MigrationWizardPage {
});
this.migrationStateModel._sourceDatabaseNames = this.migrationStateModel._databasesForMigration;

const createTableHeaderComponents = (headerNames: { name: string; requiredIndicator: boolean }[]) =>
headerNames.map(header =>
this._view.modelBuilder.divContainer().withItems([
this._view.modelBuilder.divContainer().withProps({
CSSStyles: { marginTop: '-1em' }
}).component(),
this._view.modelBuilder.text().withProps({
value: header.name,
requiredIndicator: header.requiredIndicator,
CSSStyles: {
...styles.LABEL_CSS,
margin: '0'
}
}).component(),
this._view.modelBuilder.divContainer().withProps({
CSSStyles: { marginBottom: '-1em' }
}).component()
]).component()
);

let networkShareTargetTableHeaderNames = [
{ name: constants.SOURCE_DATABASE, requiredIndicator: false },
{ name: constants.TARGET_DATABASE_NAME, requiredIndicator: true },
{ name: constants.NETWORK_SHARE_PATH, requiredIndicator: true },
]

let networkShareTargetTableHeaderComponents = createTableHeaderComponents(
networkShareTargetTableHeaderNames
);

const networkShareTargetData = this.migrationStateModel._databasesForMigration
.map((db, index) => [
{ value: db },
{ value: this._view.modelBuilder.text().withProps({ value: db, CSSStyles: { 'margin': '0' } }).component() },
{ value: this._networkShareTargetDatabaseNames[index] },
{ value: this._networkShareLocations[index] }]);
await this._networkShareTargetDatabaseNamesTable.setDataValues(networkShareTargetData);
await this._networkShareTargetDatabaseNamesTable.setDataValues([
networkShareTargetTableHeaderComponents.map(component => ({ value: component })),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will lead to another accessibility issue since now table headers won't be announced as table headers.

Copy link
Contributor Author

@akshika898 akshika898 Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this way as we do not have an option to give indicators in headers. Adding * as text was not a very good way and it would be shown in black which will be different from other indicators in extension leading to inconsistency in UI.
I understand that the header is now defined as row but even in previous version the tab did not go to headers, neither the narrator defined specifically for headers, nor I found error in accessibility insights. Do we have any other way to test it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, merging this as a temporary fix.

...networkShareTargetData,
]);

let blobContainerTargetTableHeaderNames = [
{ name: constants.SOURCE_DATABASE, requiredIndicator: false },
{ name: constants.TARGET_DATABASE_NAME, requiredIndicator: true },
{ name: constants.RESOURCE_GROUP, requiredIndicator: true },
{ name: constants.STORAGE_ACCOUNT, requiredIndicator: true },
{ name: constants.BLOB_CONTAINER, requiredIndicator: true },
{ name: constants.BLOB_CONTAINER_LAST_BACKUP_FILE, requiredIndicator: true },
{ name: constants.BLOB_CONTAINER_FOLDER, requiredIndicator: true }
]

let blobContainerTargetTableHeaderComponents = createTableHeaderComponents(
blobContainerTargetTableHeaderNames
);

const blobContainerTargetData = this.migrationStateModel._databasesForMigration
.map((db, index) => [
{ value: db },
{ value: this._view.modelBuilder.text().withProps({ value: db, CSSStyles: { 'margin': '0' } }).component() },
{ value: this._blobContainerTargetDatabaseNames[index] },
{ value: this._blobContainerResourceGroupDropdowns[index] },
{ value: this._blobContainerStorageAccountDropdowns[index] },
{ value: this._blobContainerDropdowns[index] },
{ value: this._blobContainerLastBackupFileDropdowns[index] },
{ value: this._blobContainerFolderDropdowns[index] }]);
await this._blobContainerTargetDatabaseNamesTable.setDataValues([]);
await this._blobContainerTargetDatabaseNamesTable.setDataValues(blobContainerTargetData);
await this._blobContainerTargetDatabaseNamesTable.setDataValues([
blobContainerTargetTableHeaderComponents.map(component => ({ value: component })),
...blobContainerTargetData,
]);
await this.getSubscriptionValues();
// clear change tracking flags
this.migrationStateModel.refreshDatabaseBackupPage = false;
Expand Down
5 changes: 3 additions & 2 deletions extensions/sql-migration/src/wizard/integrationRuntimePage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,8 @@ export class IntergrationRuntimePage extends MigrationWizardPage {
const subscriptionLabel = this._view.modelBuilder.text()
.withProps({
value: constants.SUBSCRIPTION,
CSSStyles: { ...styles.LABEL_CSS }
requiredIndicator: true,
CSSStyles: { ...styles.LABEL_CSS },
}).component();

this._subscriptionDropdown = this._view.modelBuilder.dropDown()
Expand All @@ -320,7 +321,7 @@ export class IntergrationRuntimePage extends MigrationWizardPage {
required: true,
fireOnTextChange: true,
placeholder: constants.SELECT_A_SERVICE,
CSSStyles: { 'margin': '0' }
CSSStyles: { 'margin': '-1em 0 0 0' }
}).component();

this._disposables.push(
Expand Down