SF-3126 Combine Add Draft dialogs

[josephmyers]

This includes:

• Delaying the creation of the project-select component until the projects list has loaded. This follows the basic pattern outlined by other usages of this component, and of course enables us to set the project on dialog creation. Setting the value programmatically after creation proved to be quite tricky, which explains why pages like Settings avoided it.

• Replaced dependence on targetProject$ in the template with the form control. Since the former was being changed in the event handler in the apply dialog, starting with a value (e.g. the current project) was causing the handler to be called immediately on dialog startup, which in turn triggered "changed after checked" exceptions. Apparently, depending on a different but equivalent property fixes this, as it likely isn't updated in quite the same cycle. I didn't solve this til much later, with it clearing up many issues, so it's possible some of my other changes aren't fully necessary.

• Using isLoading on the apply dialog. This wasn't being used at all beforehand. We consider the component to be loaded as soon as the projects are populated. This triggers the project-select to display.

• Removed a parameter from the project-select custom validation. It didn't appear to be used, though I could have been missing something.

---

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.02%. Comparing base (fa16e6a) to head (c8647b8).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...draft-apply-dialog/draft-apply-dialog.component.ts	83.33%	1 Missing and 1 partial ⚠️
...src/app/project-select/project-select.component.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2954      +/-   ##
==========================================
- Coverage   82.02%   82.02%   -0.01%     
==========================================
  Files         544      544              
  Lines       31693    31691       -2     
  Branches     5144     5152       +8     
==========================================
- Hits        25997    25995       -2     
- Misses       4927     4928       +1     
+ Partials      769      768       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Nateowami]

Can we have the Jira issue number on the PR?

[RaymondLuong3]

Nice work. I think these improvements will go a long way!

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 96 at r1 (raw file):

  }

  @Input() set value(id: string) {

Nit: You could just return early if this.paratextIdControl.value.id === id

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-apply-dialog/draft-apply-dialog.component.ts line 177 at r1 (raw file):

  }

  private validateProject(): void {

Can you leave a comment here describing why we need setTimeout? In my testing, there is an error that appears when I remove setTimeout

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts line 126 at r1 (raw file):

    const bookName: string = this.bookNumberToName(bookWithDraft.bookNumber);
    const confirmed = await this.dialogService.confirmWithOptions({
      title: bookWithDraft.draftApplied

Don't forget to remove these string from the non_checking_en.json file

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-apply-dialog/draft-apply-dialog.component.html line 22 at r1 (raw file):

        <mat-error class="offline-message">{{ t("connect_to_the_internet") }}</mat-error>
      }
      @if (projectSelectValid && canEditProject) {

This looks like a tricky one to identify :)

Code quote:

      @if (projectSelectValid && canEditProject) {

[josephmyers]

Reviewable status: 4 of 7 files reviewed, 3 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 96 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: You could just return early if this.paratextIdControl.value.id === id

Done

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-apply-dialog/draft-apply-dialog.component.html line 22 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

This looks like a tricky one to identify :)

In what way? What are you trying to identify?

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-apply-dialog/draft-apply-dialog.component.ts line 177 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Can you leave a comment here describing why we need setTimeout? In my testing, there is an error that appears when I remove setTimeout

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-preview-books/draft-preview-books.component.ts line 126 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Don't forget to remove these string from the non_checking_en.json file

Done.

[RaymondLuong3]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-apply-dialog/draft-apply-dialog.component.html line 22 at r1 (raw file):

Previously, josephmyers wrote…

In what way? What are you trying to identify?

I meant this looked like a trick problem that you solved.

---

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json line 145 at r2 (raw file):

    "project_name": "Project Name"
  },
  "draft_add_dialog": {

I think all of draft_add_dialog can be removed now.

Code quote:

  "draft_add_dialog": {

[josephmyers]

Reviewable status: 2 of 7 files reviewed, 2 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-apply-dialog/draft-apply-dialog.component.html line 22 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

I meant this looked like a trick problem that you solved.

It was a little tough to find out what was actually causing the exception. So I rearranged the check here to something equivalent to fix it

---

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json line 145 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

I think all of draft_add_dialog can be removed now.

Thanks. I had done a search for the keys throughout the rest of the code for usages, but I see now that we have other keys with exactly the same name.

[RaymondLuong3]

Nice work! I hope this ends up being helpful and a welcome change. Sometimes I feel we interrupt workflows too much with rapid changes. I wonder how many videos we publish that get out-of-date too fast...

Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @josephmyers)

[josephmyers]

Yes, I agree. Let's hope we're only making videos for things that have been stable for a little while at least. And we can also remember the benefits of code reuse and design consistency. It will help the users, even when they don't know it!