Adding database properties general tab with read-only fields #23318
Conversation
ssreerama
requested review from
corivera,
alanrenmsft,
barbaravaldez and
chlafreniere
extensions/mssql/src/objectManagement/objectManagementService.ts
Outdated
Show resolved
Hide resolved
extensions/mssql/src/objectManagement/ui/databasePropertiesDialog.ts
Outdated
Show resolved
Hide resolved
…eanup, diabling the functionality.
|
@ssreerama there shouldn't be a horizonal scroll bar, could you please investigate why is it there? also, looking at the content in the dialog, the vertical bar also shouldn't be there giving how much empty space are there in the screenshot. |
It seems @barbaravaldez is already fixed both scroll bar issue in her PR, will take a look if both issues are not fixed in her PR. Thanks |
| @@ -106,8 +107,9 @@ async function handleObjectPropertiesDialogCommand(context: azdata.ObjectExplore | |||
| return; | |||
| } | |||
| try { | |||
| const parentUrn = await getParentUrn(context); | |||
| const parentUrn = context.nodeInfo ? await getParentUrn(context) : undefined; | |||
There was a problem hiding this comment.
What about updating getParentUrn to possibly return undefined instead? That makes sense if there isn't a parent already
There was a problem hiding this comment.
So if the ParentUrn is empty, then can we cconsider the objectUrn as a parent and update the value there?
There was a problem hiding this comment.
Let's do this as a follow-up - I already asked @barbaravaldez to do a separate PR for moving this into a helper function anyways and we can discuss it there.
extensions/mssql/src/objectManagement/ui/objectManagementDialogBase.ts
Outdated
Show resolved
Hide resolved
extensions/mssql/src/objectManagement/ui/objectManagementDialogBase.ts
Outdated
Show resolved
Hide resolved
|
Please, Ignore the latest commit for review, going to work on reusing the database dialog and will have a lot of refactoring. |
|
@ssreerama Maybe close this PR until you're ready for review? Or at least make it a draft. |
| @@ -106,8 +107,9 @@ async function handleObjectPropertiesDialogCommand(context: azdata.ObjectExplore | |||
| return; | |||
| } | |||
| try { | |||
| const parentUrn = await getParentUrn(context); | |||
| const parentUrn = context.nodeInfo ? await getParentUrn(context) : undefined; | |||
There was a problem hiding this comment.
Let's do this as a follow-up - I already asked @barbaravaldez to do a separate PR for moving this into a helper function anyways and we can discuss it there.
| @@ -42,3 +42,8 @@ export function isValidSQLPassword(password: string, userName: string = 'sa'): b | |||
| const hasNonAlphas = /\W/.test(password) ? 1 : 0; | |||
| return !containsUserName && password.length >= 8 && password.length <= 128 && (hasUpperCase + hasLowerCase + hasNumbers + hasNonAlphas >= 3); | |||
| } | |||
|
|
|||
| // Converts number to two decimal placed string | |||
There was a problem hiding this comment.
Use actual JS doc syntax for doc comments please
|
|
||
| // Converts number to two decimal placed string | ||
| export function convertNumToTwoDecimalStringinMB(value: number): string { | ||
| return localizedConstants.StringValueInMB(value?.toFixed(2)); |
There was a problem hiding this comment.
If value is actually possible to be undefined then the parameter should reflect that. And in that case it'd probably be better to just return an empty string instead
This PR contains:
Some of the below tasks would be addressed in this PR or separate PRs
Images/Videos:
NEW: