From a77bf5b525ad52e8f4cae7171a442ad972d976c1 Mon Sep 17 00:00:00 2001 From: Mo Date: Tue, 17 May 2022 22:58:55 -0500 Subject: [PATCH] refactor: component viewer use state for timeout so timeout isnt set if no render eventually occurs --- .../Components/Abstract/PureComponent.tsx | 4 -- .../Components/ComponentView/index.tsx | 45 ++++++------ .../Components/NoteView/NoteView.tsx | 11 ++- .../UIModels/AppState/NotesViewState.ts | 70 ++++++++++++++----- 4 files changed, 84 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/Components/Abstract/PureComponent.tsx b/app/assets/javascripts/Components/Abstract/PureComponent.tsx index 1d562d7ec..341b3737b 100644 --- a/app/assets/javascripts/Components/Abstract/PureComponent.tsx +++ b/app/assets/javascripts/Components/Abstract/PureComponent.tsx @@ -54,10 +54,6 @@ export abstract class PureComponent

Must override - } - public get appState(): AppState { return this.application.getAppState() } diff --git a/app/assets/javascripts/Components/ComponentView/index.tsx b/app/assets/javascripts/Components/ComponentView/index.tsx index c1ae26b32..65851f376 100644 --- a/app/assets/javascripts/Components/ComponentView/index.tsx +++ b/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 = observer( ({ application, onLoad, componentViewer, requestReload }) => { const iframeRef = useRef(null) - const excessiveLoadingTimeout = useRef | undefined>(undefined) + const [loadTimeout, setLoadTimeout] = useState | undefined>(undefined) const [hasIssueLoading, setHasIssueLoading] = useState(false) const [isLoading, setIsLoading] = useState(true) @@ -88,34 +87,36 @@ export const ComponentView: FunctionalComponent = observer( } }, [hasIssueLoading, componentViewer, requestReload]) - const handleIframeTakingTooLongToLoad = useCallback(async () => { - setIsLoading(false) - setHasIssueLoading(true) - - if (!didAttemptReload) { - setDidAttemptReload(true) - requestReload?.(componentViewer) - } else { - document.addEventListener(VisibilityChangeKey, onVisibilityChange) - } - }, [didAttemptReload, onVisibilityChange, componentViewer, requestReload]) - - useMemo(() => { + useEffect(() => { const loadTimeout = setTimeout(() => { - handleIframeTakingTooLongToLoad().catch(console.error) + setIsLoading(false) + setHasIssueLoading(true) + + if (!didAttemptReload) { + setDidAttemptReload(true) + requestReload?.(componentViewer) + } else { + document.addEventListener(VisibilityChangeKey, onVisibilityChange) + } }, 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 = observer( setHasIssueLoading(false) onLoad?.(component) }, MSToWaitAfterIframeLoadToAvoidFlicker) - }, [componentViewer, onLoad, component, excessiveLoadingTimeout]) + }, [componentViewer, onLoad, component, loadTimeout]) useEffect(() => { const removeFeaturesChangedObserver = componentViewer.addEventObserver((event) => { diff --git a/app/assets/javascripts/Components/NoteView/NoteView.tsx b/app/assets/javascripts/Components/NoteView/NoteView.tsx index 41de3de7e..1086e78b0 100644 --- a/app/assets/javascripts/Components/NoteView/NoteView.tsx +++ b/app/assets/javascripts/Components/NoteView/NoteView.tsx @@ -264,21 +264,26 @@ export class NoteView extends PureComponent { 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 { if (this.state.editorComponentViewer) { this.application.componentManager?.destroyComponentViewer(this.state.editorComponentViewer) } + super.componentWillUnmount() } @@ -401,12 +407,15 @@ export class NoteView extends PureComponent { 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 { {this.state.editorComponentViewer && (

{ diff --git a/app/assets/javascripts/UIModels/AppState/NotesViewState.ts b/app/assets/javascripts/UIModels/AppState/NotesViewState.ts index 028293104..df6bcb876 100644 --- a/app/assets/javascripts/UIModels/AppState/NotesViewState.ts +++ b/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 - this.renderedNotes = renderedNotes + 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) { - const nextNote = displayableNotes[currentIndex + 1] + let nextIndex = 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) { - const currentIndex = displayableNotes.indexOf(this.activeControllerNote) - 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 - } + if (!this.activeControllerNote) { + return } - 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() }