Validating and Saving the changes before closing the dialog #23743
Conversation
ssreerama
requested review from
alanrenmsft,
Charles-Gagnon,
corivera,
kisantia,
barbaravaldez and
chlafreniere
and removed request for
Charles-Gagnon
| @@ -46,6 +47,7 @@ export class ServerPropertiesDialog extends ObjectManagementDialogBase<Server, S | |||
| constructor(objectManagementService: IObjectManagementService, options: ObjectManagementDialogOptions) { | |||
| super(objectManagementService, options); | |||
| this.dialogObject.customButtons[1].enabled = false; | |||
| this.dialogObject.okButton.label = uiLoc.ApplyText; | |||
There was a problem hiding this comment.
I'm not sure if changing the ok button label, should be done only for properties. Maybe we can add it to the other dialogs as well?
There was a problem hiding this comment.
For now I have added it to the Server/Database props and as 'Create' for create database. And yes, it would be nice to have this across the ADS.
@Charles-Gagnon / @corivera / @alanrenmsft, The validation/saving and then closing dialog is applied for current admin workstream, do you guys think we can apply this to all dialogs for this release? users/logins/etc etc
There was a problem hiding this comment.
I'm in favor of making that the standard behavior across all the dialogs.
There was a problem hiding this comment.
+1, we should have consistent experiences and behaviors
I would actually suggest removing this change until we agree on how broadly to apply this - and then go in and update everything at once. This way we aren't left with one dialog being different than the others while we discuss.
There was a problem hiding this comment.
Sure, I'm not going to merge this change, but will add a commit that makes this logic consistent to all objectManagement dialogs. People can review and share their thoughts and then we can go forward with this PR. Thanks for your inputs here :)
There was a problem hiding this comment.
Can you do that in a separate branch? We should do that general update in a separate PR - so maybe just focus on the other logic in this PR and we can get that in first.
There was a problem hiding this comment.
Removed the button label changes from the PR.
These limited dialogs still use the onConfirmation() override method to validate and save changes while dialog is in open state. All other dialogs go as is from the initialize() method.
|
Can you file an issue to update the object management base dialog with these changes? |
|
I actually thought about this before and I was planning to implement it differently: the dialog will close immediately after the validation is passed so that user won't be blocked by potentially long running operations, and if it failed, user can click on the failed task and reopen the dialog to make edits. @erinstellato-ms which do you think is better? |
@alanrenmsft So the use would click "Save" or "Apply" and the dialog would close, but then if there was an error, the dialog would re-appear? I think this could be problematic - what if the user switched apps and then didn't realize it didn't save until much later? Were you considering this approach because there is no way to change focus out of the properties dialog while a change is saving? (Which means the user cannot do anything else in ADS while waiting...whereas in other tools, they may be able to un-dock or move the window.) |
|
@erinstellato-ms right now we display the status of the operation in the tasks pane, what I am proposing is that when it fails user can click on the entry in the tasks view and reopen the dialog to make changes instead of keeping the dialog open. |
|
Seems this needs a good discussion. Lets discuss on this in our Admin sync. |
|
@ssreerama if you're blocked by this at all, let me know and we (including @alanrenmsft) can find time to discuss sooner. |
This PR adds the ability to keep the dialog open till the changes been validated and saved, this helps the user to keep editing the changes when the validation fails and errors out.
Updated Image:
1. Create New Database: Items to notice:
2. Updating database properties: Items to notice