refactor: component viewer use state for timeout so timeout isnt set if no render eventually occurs

app/assets/javascripts/Components/Abstract/PureComponent.tsx

@@ -54,10 +54,6 @@ export abstract class PureComponent<P = PureComponentProps, S = PureComponentSta
     this.deinit()
   }
 
-  render() {
-    return <div>Must override</div>
-  }
-
   public get appState(): AppState {
     return this.application.getAppState()
   }

app/assets/javascripts/Components/ComponentView/index.tsx

@@ -9,7 +9,7 @@ import {
 } from '@standardnotes/snjs'
 import { WebApplication } from '@/UIModels/Application'
 import { FunctionalComponent } from 'preact'
-import { useCallback, useEffect, useMemo, useRef, useState } from 'preact/hooks'
+import { useCallback, useEffect, useRef, useState } from 'preact/hooks'
 import { observer } from 'mobx-react-lite'
 import { OfflineRestricted } from '@/Components/ComponentView/OfflineRestricted'
 import { UrlMissing } from '@/Components/ComponentView/UrlMissing'
@@ -25,7 +25,6 @@ interface IProps {
   componentViewer: ComponentViewer
   requestReload?: (viewer: ComponentViewer, force?: boolean) => void
   onLoad?: (component: SNComponent) => void
-  manualDealloc?: boolean
 }
 
 /**
@@ -39,7 +38,7 @@ const MSToWaitAfterIframeLoadToAvoidFlicker = 35
 export const ComponentView: FunctionalComponent<IProps> = observer(
   ({ application, onLoad, componentViewer, requestReload }) => {
     const iframeRef = useRef<HTMLIFrameElement>(null)
-    const excessiveLoadingTimeout = useRef<ReturnType<typeof setTimeout> | undefined>(undefined)
+    const [loadTimeout, setLoadTimeout] = useState<ReturnType<typeof setTimeout> | undefined>(undefined)
 
     const [hasIssueLoading, setHasIssueLoading] = useState(false)
     const [isLoading, setIsLoading] = useState(true)
@@ -88,7 +87,8 @@ export const ComponentView: FunctionalComponent<IProps> = observer(
       }
     }, [hasIssueLoading, componentViewer, requestReload])
 
-    const handleIframeTakingTooLongToLoad = useCallback(async () => {
+    useEffect(() => {
+      const loadTimeout = setTimeout(() => {
         setIsLoading(false)
         setHasIssueLoading(true)
 
@@ -98,24 +98,25 @@ export const ComponentView: FunctionalComponent<IProps> = observer(
         } else {
           document.addEventListener(VisibilityChangeKey, onVisibilityChange)
         }
-    }, [didAttemptReload, onVisibilityChange, componentViewer, requestReload])
-
-    useMemo(() => {
-      const loadTimeout = setTimeout(() => {
-        handleIframeTakingTooLongToLoad().catch(console.error)
       }, MaxLoadThreshold)
 
-      excessiveLoadingTimeout.current = loadTimeout
+      setLoadTimeout(loadTimeout)
 
       return () => {
-        excessiveLoadingTimeout.current && clearTimeout(excessiveLoadingTimeout.current)
+        if (loadTimeout) {
+          clearTimeout(loadTimeout)
         }
-    }, [handleIframeTakingTooLongToLoad])
+      }
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [componentViewer])
 
     const onIframeLoad = useCallback(() => {
       const iframe = iframeRef.current as HTMLIFrameElement
       const contentWindow = iframe.contentWindow as Window
-      excessiveLoadingTimeout.current && clearTimeout(excessiveLoadingTimeout.current)
+
+      if (loadTimeout) {
+        clearTimeout(loadTimeout)
+      }
 
       try {
         componentViewer.setWindow(contentWindow)
@@ -128,7 +129,7 @@ export const ComponentView: FunctionalComponent<IProps> = observer(
         setHasIssueLoading(false)
         onLoad?.(component)
       }, MSToWaitAfterIframeLoadToAvoidFlicker)
-    }, [componentViewer, onLoad, component, excessiveLoadingTimeout])
+    }, [componentViewer, onLoad, component, loadTimeout])
 
     useEffect(() => {
       const removeFeaturesChangedObserver = componentViewer.addEventObserver((event) => {

app/assets/javascripts/Components/NoteView/NoteView.tsx

@@ -264,21 +264,26 @@ export class NoteView extends PureComponent<Props, State> {
 
     let title = this.state.editorTitle,
       text = this.state.editorText
+
     if (isPayloadSourceRetrieved(source)) {
       title = note.title
       text = note.text
     }
+
     if (!this.state.editorTitle) {
       title = note.title
     }
+
     if (!this.state.editorText) {
       text = note.text
     }
+
     if (title !== this.state.editorTitle) {
       this.setState({
         editorTitle: title,
       })
     }
+
     if (text !== this.state.editorText) {
       this.setState({
         editorText: text,
@@ -317,6 +322,7 @@ export class NoteView extends PureComponent<Props, State> {
     if (this.state.editorComponentViewer) {
       this.application.componentManager?.destroyComponentViewer(this.state.editorComponentViewer)
     }
+
     super.componentWillUnmount()
   }
 
@@ -401,12 +407,15 @@ export class NoteView extends PureComponent<Props, State> {
 
   dismissProtectedWarning = async () => {
     let showNoteContents = true
+
     if (this.application.hasProtectionSources()) {
       showNoteContents = await this.application.authorizeNoteAccess(this.note)
     }
+
     if (!showNoteContents) {
       return
     }
+
     this.setShowProtectedOverlay(false)
     this.focusTitle()
   }
@@ -1021,6 +1030,7 @@ export class NoteView extends PureComponent<Props, State> {
             {this.state.editorComponentViewer && (
               <div className="component-view">
                 <ComponentView
+                  key={this.state.editorComponentViewer.identifier}
                   componentViewer={this.state.editorComponentViewer}
                   onLoad={this.onEditorComponentLoad}
                   requestReload={this.editorComponentViewerRequestsReload}
@@ -1099,7 +1109,6 @@ export class NoteView extends PureComponent<Props, State> {
                     <ComponentView
                       key={viewer.identifier}
                       componentViewer={viewer}
-                      manualDealloc={true}
                       application={this.application}
                       appState={this.appState}
                     />

app/assets/javascripts/UIModels/AppState/NotesViewState.ts

@@ -14,7 +14,7 @@ import {
   SNTag,
   SystemViewId,
 } from '@standardnotes/snjs'
-import { action, computed, makeObservable, observable, reaction } from 'mobx'
+import { action, computed, makeObservable, observable, reaction, runInAction } from 'mobx'
 import { AppState, AppStateEvent } from '.'
 import { WebApplication } from '../Application'
 import { AbstractState } from './AbstractState'
@@ -228,7 +228,9 @@ export class NotesViewState extends AbstractState {
 
     this.notes = notes
 
+    runInAction(() => {
       this.renderedNotes = renderedNotes
+    })
 
     await this.recomputeSelectionAfterNotesReload()
 
@@ -258,7 +260,7 @@ export class NotesViewState extends AbstractState {
 
     const noteExistsInUpdatedResults = this.notes.find((note) => note.uuid === activeNote.uuid)
     if (!noteExistsInUpdatedResults && !isSearching) {
-      this.application.noteControllerGroup.closeNoteController(activeController)
+      this.closeNoteController(activeController)
 
       this.selectNextNote()
 
@@ -318,11 +320,12 @@ export class NotesViewState extends AbstractState {
   reloadPreferences = async () => {
     const freshDisplayOptions = {} as DisplayOptions
     const currentSortBy = this.displayOptions.sortBy
+
     let sortBy = this.application.getPreference(PrefKey.SortNotesBy, CollectionSort.CreatedAt)
     if (sortBy === CollectionSort.UpdatedAt || (sortBy as string) === 'client_updated_at') {
-      /** Use UserUpdatedAt instead */
       sortBy = CollectionSort.UpdatedAt
     }
