From 717c77d91e9660562509b781a2c620b3db3f5b15 Mon Sep 17 00:00:00 2001
From: tjcouch-sil <tj_couch@sil.org>
Date: Fri, 19 Apr 2024 16:36:13 -0500
Subject: [PATCH] Fixed not scrolling properly on chapter change and adjusted
 for ux feedback

---
 extensions/src/platform-scripture/src/main.ts |  4 +-
 extensions/src/resource-viewer/src/main.ts    |  4 +-
 .../src/resource-viewer.web-view.tsx          | 52 ++++++++++++++-----
 src/renderer/app.component.scss               |  1 +
 src/renderer/testing/lorem-ipsum.ts           | 21 ++++++++
 src/renderer/testing/test-layout.data.ts      |  6 +++
 src/renderer/testing/test-panel.component.tsx |  8 ++-
 7 files changed, 78 insertions(+), 18 deletions(-)
 create mode 100644 src/renderer/testing/lorem-ipsum.ts

diff --git a/extensions/src/platform-scripture/src/main.ts b/extensions/src/platform-scripture/src/main.ts
index 918d5aca2e..d5b6c73b71 100644
--- a/extensions/src/platform-scripture/src/main.ts
+++ b/extensions/src/platform-scripture/src/main.ts
@@ -16,7 +16,9 @@ const booksPresentValidator: ProjectSettingValidator<'platformScripture.booksPre
 const versificationValidator: ProjectSettingValidator<'platformScripture.versification'> = async (
   newValue: number,
 ): Promise<boolean> => {
-  return typeof newValue === 'number' && newValue >= 0 && newValue <= 6;
+  return (
+    typeof newValue === 'number' && newValue >= 0 && newValue <= 6 && Number.isInteger(newValue)
+  );
 };
 
 // #endregion
