SF-3319 Configure usfm format in drafts

[RaymondLuong3]

This is the working concept for the configure USFM feature for drafts. It shows that you can navigate to the chapter and open a tab for the draft. Then the user can update the configuration on their project and the draft will show the version of the chapter with the select USFM configuration.

Please suggest wording improvements if the descriptions for the different marker formats in not clear.

---

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 89.60000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 81.91%. Comparing base (1d1ab99) to head (03c6d62).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...n/draft-usfm-format/draft-usfm-format.component.ts	86.76%	6 Missing and 3 partials ⚠️
...slate/draft-generation/draft-generation.service.ts	66.66%	2 Missing ⚠️
...pture/ClientApp/src/app/core/sf-project.service.ts	0.00%	1 Missing ⚠️
...SIL.XForge.Scripture/Services/MachineApiService.cs	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3182      +/-   ##
==========================================
+ Coverage   81.89%   81.91%   +0.01%     
==========================================
  Files         602      604       +2     
  Lines       34523    34644     +121     
  Branches     5602     5612      +10     
==========================================
+ Hits        28274    28380     +106     
- Misses       5417     5426       +9     
- Partials      832      838       +6     

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[josephmyers]

I'd be interested in seeing the other design options fleshed out here, as to my knowledge the team never discussed which design would be best. Maybe you received better or more recent guidance? I'm up for a video call, to get on the same page here.

[RaymondLuong3]

@josephmyers Let's plan to do a video call in the next day or so. I'll send you an invitation.

[josephmyers]

Thanks for your work so far on this. Did you encounter something that made what we talked about not feasible? I'm wondering what that was.

I do think that the layout you had for the larger screens worked better than what you have now, the most glaring reasons being that there's not a ton of room anymore to actually see your changes taking effect and that you have to scroll all the way to the top to access these options (meaning you can only see their impact for the very beginning of the text). It was helpful to be able to scroll through the text while toggling the options.

For smaller screens, what you have has the same issues, but I think it's acceptable on these screens. However, if you wanted to explore better options, we could do so. I think you could have a Formatting dropdown (or maybe icon?) anchored to the top of the screen, containing those checkboxes. Feel free to reach out if you want to pursue this and want more details.

One other thing I'm thinking: are you sure we need the Save changes feature? It just is an extra step, and I want to make sure it's required. Assuming it is, it has a UX issue in that it closes the current page. Typically, CTA buttons are not at the top of the page, especially if they close the page. The users will be following the workflow and clicking the CTA, probably before they've even seen the text below. Whatever layout we settle on, the Save changes button should only ever save (and not close the page). The alternative would be to have the button docked/anchored at the bottom.

Reviewed 30 of 30 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 4 at r1 (raw file):

  <h2>{{ t("usfm_format_title") }}</h2>
  @if (!isOnline) {
    <p class="offline-text">{{ t("connect_to_the_internet") }}</p>

Nit: when offline, if you change any of these toggles, the scripture content goes blank. It's a bit weird, but understandable. Should we disable these controls while offline?

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate.module.ts line 66 at r1 (raw file):

    FontUnsupportedMessageComponent
  ],
  exports: [EditorDraftComponent]

I think this is unused?

[Nateowami]

Overall this looks pretty good, but I have a few changes to request:

1. The chapter should not completely disappear when it is being reloaded. Half a second of it disappearing looks kind of wrong to me, but for users with slower networks (or when something is broken), it will not be obvious what is happening. Instead, set an opacity of about 0.5 on the app-text, then remove it once the new chapter is swapped in.
2. The editor, not the entire page, should scroll (whether this is important to do now depends on whether we're keeping this page as-is, or moving it to a tab, so we can postpone this until we decided where this UI is going).
3. The explanation text with the checkboxes is extremely small and light. We should at least choose a size and color combination that passes the AA standard (see https://webaim.org/resources/contrastchecker/), which is really not that strict. I'm guessing you copied this style from the settings page; it's a poor choice there as well. The color picker in the chromium dev tools has built-in contrast measurement to help with this.
4. I think the save and cancel buttons would probably be better as footer actions in the card. Also, I think "Save changes" can be just "Save", and the icons we use on the configure sources page can be added to the buttons.
5. If you put the button to get to this page behind a feature flag, we can probably merge this fairly soon (even if we ultimately decided to put the UI somewhere else).
6. Can you ask the Serval team (or someone else), for a good example project or USFM file for testing these settings?

[Nateowami]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 15 at r1 (raw file):

              <span class="description">{{ t("paragraph_marker_description") }}</span>
            </mat-checkbox>
            <mat-checkbox formControlName="preserveStyles">

The preserve styles option has been cancelled/indefinitely postponed. At the very least it needs to be put behind a feature flag, separate from the feature flag for this page, as we're not shipping it in the near term, if ever.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 19 at r1 (raw file):

              <span class="description">{{ t("style_marker_description") }}</span>
            </mat-checkbox>
            <mat-checkbox formControlName="preserveEmbeds">

The preserve embeds option likewise is cancelled/postponed and needs the same treatment.

[RaymondLuong3]

@josephmyers

Did you encounter something that made what we talked about not feasible? I'm wondering what that was.

I'm not sure if I remember exactly what you were referring to. Was this about implementing this as a draft tab?

I do think that the layout you had for the larger screens worked better than what you have now

Thanks for the input. I have adjusted this now so that on small screens or larger, the options and text viewer are side by side. On smaller screens, the viewer can be quite narrow, but the benefit of the side-by-side view is really nice and worth the narrowed view.

One other thing I'm thinking: are you sure we need the Save changes feature?

Yes, there is a technical reason for this. When we save the changes we also need to update the current draft that is stored in mongo with the draft using the new usfm format. Otherwise I think it would be reasonable to omit the save button altogether. So the save button saves the configuration and updates the stored draft.

@Nateowami
Thanks for the through feedback. I am leaning more towards implementing this page separate from the draft tab because I think it makes sense to keep configuration options on the generate draft page. Since the most common workflow is that the user starts at the generate draft page to begin a new draft or configure sources, to me it also makes sense that updating the usfm format could be accessed on this page also.

The chapter should not completely disappear when it is being reloaded.

Excellent idea. Done.

The editor, not the entire page, should scroll

I agree! It took me a while to get this to work, but I think it looks much better now.

The explanation text with the checkboxes is extremely small and light

I agree. Thanks for the helpful resource. I've updated the text to meet the AA guideline. I haven't tried to implement proper material theming yet, I'm hoping to get the most important pieces in before updating the theming.

I think the save and cancel buttons would probably be better as footer actions in the card

I've added the save button as a footer to the card. The cancel button seems misleading to me. I wanted to use "close", but that also didn't seem right. Instead, I chose Back similar to the draft generation steps component.

If you put the button to get to this page behind a feature flag,

Done.

Reviewable status: 13 of 33 files reviewed, 2 unresolved discussions (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 4 at r1 (raw file):

Previously, josephmyers wrote…

Nit: when offline, if you change any of these toggles, the scripture content goes blank. It's a bit weird, but understandable. Should we disable these controls while offline?

Good idea. I've disabled the form when the user is offline.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 15 at r1 (raw file):

Previously, Nateowami wrote…

The preserve styles option has been cancelled/indefinitely postponed. At the very least it needs to be put behind a feature flag, separate from the feature flag for this page, as we're not shipping it in the near term, if ever.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 19 at r1 (raw file):

Previously, Nateowami wrote…

The preserve embeds option likewise is cancelled/postponed and needs the same treatment.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate.module.ts line 66 at r1 (raw file):

Previously, josephmyers wrote…

I think this is unused?

Done.

[josephmyers]

Well, my original suggestion, which I think would have worked best, had either a menu icon or dropdown in the tab toolbar (it could have been on the edit page, or on a new page like you've made). I think you had mentioned some good reason why you didn't want to go that route, but in lieu of that, we had talked a little about still using the menu button or dropdown, only elsewhere. Here's a real rough draft of what I was thinking:

However! Now that there's only one option left, my idea makes less sense. Honestly, at this point, having a whole page dedicated to this one toggle doesn't make much sense, either. So I'm fine with the current direction, given these updates and the work we've put into this already. I'm also fine with discussing a more appropriate direction for this one remaining toggle.

Reviewed 21 of 21 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 4 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Good idea. I've disabled the form when the user is offline.

You did disable the form, great. But now the whole chapter hides. We want to keep the text visible while offline.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 23 at r2 (raw file):

Previously, josephmyers wrote…

Yes, there is a technical reason for this. When we save the changes we also need to update the current draft that is stored in mongo with the draft using the new usfm format. Otherwise I think it would be reasonable to omit the save button altogether. So the save button saves the configuration and updates the stored draft.

The above should be a quote from you, but Reviewable needs to get their icons (and tooltips) better. Sorry!

Regarding the save capability, I do think it'd be better from a usability standpoint and a UI layout standpoint, if we could do away with Save. For this, we'd Save to mongo whenever they toggle, and then they wouldn't have to worry about saved or unsaved changes, etc. I do think it'd be valuable, but probably not valuable enough to be worth our time at this point.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 2 at r2 (raw file):

<ng-container *transloco="let t; read: 'draft_usfm_format'">
  <h2>{{ t("usfm_format_title") }}</h2>

Page titles should be an h1. You could argue that this isn't a page title, but then I'd say we need one, haha

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 17 at r2 (raw file):

            <mat-checkbox formControlName="preserveParagraphs">
              <span>{{ t("preserve_paragraphs") }}</span>
              <span class="description">{{ t("paragraph_marker_description") }}</span>

Too things to fix here for dark mode:

• Spacing above form
• Description text

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss line 5 at r2 (raw file):

:host {
  height: 80vh;

This is weird to me, but maybe you intended it? We generally try to avoid this, however. One problem I can instantly see with this is that the page is cut off at the bottom, on desktop, and the page content extends past the bounds of the host element. This is viewable with Chrome Dev Tools, and will cause us problems if we ever need something docked at the bottom of the page.

I'm sure you added this to work around the issue that our top section is taking up too much space on mobile, such that the scripture portion is unusable unless you scroll down first. It's something that we could solve by taking up less space to begin with.

Alternatively, you can take a look at how the other pages, like the editor, have solved this. I was able to get this to work locally (there were two scrollbars, as expected) by matching the CSS of the editor exactly, no template restructuring needed.

[josephmyers]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @RaymondLuong3)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 17 at r2 (raw file):

Previously, josephmyers wrote…

Too things to fix here for dark mode:

• Spacing above form
• Description text

*Two (wow)

[RaymondLuong3]

I see. Yes, I agree that now there is only 1 option for preserving paragraphs that we should probably not put it behind another toggle or dropdown and just show the option.

Reviewable status: 29 of 35 files reviewed, 4 unresolved discussions (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 4 at r1 (raw file):

Previously, josephmyers wrote…

You did disable the form, great. But now the whole chapter hides. We want to keep the text visible while offline.

Good thinking. This should work as you mentioned now.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 2 at r2 (raw file):

Previously, josephmyers wrote…

Page titles should be an h1. You could argue that this isn't a page title, but then I'd say we need one, haha

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 17 at r2 (raw file):

Previously, josephmyers wrote…

*Two (wow)

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html line 23 at r2 (raw file):

Previously, josephmyers wrote…

The above should be a quote from you, but Reviewable needs to get their icons (and tooltips) better. Sorry!

Regarding the save capability, I do think it'd be better from a usability standpoint and a UI layout standpoint, if we could do away with Save. For this, we'd Save to mongo whenever they toggle, and then they wouldn't have to worry about saved or unsaved changes, etc. I do think it'd be valuable, but probably not valuable enough to be worth our time at this point.

Yes, it would be more expensive to update the drafts anytime someone changes this setting, but it can be done and we can always come back to it and implement it that way. Maybe we are better leaving it with the current approach and if we really do not like it, we can update it to not require the save.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss line 5 at r2 (raw file):

Previously, josephmyers wrote…

This is weird to me, but maybe you intended it? We generally try to avoid this, however. One problem I can instantly see with this is that the page is cut off at the bottom, on desktop, and the page content extends past the bounds of the host element. This is viewable with Chrome Dev Tools, and will cause us problems if we ever need something docked at the bottom of the page.

I'm sure you added this to work around the issue that our top section is taking up too much space on mobile, such that the scripture portion is unusable unless you scroll down first. It's something that we could solve by taking up less space to begin with.

Alternatively, you can take a look at how the other pages, like the editor, have solved this. I was able to get this to work locally (there were two scrollbars, as expected) by matching the CSS of the editor exactly, no template restructuring needed.

I had to make some adjustments, but I finally figured out that the issue I had was that the host container was not a flex container, which is why elements seemed to overflow their parent components. I've updated it and now it is working as expected. Child elements will not overflow the parent elements.

[josephmyers]

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

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss line 5 at r2 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

I had to make some adjustments, but I finally figured out that the issue I had was that the host container was not a flex container, which is why elements seemed to overflow their parent components. I've updated it and now it is working as expected. Child elements will not overflow the parent elements.

Thanks for fixing this. I think what you have looks solid, though I do note that a downside of this iteration is that the entire top section is now fixed to the top of the page, where before you could scroll most of it away. However, the page is usable on the smaller phone presets from Chrome, like iPhone SE. So all in all, good job on this