+
     freshDisplayOptions.sortBy = sortBy
     freshDisplayOptions.sortReverse = this.application.getPreference(PrefKey.SortNotesReverse, false)
     freshDisplayOptions.showArchived = this.application.getPreference(PrefKey.NotesShowArchived, false)
@@ -468,17 +471,29 @@ export class NotesViewState extends AbstractState {
 
   selectNextNote = () => {
     const displayableNotes = this.notes
+
     const currentIndex = displayableNotes.findIndex((candidate) => {
       return candidate.uuid === this.activeControllerNote?.uuid
     })
 
-    if (currentIndex + 1 < displayableNotes.length) {
+    let nextIndex = currentIndex + 1
-      const nextNote = displayableNotes[currentIndex + 1]
+
+    while (nextIndex < displayableNotes.length) {
+      const nextNote = displayableNotes[nextIndex]
+
+      nextIndex++
+
+      if (nextNote.protected) {
+        continue
+      }
 
       this.selectNoteWithScrollHandling(nextNote).catch(console.error)
 
       const nextNoteElement = document.getElementById(`note-${nextNote.uuid}`)
+
       nextNoteElement?.focus()
+
+      return
     }
   }
 
@@ -495,20 +510,31 @@ export class NotesViewState extends AbstractState {
   selectPreviousNote = () => {
     const displayableNotes = this.notes
 
-    if (this.activeControllerNote) {
+    if (!this.activeControllerNote) {
-      const currentIndex = displayableNotes.indexOf(this.activeControllerNote)
+      return
-      if (currentIndex - 1 >= 0) {
-        const previousNote = displayableNotes[currentIndex - 1]
-        this.selectNoteWithScrollHandling(previousNote).catch(console.error)
-        const previousNoteElement = document.getElementById(`note-${previousNote.uuid}`)
-        previousNoteElement?.focus()
-        return true
-      } else {
-        return false
-      }
     }
 
-    return undefined
+    const currentIndex = displayableNotes.indexOf(this.activeControllerNote)
+
+    let previousIndex = currentIndex - 1
+
+    while (previousIndex >= 0) {
+      const previousNote = displayableNotes[previousIndex]
+
+      previousIndex--
+
+      if (previousNote.protected) {
+        continue
+      }
+
+      this.selectNoteWithScrollHandling(previousNote).catch(console.error)
+
+      const previousNoteElement = document.getElementById(`note-${previousNote.uuid}`)
+
+      previousNoteElement?.focus()
+
+      return
+    }
   }
 
   setNoteFilterText = (text: string) => {
@@ -543,10 +569,14 @@ export class NotesViewState extends AbstractState {
     }
   }
 
+  private closeNoteController(controller: NoteViewController): void {
+    this.application.noteControllerGroup.closeNoteController(controller)
+  }
+
   handleTagChange = () => {
     const activeNoteController = this.getActiveNoteController()
     if (activeNoteController?.isTemplateNote) {
-      this.application.noteControllerGroup.closeNoteController(activeNoteController)
+      this.closeNoteController(activeNoteController)
     }
 
     this.resetScrollPosition()
@@ -591,7 +621,9 @@ export class NotesViewState extends AbstractState {
     if (this.searchSubmitted) {
       this.searchSubmitted = false
     }
+
     this.reloadNotesDisplayOptions()
+
     void this.reloadNotes()
   }