Grey-out buttons after submission

[shikhavani]

#456 # @olliefreeman @pjmonks

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Shikha Vani seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[pjmonks]

Generally the solution looks good, thank you. Please could you review my comments though to fix up a couple of things.

Also, there are linting issues found in your changes which our build picked up on. If you run this command you will get a HTML report, you'll only need to care about the errors found (there will be lots of warnings, please ignore those):

npm run eslint -- --format html -o eslint_report.html

@jamesrwelch You've mentioned that there may be more places where a submission button should be disabled whilst saving, perhaps the issue should be updated to list these places to clearly identify them? Then this PR could cover all those cases too.

[pjmonks]

Could this be moved to above the constructor please? So directly below the field parentFolder? This will keep all fields organised the same way.

Also, I would suggest renaming this to isSaving to give more semantic meaning.

[pjmonks]

There is a shorter way of setting this flag back to false without having to duplicate it. There is an rxjs operator called finalize() which, if included in the pipe(), will always be called regardless of whether it was successful or not. Including something like the following would then you only need to set this flag once:

.addToFolder()
.pipe(
  catchError(error => {
    // Error case
  }),
  finalize(() => this.disabledSave = false)  // Always called at the end
)
.subscribe((response) => {
  // Success case
});

[shikhavani]

Ok. Sure. I will go through it and will update you

…

On Sat, Feb 5, 2022 at 8:39 PM Peter Monks ***@***.***> wrote: ***@***.**** requested changes on this pull request. Generally the solution looks good, thank you. Please could you review my comments though to fix up a couple of things. Also, there are linting issues found in your changes which our build picked up on. If you run this command you will get a HTML report, you'll only need to care about the errors found (there will be lots of warnings, please ignore those): npm run eslint -- --format html -o eslint_report.html @jamesrwelch <https://github.com/jamesrwelch> You've mentioned that there may be more places where a submission button should be disabled whilst saving, perhaps the issue should be updated to list these places to clearly identify them? Then this PR could cover all those cases too. ------------------------------ In src/app/wizards/dataModel/data-model-main/data-model-main.component.ts <#457 (comment)> : > @@ -51,6 +51,7 @@ export class DataModelMainComponent implements OnInit { private folders: FolderService, private title: Title) { } + disabledSave = false; Could this be moved to above the constructor please? So directly below the field parentFolder? This will keep all fields organised the same way. Also, I would suggest renaming this to isSaving to give more semantic meaning. ------------------------------ In src/app/wizards/dataModel/data-model-main/data-model-main.component.ts <#457 (comment)> : > this.stateHandler.Go('datamodel', { id: response.body.id }, { reload: true, location: true }); + }, error => { + this.disabledSave = false; There is a shorter way of setting this flag back to false without having to duplicate it. There is an rxjs operator called finalize() which, if included in the pipe(), will always be called regardless of whether it was successful or not. Including something like the following would then you only need to set this flag once: .addToFolder().pipe( catchError(error => { // Error case }), finalize(() => this.disabledSave = false) // Always called at the end).subscribe((response) => { // Success case}); — Reply to this email directly, view it on GitHub <#457 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIPSN4RLOCKSOREIFPQEWKDUZU4RXANCNFSM5NMEJK4Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[joe-crawford]

Thanks for your PR @shikhavani. Sorry for the delay, we'd missed this update. Would you be able to sign the Contributor Licence Agreement for this to be merged?

Link to the CLA: https://cla-assistant.io/MauroDataMapper/mdm-ui?pullRequest=457

cc @jamesrwelch