diff --git a/extensions/src/resource-viewer/src/main.ts b/extensions/src/resource-viewer/src/main.ts
index 2743e1a113..48e1d5af96 100644
--- a/extensions/src/resource-viewer/src/main.ts
+++ b/extensions/src/resource-viewer/src/main.ts
@@ -58,9 +58,7 @@ const resourceWebViewProvider: IWebViewProvider = {
       undefined;
     return {
       title: projectId
-        ? `Resource Viewer : ${
-            (await papi.projectLookup.getMetadataForProject(projectId)).name ?? projectId
-          }`
+        ? (await papi.projectLookup.getMetadataForProject(projectId)).name ?? projectId
         : 'Resource Viewer',
       ...savedWebView,
       content: resourceViewerWebView,
diff --git a/extensions/src/resource-viewer/src/resource-viewer.web-view.tsx b/extensions/src/resource-viewer/src/resource-viewer.web-view.tsx
index 7f4e7362a6..7ba7f2dac4 100644
--- a/extensions/src/resource-viewer/src/resource-viewer.web-view.tsx
+++ b/extensions/src/resource-viewer/src/resource-viewer.web-view.tsx
@@ -13,6 +13,15 @@ import {
   usxStringToUsj,
 } from '@biblionexus-foundation/platform-editor';
 
+/** The offset in pixels from the top of the window to scroll to show the verse number */
+const VERSE_NUMBER_SCROLL_OFFSET = 80;
+
+/**
+ * Time in ms to delay taking action to wait for the editor to load. Hope to be obsoleted by a way
+ * to listen for the editor to finish loading
+ */
+const EDITOR_LOAD_DELAY_TIME = 100;
+
 const defaultScrRef: ScriptureReference = {
   bookNum: 1,
   chapterNum: 1,
@@ -20,14 +29,20 @@ const defaultScrRef: ScriptureReference = {
 };
 
 function scrollToScrRef(scrRef: ScriptureReference) {
-  // We are querying for a span, so this Element will be an HTMLElement
-  // eslint-disable-next-line no-type-assertion/no-type-assertion
-  const verseElement = document.querySelector(
+  const verseElement = document.querySelector<HTMLElement>(
     `.editor-container span[data-marker="v"][data-number="${scrRef.verseNum}"]`,
-  ) as HTMLElement | undefined;
-  if (verseElement) {
+  );
+
+  // Scroll if we find the verse or we're at the start of the chapter
+  if (verseElement || scrRef.verseNum === 1) {
+    // If we're at the first verse, scroll to the top so we can see intro material
+    let scrollTop = 0;
+    if (verseElement && scrRef.verseNum > 1)
+      scrollTop =
+        verseElement.getBoundingClientRect().top + window.scrollY - VERSE_NUMBER_SCROLL_OFFSET;
+
     window.scrollTo({
-      top: verseElement.getBoundingClientRect().top + window.scrollY - 55,
+      top: scrollTop,
       behavior: 'smooth',
     });
   }
@@ -38,7 +53,6 @@ globalThis.webViewComponent = function ResourceViewer({
   useWebViewState,
 }: WebViewProps): JSX.Element {
   const [projectId] = useWebViewState<string>('projectId', '');
-  logger.debug(`Resource Viewer project ID: ${projectId}`);
 
   // Using react's ref api which uses null, so we must use null
   // eslint-disable-next-line no-null/no-null
@@ -85,9 +99,9 @@ globalThis.webViewComponent = function ResourceViewer({
   useEffect(() => {
     if (usx && !hasFirstRetrievedScripture.current) {
       hasFirstRetrievedScripture.current = true;
-      // Wait 100 ms before scrolling to make sure there is plenty of time for the editor to load
+      // Wait before scrolling to make sure there is time for the editor to load
       // TODO: hook into the editor and detect when it has loaded somehow
-      setTimeout(() => scrollToScrRef(scrRef), 100);
+      setTimeout(() => scrollToScrRef(scrRef), EDITOR_LOAD_DELAY_TIME);
     }
   }, [usx, scrRef]);
 
@@ -106,13 +120,25 @@ globalThis.webViewComponent = function ResourceViewer({
       return () => {};
     }
 
-    // Add a highlight to the current verse element
-    const highlightedVerseElement = scrollToScrRef(scrRef);
-    if (highlightedVerseElement) highlightedVerseElement.classList.add('highlighted');
+    // Using react's ref api which uses null, so we must use null
+    // eslint-disable-next-line no-null/no-null
+    let highlightedVerseElement: HTMLElement | null;
+
+    // Wait before scrolling to make sure there is time for the editor to load
+    // TODO: hook into the editor and detect when it has loaded somehow
+    const scrollTimeout = setTimeout(() => {
+      // Scroll to and add a highlight to the current verse element
+      highlightedVerseElement = scrollToScrRef(scrRef);
+      if (highlightedVerseElement) highlightedVerseElement.classList.add('highlighted');
 
-    internallySetScrRefRef.current = undefined;
+      internallySetScrRefRef.current = undefined;
+    }, EDITOR_LOAD_DELAY_TIME);
 
     return () => {
+      // Cancel this timeout to scroll if it is running because the scrRef changed and we need to
+      // scroll somewhere else
+      clearTimeout(scrollTimeout);
+
       // Remove highlight from the current verse element
       if (highlightedVerseElement) highlightedVerseElement.classList.remove('highlighted');
     };
diff --git a/src/renderer/app.component.scss b/src/renderer/app.component.scss
index 403c9eb279..e1a234d84f 100644
--- a/src/renderer/app.component.scss
+++ b/src/renderer/app.component.scss
@@ -71,4 +71,5 @@ a:hover {
 
 .test-panel {
   margin: 0.5em;
+  overflow-y: auto;
 }
diff --git a/src/renderer/testing/lorem-ipsum.ts b/src/renderer/testing/lorem-ipsum.ts
new file mode 100644
index 0000000000..4c2db08eb9
--- /dev/null
+++ b/src/renderer/testing/lorem-ipsum.ts
@@ -0,0 +1,21 @@
+const LOREM_IPSUM = `Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam convallis aliquet justo vitae vehicula. Vestibulum tincidunt porttitor urna, non scelerisque nibh commodo quis. Nunc vel porta nisl, in maximus massa. Donec convallis sem in magna iaculis, ut sagittis lectus semper. Aliquam pellentesque orci et scelerisque egestas. Sed ac vehicula arcu. Etiam rutrum sapien risus, a venenatis urna accumsan sit amet. In vel turpis vitae risus tristique tincidunt. Aenean et leo vel lacus venenatis bibendum. Donec semper libero sed purus efficitur, condimentum lacinia odio rhoncus. Proin et quam magna. Cras risus arcu, dictum vel sapien a, accumsan lacinia est. Pellentesque sit amet dolor nec ex pellentesque aliquam. Mauris eget libero gravida, egestas orci sed, feugiat erat.
+
+Fusce eget mattis eros. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Aenean consectetur turpis urna, ut ullamcorper tortor ultricies non. Phasellus nec bibendum quam, eget rutrum dolor. Integer vel tempor neque. Suspendisse id mollis nisl. Nunc finibus ac enim sed congue. Vivamus ac accumsan turpis.
+
+Aliquam eu tempor nibh. Curabitur scelerisque tortor purus, volutpat efficitur magna pellentesque sit amet. Curabitur id pulvinar sem, vel porta eros. Maecenas nunc urna, imperdiet nec lobortis eget, rutrum a neque. Suspendisse eu vulputate lorem. Interdum et malesuada fames ac ante ipsum primis in faucibus. Nam dictum at ante ut dignissim. Sed tempor tempus odio id varius. Praesent eu neque leo. Mauris bibendum dui sollicitudin, imperdiet justo ut, porttitor justo. Nulla feugiat mi id dictum feugiat. Duis feugiat arcu risus, a tincidunt ex faucibus sed. Donec egestas lectus sit amet tincidunt maximus. Mauris scelerisque, mauris suscipit dapibus pretium, nisi nulla dignissim metus, elementum tincidunt massa ipsum vel velit.
+
+Donec sit amet felis leo. Duis vitae sem id libero sodales iaculis. Interdum et malesuada fames ac ante ipsum primis in faucibus. Duis eu dolor condimentum, pretium libero quis, ornare sapien. Nullam efficitur ornare ipsum nec tincidunt. Interdum et malesuada fames ac ante ipsum primis in faucibus. Nulla iaculis ultrices lacus in sagittis. Maecenas facilisis fringilla turpis ac cursus. Duis commodo fringilla dui.
+
+Fusce convallis massa nec lectus venenatis vulputate vel id justo. Donec vel mauris orci. Suspendisse finibus accumsan lacus a feugiat. Pellentesque interdum pellentesque orci, at ultrices ipsum aliquam a. Curabitur scelerisque, diam nec commodo vestibulum, neque nisl feugiat lorem, et iaculis nisl mi sit amet dui. Mauris ut nisl viverra, mollis felis id, aliquet tellus. Praesent ullamcorper faucibus lectus, maximus pellentesque mi vehicula et. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Maecenas metus felis, interdum nec urna sed, sodales ornare eros. Quisque volutpat justo sed sem aliquet, in fermentum lectus feugiat. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Integer quis ultricies lacus, euismod placerat dolor. Donec tempor ullamcorper ex, sit amet euismod ligula pharetra sed. Fusce sagittis molestie odio et iaculis. Nullam id dolor aliquet risus venenatis congue eget id erat. Ut eget vulputate est, nec luctus quam.
+
+Mauris nec condimentum nulla, eu malesuada lectus. Integer blandit tellus eu sem gravida aliquam. Donec convallis lacinia ante in porta. Aliquam nec felis suscipit, efficitur mi non, convallis diam. Etiam in mollis nisi. Cras pellentesque luctus justo, vel convallis elit eleifend ut. Ut pulvinar nisl urna, in volutpat dui facilisis a. Sed in imperdiet ligula. Praesent feugiat neque nibh, non eleifend ex vestibulum in.
+
+Mauris auctor nibh at magna posuere commodo. Maecenas sit amet fermentum lorem, eu rhoncus augue. Aenean eu urna ultricies, consequat augue nec, eleifend libero. Nunc eu diam sagittis, condimentum quam quis, vestibulum leo. Pellentesque nec ante in purus lacinia luctus. In non gravida tellus. Duis quam erat, faucibus at pharetra eu, blandit a nunc. Sed felis quam, bibendum a orci id, sodales facilisis mauris. Fusce feugiat nisi nulla, ut vehicula mauris pulvinar in. Aliquam scelerisque faucibus risus et dignissim. Suspendisse ut finibus mi. Proin finibus aliquet feugiat. Sed et tortor vestibulum, pharetra libero in, facilisis quam. Etiam bibendum efficitur ligula id congue. Suspendisse sit amet sapien ac nisl maximus ullamcorper id eget ante.
+
+Aliquam consequat elit et rhoncus porttitor. Sed condimentum pulvinar lorem, at varius ante laoreet ut. Suspendisse egestas euismod lorem, quis pretium odio iaculis eu. Aliquam erat volutpat. Cras faucibus, elit quis scelerisque vestibulum, est justo pharetra urna, et sodales augue magna a leo. Fusce eu massa convallis, commodo sapien quis, sagittis ex. Ut quis mattis erat. Ut purus massa, varius sit amet tempus efficitur, cursus eleifend velit. Aliquam a ipsum eu velit dictum semper. Nulla dignissim at tellus nec interdum. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Suspendisse sed ultrices dui. Sed rhoncus turpis elit, eget pharetra ligula pharetra ut.
+
+Quisque quis est nec orci aliquet dictum id vulputate turpis. Ut nisl sapien, aliquam sed pulvinar sed, dignissim ac ipsum. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae; Fusce auctor odio facilisis purus porta ullamcorper. Morbi tincidunt lorem ac orci hendrerit, eget sollicitudin metus sagittis. Nunc pellentesque justo quis massa interdum congue. Praesent a ultrices justo.
+
+Suspendisse laoreet metus a malesuada laoreet. Aliquam erat volutpat. Nam sagittis mattis rutrum. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae; Nunc eget tempor felis. Sed ultricies justo vitae lorem elementum, sit amet tincidunt lorem imperdiet. Maecenas id porta mauris, eu auctor augue. Vestibulum ultrices erat at augue tempus malesuada. Aliquam facilisis urna at lectus feugiat, sit amet porta erat sagittis. Nulla interdum condimentum leo vitae dictum. Donec semper suscipit auctor. Nunc a nibh arcu. Integer vehicula magna vel augue scelerisque tempus. Aenean hendrerit sem et mi sollicitudin, vel dignissim libero fringilla. Nam at interdum elit.`;
+
+export default LOREM_IPSUM;
diff --git a/src/renderer/testing/test-layout.data.ts b/src/renderer/testing/test-layout.data.ts
index 700d7a8a93..8adb0a38ee 100644
--- a/src/renderer/testing/test-layout.data.ts
+++ b/src/renderer/testing/test-layout.data.ts
@@ -10,6 +10,7 @@ import { TAB_TYPE_TEST } from '@renderer/testing/test-panel.component';
 import { TAB_TYPE_SETTINGS_DIALOG } from '@renderer/components/settings-dialog/settings-tab.component';
 import { TAB_TYPE_RUN_BASIC_CHECKS } from '@renderer/components/run-basic-checks-dialog/run-basic-checks-tab.component';
 import { TAB_TYPE_BASIC_LIST } from '@renderer/components/basic-list/basic-list.component';
+import LOREM_IPSUM from './lorem-ipsum';
 
 export const FIRST_TAB_ID = 'About';
 
@@ -28,6 +29,11 @@ const testLayout: LayoutBase = {
               { id: 'About', tabType: TAB_TYPE_ABOUT },
               { id: 'Test Tab Two', tabType: TAB_TYPE_TEST },
               { id: 'Test Tab One', tabType: TAB_TYPE_TEST },
+              {
+                id: 'Lorem Ipsum',
+                tabType: TAB_TYPE_TEST,
+                data: { content: LOREM_IPSUM },
+              },
             ] as SavedTabInfo[],
           },
         ],
diff --git a/src/renderer/testing/test-panel.component.tsx b/src/renderer/testing/test-panel.component.tsx
index 90bcc19f98..ca9a39510b 100644
--- a/src/renderer/testing/test-panel.component.tsx
+++ b/src/renderer/testing/test-panel.component.tsx
@@ -2,6 +2,8 @@ import { SavedTabInfo, TabInfo } from '@shared/models/docking-framework.model';
 
 export const TAB_TYPE_TEST = 'tab';
 
+type TestTabData = { content?: string } | undefined;
+
 export default function TestPanel({ content }: { content: string }) {
   return <div className="test-panel">{content}</div>;
 }
@@ -9,8 +11,12 @@ export default function TestPanel({ content }: { content: string }) {
 export function loadTestTab(savedTabInfo: SavedTabInfo): TabInfo {
   if (!savedTabInfo.id) throw new Error('Tab creation "id" is missing');
 
+  // We set this in `test-layout.data.ts`
+  // eslint-disable-next-line no-type-assertion/no-type-assertion
+  const data = savedTabInfo.data as TestTabData;
+
   const tabTitle = savedTabInfo.id || 'Unknown';
-  const content = savedTabInfo.id ? `Content for ${tabTitle}` : 'Unknown';
+  const content = data?.content ?? (savedTabInfo.id ? `Content for ${tabTitle}` : 'Unknown');
   return {
     ...savedTabInfo,
     tabTitle,