Made login page, project list, opening draft with editor decorations, menu item, joining project, error handling

[tjcouch-sil]

Requires https://github.com/paranext/paranext-core/tree/2192-support-slingshot to run.

Before squash:

74e7250 (HEAD -> hook-up-editor, origin/hook-up-editor) Handled errors, fixed joining project, removed unnecessary setting
bbc458f Joining and finishing generate draft updates project table, incorporated draft setup state, made ui wording more concise, other misc tweaks
6ea6b08 Enabled joining a project in the extension, used new usj endpoint
cd21035 Completed project table, added sample SF API for testing, misc UI tweaks
aaa00fa update web view menu contribution, add basic project list, add log in page
75d6817 add menu item to open scripture forge home
8f2604a Open editor with decorations to indicate it is ai draft
34a6e77 Registered PDPF in main, added project test buttons, misc cleanup

Login page:

Must log into Scripture Forge in browser:

Projects table:

View Draft opens the editor with some decorations:

---

This change is 

[jolierabideau]

This looks great! Thanks for the hard work on this TJ! Most of my comments are small things or localization. There are also some strings in the project list that need localized. Were we planning to do that for this first iteration?

Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @tjcouch-sil)

---

src/scripture-forge/contributions/menus.json line 7 at r1 (raw file):

    "items": [
      {
        "label": "%mainMenu_scripture_forge_openAutoDrafts_label%",

I know I did this one but same comment about "scripture_forge" vs "scriptureForge"

---

src/scripture-forge/src/projects/scripture-forge-sample-api.model.ts line 314 at r1 (raw file):

    const didSfTest6Finish = Date.now() - this.firstGetProjectsTime > 20 * 1000;

    const hasDraft = {

Should this be called something like completedDraftStatus?

---

src/scripture-forge/src/projects/scripture-forge-sample-api.model.ts line 331 at r1 (raw file):

      percentCompleted: 1,
      message: 'Completed',
      // eslint-disable-next-line no-type-assertion/no-type-assertion

Descriptive comment above?

---

src/scripture-forge/src/projects/scripture-forge-sample-api.model.ts line 354 at r1 (raw file):

    const didSfTest6Finish = Date.now() - this.firstGetProjectsTime > 20 * 1000;

    const finishedDraft = {

Should this be called draftStatus

---

src/scripture-forge/contributions/localizedStrings.json line 8 at r1 (raw file):

      "%project_settings_scriptureForge_projectId_label%": "Scripture Forge Project ID",
      "%project_settings_scriptureForge_projectId_description%": "This project's ID according to Scripture Forge. It is generated upon connecting a project to Scripture Forge. This is distinct from the project's ID according to Paratext. Empty means the project is not connected to Scripture Forge.",
      "%settings_scripture_forge_group1_label%": "Scripture Forge Settings",

Some of these keys are "scripture_forge" and some of them are "scriptureForge", should make them all match?

---

src/scripture-forge/src/home/home.web-view.tsx line 104 at r1 (raw file):

        <CardHeader>
          <CardTitle>AI Drafts extension</CardTitle>
          <CardDescription>By Scripture Forge</CardDescription>

Do these two need to be localized? There is also the markdown, "Error", "Scripture Forge", and "Auto Drafts."

---

src/scripture-forge/src/home/home.web-view.tsx line 127 at r1 (raw file):

          </Button>
          {loginErrorMessage && (
            <Alert variant="destructive">

This is duplicated, so it could be pulled out into one component. But it's small and I don't think its a big deal.

---

src/scripture-forge/src/main.ts line 30 at r1 (raw file):

    return {
      iconUrl: 'papi-extension://scriptureForge/assets/images/sf.svg',
      title: 'Auto Drafts',

Missing localized string here?

[tjcouch-sil]

Reviewable status: 8 of 17 files reviewed, 8 unresolved discussions (waiting on @jolierabideau)

---

src/scripture-forge/contributions/localizedStrings.json line 8 at r1 (raw file):

Previously, jolierabideau wrote…

Some of these keys are "scripture_forge" and some of them are "scriptureForge", should make them all match?

Good call

---

src/scripture-forge/contributions/menus.json line 7 at r1 (raw file):

Previously, jolierabideau wrote…

I know I did this one but same comment about "scripture_forge" vs "scriptureForge"

Good call

---

src/scripture-forge/src/main.ts line 30 at r1 (raw file):

Previously, jolierabideau wrote…

Missing localized string here?

Went ahead and pushed all the localization stuff

---

src/scripture-forge/src/home/home.web-view.tsx line 104 at r1 (raw file):

Previously, jolierabideau wrote…

Do these two need to be localized? There is also the markdown, "Error", "Scripture Forge", and "Auto Drafts."

Went ahead and pushed all the localization stuff

---

src/scripture-forge/src/home/home.web-view.tsx line 127 at r1 (raw file):

Previously, jolierabideau wrote…

This is duplicated, so it could be pulled out into one component. But it's small and I don't think its a big deal.

Done

---

src/scripture-forge/src/projects/scripture-forge-sample-api.model.ts line 314 at r1 (raw file):

Previously, jolierabideau wrote…

Should this be called something like completedDraftStatus?

Renamed

---

src/scripture-forge/src/projects/scripture-forge-sample-api.model.ts line 331 at r1 (raw file):

Previously, jolierabideau wrote…

Descriptive comment above?

Reworked to avoid this problem in the first place

---

src/scripture-forge/src/projects/scripture-forge-sample-api.model.ts line 354 at r1 (raw file):

Previously, jolierabideau wrote…

Should this be called draftStatus

Renamed

[jolierabideau]

Just one more comment, feel free to disregard if not relevant!

Reviewed 9 of 9 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tjcouch-sil)

---

src/scripture-forge/src/home/home.web-view.tsx line 98 at r2 (raw file):

  });

  const [localizedStrings] = useLocalizedStrings(localizedStringKeys);

Usually I see localizedStringKeys wrapped in a useMemo. I am not sure if that is only relevant in certain instances, or just added to make it more stable? Maybe it's not relevant when the keys are pulled out of the component like it is here.

[tjcouch-sil]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jolierabideau)

---

src/scripture-forge/src/home/home.web-view.tsx line 98 at r2 (raw file):

Previously, jolierabideau wrote…

Usually I see localizedStringKeys wrapped in a useMemo. I am not sure if that is only relevant in certain instances, or just added to make it more stable? Maybe it's not relevant when the keys are pulled out of the component like it is here.

If an array that is defined at the top level inside the function component and isn't wrapped in a useMemo or pulled out of the component, it will be re-created every render with a new reference. Because of how our useData (used inside useLocalizedStrings) works, this becomes a really big problem and causes an infinite loop.

Here, I defined the array outside the function component, so it won't be re-created every render. :)

[jolierabideau]

Thanks TJ!

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tjcouch-sil)