From a08eaaf0833d20d948dff761d2f87dabfb4bbdd9 Mon Sep 17 00:00:00 2001 From: linearcombination <4829djaskdfj@gmail.com> Date: Fri, 7 Jun 2024 14:36:18 -0700 Subject: [PATCH 1/4] Fix several UI behavioral oddities These are behavior oddities/annoyances that happen when you modify or create more than one document request during the same browser session session. --- frontend/src/lib/WizardBreadcrumb.svelte | 24 +--- frontend/src/lib/stores/ResourceTypesStore.ts | 13 +- frontend/src/lib/stores/SettingsStore.ts | 2 + frontend/src/lib/utils.ts | 23 +--- frontend/src/routes/books/+page.svelte | 89 ++++++++----- frontend/src/routes/languages/+page.svelte | 2 - .../src/routes/resource_types/+page.svelte | 4 +- frontend/src/routes/settings/+page.svelte | 65 ++++++--- .../routes/settings/GenerateDocument.svelte | 124 ++++++++---------- frontend/src/routes/settings/Switch.svelte | 6 +- 10 files changed, 183 insertions(+), 169 deletions(-) diff --git a/frontend/src/lib/WizardBreadcrumb.svelte b/frontend/src/lib/WizardBreadcrumb.svelte index 77172ccd..a2e68515 100644 --- a/frontend/src/lib/WizardBreadcrumb.svelte +++ b/frontend/src/lib/WizardBreadcrumb.svelte @@ -6,7 +6,7 @@ import { resourceTypesStore, resourceTypesCountStore } from '$lib/stores/ResourceTypesStore' import { getResourceTypeLangCode, - // resetStores, + resetStores, langRegExp, bookRegExp, resourceTypeRegExp, @@ -17,33 +17,17 @@ import DesktopBreadcrumb from '$lib/DesktopBreadcrumb.svelte' function submitLanguages() { - // If books store or resource types store are not empty, then we - // should reset them when we change the languages. - if ($bookCountStore > 0 || $resourceTypesCountStore > 0) { - $resetValuesStore = true - } - // resetStores('books') - // resetStores('resource_types') - // resetStores('settings') - // resetStores('notifications') + resetStores('notifications') goto('/books') } function submitBooks() { - // If resource types store is not empty, then we - // should reset it when we change books. - if ($resourceTypesCountStore > 0) { - $resetValuesStore = true - } - // resetStores('resource_types') - // resetStores('settings') - // resetStores('notifications') + resetStores('notifications') goto('/resource_types') } function submitResourceTypes() { - // resetStores('settings') - // resetStores('notifications') + resetStores('notifications') goto('/settings') } diff --git a/frontend/src/lib/stores/ResourceTypesStore.ts b/frontend/src/lib/stores/ResourceTypesStore.ts index 5a97b09b..d46a3c73 100644 --- a/frontend/src/lib/stores/ResourceTypesStore.ts +++ b/frontend/src/lib/stores/ResourceTypesStore.ts @@ -1,8 +1,7 @@ -import { writable } from 'svelte/store'; -import type { Writable } from 'svelte/store'; +import { writable } from 'svelte/store' +import type { Writable } from 'svelte/store' -export let resourceTypesStore: Writable> = writable>([]); -export let lang0ResourceTypesStore: Writable> = writable>([]); -export let lang1ResourceTypesStore: Writable> = writable>([]); -export let resourceTypesCountStore: Writable = writable(0); -export let twResourceRequestedStore: Writable = writable(false); +export let resourceTypesStore: Writable> = writable>([]) +export let lang0ResourceTypesStore: Writable> = writable>([]) +export let lang1ResourceTypesStore: Writable> = writable>([]) +export let resourceTypesCountStore: Writable = writable(0) diff --git a/frontend/src/lib/stores/SettingsStore.ts b/frontend/src/lib/stores/SettingsStore.ts index 226ead22..c9d2ecf2 100644 --- a/frontend/src/lib/stores/SettingsStore.ts +++ b/frontend/src/lib/stores/SettingsStore.ts @@ -16,3 +16,5 @@ export let generateDocxStore: Writable = writable(false) export let emailStore: Writable = writable(null) export let documentRequestKeyStore: Writable = writable('') export let settingsUpdated: Writable = writable(false) +export let twResourceRequestedStore: Writable = writable(false) +export let usfmResourceRequestedStore: Writable = writable(false) diff --git a/frontend/src/lib/utils.ts b/frontend/src/lib/utils.ts index fbdd2a4d..d1d6d2e9 100644 --- a/frontend/src/lib/utils.ts +++ b/frontend/src/lib/utils.ts @@ -16,8 +16,7 @@ import { // TODO should we include resourceTypesStore and reset it also? lang0ResourceTypesStore, lang1ResourceTypesStore, - resourceTypesCountStore, - twResourceRequestedStore + resourceTypesCountStore } from '$lib/stores/ResourceTypesStore' import { documentReadyStore, errorStore } from '$lib/stores/NotificationStore' import { @@ -26,7 +25,8 @@ import { generatePdfStore, generateEpubStore, generateDocxStore, - documentRequestKeyStore + documentRequestKeyStore, + twResourceRequestedStore } from '$lib/stores/SettingsStore' const languageBookOrder: string = PUBLIC_LANGUAGE_BOOK_ORDER @@ -77,23 +77,6 @@ export function resetStores(storeGroup: StoreGroup) { } } -// FIXME: These are too inconsequential to be here, just use them from $env/dynamic/private where needed -// export function getApiRootUrl(): string { -// return PUBLIC_BACKEND_API_URL -// } - -// export function getFileServerUrl(): string { -// return env.PUBLIC_FILE_SERVER_URL -// } - -// export function getLogRocketId(): string { -// return env.PUBLIC_LOGROCKET_ID -// } - -/** - * Indicate whether to show Mast, Tabs, and Sidebar - **/ - export function getName(codeAndName: string): string { return codeAndName?.split(/, (.*)/s)[1] } diff --git a/frontend/src/routes/books/+page.svelte b/frontend/src/routes/books/+page.svelte index dab26344..caa5f62a 100644 --- a/frontend/src/routes/books/+page.svelte +++ b/frontend/src/routes/books/+page.svelte @@ -49,40 +49,67 @@ if ($langCountStore > 1) { getSharedBookCodesAndNames($langCodesStore[0], $langCodesStore[1]) .then((bookCodesAndNames) => { - // Filter set of all resource codes into old testament - // book codes. - otBookCodes = bookCodesAndNames - .filter((element: [string, string]) => { - return otBooks.some((item) => item === element[0]) - }) - .map((tuple) => `${tuple[0]}, ${tuple[1]}`) + if (bookCodesAndNames) { + // Filter set of all resource codes into old testament + // book codes. + otBookCodes = bookCodesAndNames + .filter((element: [string, string]) => { + return otBooks.some((item) => item === element[0]) + }) + .map((tuple) => `${tuple[0]}, ${tuple[1]}`) - // If otBookStore has contents, then assume we are coming - // back here from the user clicking to edit their book - // selections in the wizard basket, so we want to eliminate - // any otBookStore elements that are not in otBookCodes. - if ($otBookStore.length > 0) { - $otBookStore = $otBookStore.filter((item) => { - return otBookCodes.some((element) => element === item) - }) - } + // If otBookStore has contents, then assume we are coming + // back here from the user clicking to edit their book + // selections in the wizard basket, so we want to eliminate + // any otBookStore elements that are not in otBookCodes. + if ($otBookStore.length > 0) { + $otBookStore = $otBookStore.filter((item) => { + return otBookCodes.some((element) => element === item) + }) + } - // Filter set of all book codes into new testament - // book codes. - ntBookCodes = bookCodesAndNames - .filter((element: [string, string]) => { - return !otBooks.some((item) => item === element[0]) - }) - .map((tuple) => `${tuple[0]}, ${tuple[1]}`) + // Filter set of all book codes into new testament + // book codes. + ntBookCodes = bookCodesAndNames + .filter((element: [string, string]) => { + return !otBooks.some((item) => item === element[0]) + }) + .map((tuple) => `${tuple[0]}, ${tuple[1]}`) - // If ntBookStore has contents, then assume we are coming - // back here from the user clicking to edit their book - // selections in the wizard basket, so we want to eliminate - // any ntBookStore elements that are not in ntBookCodes. - if ($ntBookStore.length > 0) { - $ntBookStore = $ntBookStore.filter((item) => { - return ntBookCodes.some((element) => element === item) - }) + // If ntBookStore has contents, then assume we are coming + // back here from the user clicking to edit their book + // selections in the wizard basket, so we want to eliminate + // any ntBookStore elements that are not in ntBookCodes. + if ($ntBookStore.length > 0) { + $ntBookStore = $ntBookStore.filter((item) => { + return ntBookCodes.some((element) => element === item) + }) + } + } else { + // Filter set of all resource codes into old testament + // book codes. + otBookCodes = [] + // If otBookStore has contents, then assume we are coming + // back here from the user clicking to edit their book + // selections in the wizard basket, so we want to eliminate + // any otBookStore elements that are not in otBookCodes. + if ($otBookStore.length > 0) { + $otBookStore = $otBookStore.filter((item) => { + return otBookCodes.some((element) => element === item) + }) + } + // Filter set of all book codes into new testament + // book codes. + ntBookCodes = [] + // If ntBookStore has contents, then assume we are coming + // back here from the user clicking to edit their book + // selections in the wizard basket, so we want to eliminate + // any ntBookStore elements that are not in ntBookCodes. + if ($ntBookStore.length > 0) { + $ntBookStore = $ntBookStore.filter((item) => { + return ntBookCodes.some((element) => element === item) + }) + } } }) .catch((err) => console.error(err)) diff --git a/frontend/src/routes/languages/+page.svelte b/frontend/src/routes/languages/+page.svelte index 73177e8c..b1e68064 100644 --- a/frontend/src/routes/languages/+page.svelte +++ b/frontend/src/routes/languages/+page.svelte @@ -58,13 +58,11 @@ .then((langCodeNameAndTypes_) => { // Save result for later use langCodeNameAndTypes = langCodeNameAndTypes_ - gatewayCodesAndNames = langCodeNameAndTypes_ .filter((element: [string, string, boolean]) => { return element[2] }) .map((tuple) => `${tuple[0]}, ${tuple[1]}`) - heartCodesAndNames = langCodeNameAndTypes_ .filter((element: [string, string, boolean]) => { return !element[2] diff --git a/frontend/src/routes/resource_types/+page.svelte b/frontend/src/routes/resource_types/+page.svelte index a423b658..3aa03b03 100644 --- a/frontend/src/routes/resource_types/+page.svelte +++ b/frontend/src/routes/resource_types/+page.svelte @@ -22,7 +22,7 @@ ): Promise> { // Form the URL to ultimately invoke // resource_lookup.shared_resource_types. - const url_ = `${apiRootUrl}${sharedResourceTypesUrl}${langCode}/` + const url_ = `${apiRootUrl}${sharedResourceTypesUrl}${langCode}` const url = new URL(url_) bookCodeAndNames.map((bookCodeAndName) => url.searchParams.append('book_codes', bookCodeAndName[0]) @@ -209,7 +209,7 @@ {/if} - {#if $langCountStore > 1 && lang1ResourceTypesAndNames} + {#if $langCountStore > 1}

{$langNamesStore[1]}

diff --git a/frontend/src/routes/settings/+page.svelte b/frontend/src/routes/settings/+page.svelte index eeed7224..7fb8e89d 100644 --- a/frontend/src/routes/settings/+page.svelte +++ b/frontend/src/routes/settings/+page.svelte @@ -12,14 +12,12 @@ emailStore, limitTwStore, documentRequestKeyStore, - settingsUpdated + settingsUpdated, + twResourceRequestedStore, + usfmResourceRequestedStore } from '$lib/stores/SettingsStore' - import { documentReadyStore } from '$lib/stores/NotificationStore' - import { - resourceTypesStore, - resourceTypesCountStore, - twResourceRequestedStore - } from '$lib/stores/ResourceTypesStore' + import { documentReadyStore, errorStore } from '$lib/stores/NotificationStore' + import { resourceTypesStore, resourceTypesCountStore } from '$lib/stores/ResourceTypesStore' import { langCodesStore, langCountStore } from '$lib/stores/LanguagesStore' import { bookCountStore } from '$lib/stores/BooksStore' import GenerateDocument from './GenerateDocument.svelte' @@ -35,14 +33,34 @@ // Set whether TW has been requested for any of the languages // requested so that we can use this fact in the UI to trigger the // presence or absence of the toggle to limit TW words. - let regexp = new RegExp('.*tw.*') + let twRegexp = new RegExp('.*tw.*') + let usfmRegexp = new RegExp('ayt|cuv|f10|nav|reg|udb|ugnt|uhb|ulb') $: { if ($resourceTypesStore) { - $twResourceRequestedStore = $resourceTypesStore.some((item) => regexp.test(item)) + $twResourceRequestedStore = $resourceTypesStore.some((item) => twRegexp.test(item)) + $usfmResourceRequestedStore = $resourceTypesStore.some((item) => usfmRegexp.test(item)) } } $: { - if ($twResourceRequestedStore) { + // If you use the commented out conditional, the user is always + // given the option of limiting translation words to those + // occuring in scripture (USFM) for the selected book(s) even + // if they have not requested scripture (USFM). Doing this is + // very inefficient to since, if limit is chosen, the USFM must be + // acquired, parsed, and analyzed during this last step and + // because it is done separately from the normal flow it leads to + // code duplication due to subtle differences in requesting USFM this + // late in the process, i.e., on the last step, which cannot be reused. + // + // The other option, and the one used here, is to only provide the + // option of limiting translation words if scripture (USFM) was + // actually requested by the user. This can make sense because + // translation words are a language scoped resource and not a + // book scoped resource so, presumably, if the user has requested + // translations words without scripture they very likely could + // want to just see all the translation words for that language. + // if ($twResourceRequestedStore) { + if ($twResourceRequestedStore && $usfmResourceRequestedStore) { $limitTwStore = true } else { $limitTwStore = false @@ -141,7 +159,10 @@ value={'docx'} bind:group={$docTypeStore} type="radio" - on:change={() => ($settingsUpdated = true)} + on:change={() => { + $settingsUpdated = true + $errorStore = '' + }} class="h-4 w-4 border-gray-300 bg-gray-100 text-blue-600 focus:ring-2 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800 dark:focus:ring-blue-600" /> Docx @@ -154,7 +175,10 @@ value={'epub'} bind:group={$docTypeStore} type="radio" - on:change={() => ($settingsUpdated = true)} + on:change={() => { + $settingsUpdated = true + $errorStore = '' + }} class="h-4 w-4 border-gray-300 bg-gray-100 text-blue-600 focus:ring-2 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800 dark:focus:ring-blue-600" /> ePub @@ -168,7 +192,10 @@ value={'pdf'} bind:group={$docTypeStore} type="radio" - on:change={() => ($settingsUpdated = true)} + on:change={() => { + $settingsUpdated = true + $errorStore = '' + }} class="h-4 w-4 border-gray-300 bg-gray-100 text-blue-600 focus:ring-2 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800 dark:focus:ring-blue-600" /> PDF @@ -186,7 +213,10 @@ value={'lbo'} bind:group={$assemblyStrategyKindStore} type="radio" - on:change={() => ($settingsUpdated = true)} + on:change={() => { + $settingsUpdated = true + $errorStore = '' + }} class="h-4 w-4 border-gray-300 bg-gray-100 text-blue-600 focus:ring-2 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800 dark:focus:ring-blue-600" /> Interleave content by book @@ -199,7 +229,10 @@ value={'blo'} bind:group={$assemblyStrategyKindStore} type="radio" - on:change={() => ($settingsUpdated = true)} + on:change={() => { + $settingsUpdated = true + $errorStore = '' + }} class="h-4 w-4 border-gray-300 bg-gray-100 text-blue-600 focus:ring-2 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800 dark:focus:ring-blue-600" /> Interleave content by chapter @@ -215,7 +248,7 @@ >Enabling this option will remove extra whitespace - {#if $twResourceRequestedStore} + {#if $twResourceRequestedStore && $usfmResourceRequestedStore}
Limit TW words diff --git a/frontend/src/routes/settings/GenerateDocument.svelte b/frontend/src/routes/settings/GenerateDocument.svelte index 97dbbf3a..a6f06e44 100644 --- a/frontend/src/routes/settings/GenerateDocument.svelte +++ b/frontend/src/routes/settings/GenerateDocument.svelte @@ -45,7 +45,6 @@ // Update some UI-related state generatingDocument = true $settingsUpdated = false - let resourceRequests = [] let bookCodes = [...$otBookStore, ...$ntBookStore] for (let bookCode of bookCodes) { @@ -70,7 +69,6 @@ } } } - // Create the JSON structure to POST. let documentRequest = { email_address: $emailStore, @@ -87,11 +85,11 @@ $errorStore = null $documentReadyStore = false $documentRequestKeyStore = '' - let endpointUrl = 'documents' + let endpointUrl = `${apiRootUrl}/documents` if ($generateDocxStore) { - endpointUrl = 'documents_docx' + endpointUrl = `${apiRootUrl}/documents_docx` } - const response = await fetch(`${apiRootUrl}/${endpointUrl}`, { + const response = await fetch(endpointUrl, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(documentRequest) @@ -102,9 +100,32 @@ $errorStore = data.detail } else { console.log(`data: ${JSON.stringify(data)}`) - // Setting value of taskIdStore will reactively trigger polling - // of task status. - $taskIdStore = data.task_id + const timerIntervalId = setInterval(async function () { + // Poll the server for the task state and result + let results = await poll(data.task_id) + console.log(`results: ${results}`) + $taskStateStore = Array.isArray(results) ? results[0] : results + console.log(`$taskStateStore: ${$taskStateStore}`) + if ($taskStateStore === 'SUCCESS' && Array.isArray(results) && results[1]) { + let finishedDocumentRequestKey = results[1] + console.log(`finishedDocumentReuestKey: ${finishedDocumentRequestKey}`) + // Update some UI-related state + $documentReadyStore = true + $documentRequestKeyStore = finishedDocumentRequestKey + $errorStore = null + $taskStateStore = '' + generatingDocument = false + clearInterval(timerIntervalId) + } else if ($taskStateStore === 'FAILURE') { + console.log("We're sorry, an internal error occurred which we'll investigate.") + // Update some UI-related state + $errorStore = + "We're sorry. An error occurred. The document you requested may not yet be supported or we may have experienced an internal problem which we'll investigate. Please try another document request." + $taskStateStore = '' + generatingDocument = false + clearInterval(timerIntervalId) + } + }, 5000) } } @@ -168,46 +189,30 @@ event.returnValue = `Are you sure you want to leave while your document is being generated?` } }) - - $: { - if ($taskIdStore) { - console.log(`$taskIdStore: ${$taskIdStore}`) - generatingDocument = true - const timer = setInterval(async function () { - // Poll the server for the task state and result - let results = await poll($taskIdStore) - console.log(`results: ${results}`) - $taskStateStore = Array.isArray(results) ? results[0] : results - console.log(`$taskStateStore: ${$taskStateStore}`) - if ($taskStateStore === 'SUCCESS' && Array.isArray(results) && results[1]) { - let finishedDocumentRequestKey = results[1] - console.log(`finishedDocumentReuestKey: ${finishedDocumentRequestKey}`) - - // Update some UI-related state - $documentReadyStore = true - $documentRequestKeyStore = finishedDocumentRequestKey - $errorStore = null - $taskStateStore = '' - generatingDocument = false - - clearInterval(timer) - } else if ($taskStateStore === 'FAILURE') { - console.log("We're sorry, an internal error occurred which we'll investigate.") - // Update some UI-related state - $errorStore = - "We're sorry. An error occurred. The document you requested may not yet be supported or we may have experienced an internal problem which we'll investigate. Please try another document request." - $taskStateStore = '' - generatingDocument = false - - clearInterval(timer) - } - }, 5000) - } - }
- {#if !generatingDocument || $settingsUpdated} + {#if $errorStore} +
+ + + +

Uh Oh...

+
+

+ Something went wrong. Please review your selections or contact tech support for + assistance. +

+
+
+ {:else if (!generatingDocument && !$documentReadyStore) || $settingsUpdated} {#if ($langCountStore > 0 || $langCountStore <= 2) && $assemblyStrategyKindStore && $bookCountStore > 0 && $resourceTypesCountStore > 0}
diff --git a/frontend/src/routes/settings/Switch.svelte b/frontend/src/routes/settings/Switch.svelte index dcc1b014..80e16df5 100644 --- a/frontend/src/routes/settings/Switch.svelte +++ b/frontend/src/routes/settings/Switch.svelte @@ -1,5 +1,6 @@