Add note loading boundary (#2040)

* Remove redundant equal value

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Add NoteLoadingBoundary to fetch note from API before rendering

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Improve debug message for setHandler

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Add test for boundary

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Use common error page for note loading errors

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Fix tests

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Format code

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Add missing snapshot

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

* Reformat code

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>
This commit is contained in:
Tilman Vatteroth 2022-05-11 12:47:58 +02:00 committed by GitHub
parent 0419113d36
commit 880e542351
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 282 additions and 166 deletions

View file

@ -40,6 +40,15 @@
"uploadMessage": "Uploading file"
}
},
"noteLoadingBoundary": {
"errorWhileLoadingContent": "Error while loading note."
},
"api": {
"note": {
"notFound": "Note not found.",
"accessDenied": "You don't have the needed permission to access this note."
}
},
"landing": {
"intro": {
"exploreFeatures": "Explore all features",

View file

@ -15,7 +15,9 @@ import { DeleteApiRequestBuilder } from '../common/api-request-builder/delete-ap
* @return Content and metadata of the specified note.
*/
export const getNote = async (noteIdOrAlias: string): Promise<Note> => {
const response = await new GetApiRequestBuilder<Note>('notes/' + noteIdOrAlias).sendRequest()
const response = await new GetApiRequestBuilder<Note>('notes/' + noteIdOrAlias)
.withStatusCodeErrorMapping({ 404: 'api.note.notFound', 403: 'api.note.accessDenied' })
.sendRequest()
return response.asParsedJsonObject()
}

View file

@ -5,6 +5,6 @@
*/
export class ApplicationLoaderError extends Error {
constructor(taskName: string) {
super(`Task ${taskName} failed`)
super(`The task ${taskName} failed`)
}
}

View file

@ -5,7 +5,7 @@
*/
import type { PropsWithChildren } from 'react'
import React, { Suspense } from 'react'
import React, { Fragment, Suspense, useMemo } from 'react'
import { createSetUpTaskList } from './initializers'
import { LoadingScreen } from './loading-screen/loading-screen'
import { Logger } from '../../utils/logger'
@ -27,8 +27,20 @@ export const ApplicationLoader: React.FC<PropsWithChildren<unknown>> = ({ childr
}
}, [])
if (loading) {
return <LoadingScreen failedTaskName={error?.message} />
const errorBlock = useMemo(() => {
if (error) {
return (
<Fragment>
{error.message}
<br />
For further information look into the browser console.
</Fragment>
)
}
}, [error])
if (loading || !!errorBlock) {
return <LoadingScreen errorMessage={errorBlock} />
} else {
return <Suspense fallback={<LoadingScreen />}>{children}</Suspense>
}

View file

@ -4,6 +4,7 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
import type { ReactElement } from 'react'
import React from 'react'
import { Alert } from 'react-bootstrap'
import { LoadingAnimation } from './loading-animation'
@ -11,7 +12,7 @@ import { ShowIf } from '../../common/show-if/show-if'
import styles from '../application-loader.module.scss'
export interface LoadingScreenProps {
failedTaskName?: string
errorMessage?: string | ReactElement
}
/**
@ -19,20 +20,16 @@ export interface LoadingScreenProps {
*
* @param failedTaskName Should be set if a task failed to load. The name will be shown on screen.
*/
export const LoadingScreen: React.FC<LoadingScreenProps> = ({ failedTaskName }) => {
export const LoadingScreen: React.FC<LoadingScreenProps> = ({ errorMessage }) => {
return (
<div className={`${styles.loader} ${styles.middle} text-light overflow-hidden`}>
<div className='mb-3 text-light'>
<span className={`d-block`}>
<LoadingAnimation error={!!failedTaskName} />
<LoadingAnimation error={!!errorMessage} />
</span>
</div>
<ShowIf condition={!!failedTaskName}>
<Alert variant={'danger'}>
The task {failedTaskName} failed.
<br />
For further information look into the browser console.
</Alert>
<ShowIf condition={!!errorMessage}>
<Alert variant={'danger'}>{errorMessage}</Alert>
</ShowIf>
</div>
)

View file

@ -0,0 +1,32 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`Note loading boundary loads a note 1`] = `
<div>
<span
data-testid="success"
>
success!
</span>
</div>
`;
exports[`Note loading boundary shows an error 1`] = `
<div>
<span
data-testid="CommonErrorPage"
>
This is a mock for CommonErrorPage.
</span>
<span>
titleI18nKey:
noteLoadingBoundary.errorWhileLoadingContent
</span>
<span>
descriptionI18nKey:
CRAAAAASH
</span>
<span>
children:
</span>
</div>
`;

View file

@ -0,0 +1,28 @@
/*
* SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { useAsync } from 'react-use'
import { getNote } from '../../../../api/notes'
import { setNoteDataFromServer } from '../../../../redux/note-details/methods'
import { useSingleStringUrlParameter } from '../../../../hooks/common/use-single-string-url-parameter'
import type { AsyncState } from 'react-use/lib/useAsyncFn'
/**
* Reads the note id from the current URL, requests the note from the backend and writes it into the global application state.
*
* @return An {@link AsyncState async state} that represents the current state of the loading process.
*/
export const useLoadNoteFromServer = (): AsyncState<void> => {
const id = useSingleStringUrlParameter('noteId', undefined)
return useAsync(async () => {
if (id === undefined) {
throw new Error('Invalid id')
}
const noteFromServer = await getNote(id)
setNoteDataFromServer(noteFromServer)
}, [id])
}

View file

@ -0,0 +1,115 @@
/*
* SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import * as useSingleStringUrlParameterModule from '../../../hooks/common/use-single-string-url-parameter'
import * as getNoteModule from '../../../api/notes'
import * as setNoteDataFromServerModule from '../../../redux/note-details/methods'
import type { Note } from '../../../api/notes/types'
import { Mock } from 'ts-mockery'
import { render, screen } from '@testing-library/react'
import { NoteLoadingBoundary } from './note-loading-boundary'
import { testId } from '../../../utils/test-id'
import { Fragment } from 'react'
import { mockI18n } from '../../markdown-renderer/test-utils/mock-i18n'
import * as CommonErrorPageModule from '../../error-pages/common-error-page'
import * as LoadingScreenModule from '../../../components/application-loader/loading-screen/loading-screen'
describe('Note loading boundary', () => {
const mockedNoteId = 'mockedNoteId'
afterEach(() => {
jest.resetAllMocks()
jest.resetModules()
})
beforeEach(async () => {
await mockI18n()
jest.spyOn(LoadingScreenModule, 'LoadingScreen').mockImplementation(({ errorMessage }) => {
return (
<Fragment>
<span {...testId('LoadingScreen')}>This is a mock for LoadingScreen.</span>
<span>errorMessage: {errorMessage}</span>
</Fragment>
)
})
jest
.spyOn(CommonErrorPageModule, 'CommonErrorPage')
.mockImplementation(({ titleI18nKey, descriptionI18nKey, children }) => {
return (
<Fragment>
<span {...testId('CommonErrorPage')}>This is a mock for CommonErrorPage.</span>
<span>titleI18nKey: {titleI18nKey}</span>
<span>descriptionI18nKey: {descriptionI18nKey}</span>
<span>children: {children}</span>
</Fragment>
)
})
mockGetNoteIdQueryParameter()
})
const mockGetNoteIdQueryParameter = () => {
const expectedQueryParameter = 'noteId'
jest.spyOn(useSingleStringUrlParameterModule, 'useSingleStringUrlParameter').mockImplementation((parameter) => {
expect(parameter).toBe(expectedQueryParameter)
return mockedNoteId
})
}
const mockGetNoteApiCall = (returnValue: Note) => {
jest.spyOn(getNoteModule, 'getNote').mockImplementation((id) => {
expect(id).toBe(mockedNoteId)
return new Promise((resolve) => {
setTimeout(() => resolve(returnValue), 0)
})
})
}
const mockCrashingNoteApiCall = () => {
jest.spyOn(getNoteModule, 'getNote').mockImplementation((id) => {
expect(id).toBe(mockedNoteId)
return new Promise((resolve, reject) => {
setTimeout(() => reject(new Error('CRAAAAASH')), 0)
})
})
}
const mockSetNoteInRedux = (expectedNote: Note): jest.SpyInstance<void, [apiResponse: Note]> => {
return jest.spyOn(setNoteDataFromServerModule, 'setNoteDataFromServer').mockImplementation((givenNote) => {
expect(givenNote).toBe(expectedNote)
})
}
it('loads a note', async () => {
const mockedNote: Note = Mock.of<Note>()
mockGetNoteApiCall(mockedNote)
const setNoteInReduxFunctionMock = mockSetNoteInRedux(mockedNote)
const view = render(
<NoteLoadingBoundary>
<span data-testid={'success'}>success!</span>
</NoteLoadingBoundary>
)
await screen.findByTestId('LoadingScreen')
await screen.findByTestId('success')
expect(view.container).toMatchSnapshot()
expect(setNoteInReduxFunctionMock).toBeCalledWith(mockedNote)
})
it('shows an error', async () => {
const mockedNote: Note = Mock.of<Note>()
mockCrashingNoteApiCall()
const setNoteInReduxFunctionMock = mockSetNoteInRedux(mockedNote)
const view = render(
<NoteLoadingBoundary>
<span data-testid={'success'}>success!</span>
</NoteLoadingBoundary>
)
await screen.findByTestId('LoadingScreen')
await screen.findByTestId('CommonErrorPage')
expect(view.container).toMatchSnapshot()
expect(setNoteInReduxFunctionMock).not.toBeCalled()
})
})

View file

@ -0,0 +1,35 @@
/*
* SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import type { PropsWithChildren } from 'react'
import React, { Fragment } from 'react'
import { useLoadNoteFromServer } from './hooks/use-load-note-from-server'
import { LoadingScreen } from '../../application-loader/loading-screen/loading-screen'
import { CommonErrorPage } from '../../error-pages/common-error-page'
/**
* Loads the note identified by the note-id in the URL.
* During the loading a {@link LoadingScreen loading screen} will be rendered instead of the child elements.
* The boundary also shows errors that occur during the loading process.
*
* @param children The react elements that will be shown when the loading was successful.
*/
export const NoteLoadingBoundary: React.FC<PropsWithChildren<unknown>> = ({ children }) => {
const { error, loading } = useLoadNoteFromServer()
if (loading) {
return <LoadingScreen />
} else if (error) {
return (
<CommonErrorPage
titleI18nKey={'noteLoadingBoundary.errorWhileLoadingContent'}
descriptionI18nKey={error.message}
/>
)
} else {
return <Fragment>{children}</Fragment>
}
}

View file

@ -1,27 +0,0 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import React from 'react'
import { Alert } from 'react-bootstrap'
import { Trans, useTranslation } from 'react-i18next'
import { ShowIf } from '../common/show-if/show-if'
import type { SimpleAlertProps } from '../common/simple-alert/simple-alert-props'
export const ErrorWhileLoadingNoteAlert: React.FC<SimpleAlertProps> = ({ show }) => {
useTranslation()
return (
<ShowIf condition={show}>
<Alert variant={'danger'} className={'my-2'}>
<b>
<Trans i18nKey={'views.readOnly.error.title'} />
</b>
<br />
<Trans i18nKey={'views.readOnly.error.description'} />
</Alert>
</ShowIf>
)
}

View file

@ -1,21 +0,0 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import React from 'react'
import { Alert } from 'react-bootstrap'
import { Trans } from 'react-i18next'
import { ShowIf } from '../common/show-if/show-if'
import type { SimpleAlertProps } from '../common/simple-alert/simple-alert-props'
export const LoadingNoteAlert: React.FC<SimpleAlertProps> = ({ show }) => {
return (
<ShowIf condition={show}>
<Alert variant={'info'} className={'my-2'}>
<Trans i18nKey={'views.readOnly.loading'} />
</Alert>
</ShowIf>
)
}

View file

@ -9,12 +9,8 @@ import { useTranslation } from 'react-i18next'
import { useApplyDarkMode } from '../../hooks/common/use-apply-dark-mode'
import { setCheckboxInMarkdownContent, updateNoteTitleByFirstHeading } from '../../redux/note-details/methods'
import { MotdModal } from '../common/motd-modal/motd-modal'
import { ShowIf } from '../common/show-if/show-if'
import { ErrorWhileLoadingNoteAlert } from '../document-read-only-page/ErrorWhileLoadingNoteAlert'
import { LoadingNoteAlert } from '../document-read-only-page/LoadingNoteAlert'
import { AppBar, AppBarMode } from './app-bar/app-bar'
import { EditorMode } from './app-bar/editor-view-mode'
import { useLoadNoteFromServer } from './hooks/useLoadNoteFromServer'
import { useViewModeShortcuts } from './hooks/useViewModeShortcuts'
import { Sidebar } from './sidebar/sidebar'
import { Splitter } from './splitter/splitter'
@ -31,10 +27,6 @@ import { NoteAndAppTitleHead } from '../layout/note-and-app-title-head'
import equal from 'fast-deep-equal'
import { EditorPane } from './editor-pane/editor-pane'
export interface EditorPagePathParams {
id: string
}
export enum ScrollSource {
EDITOR = 'editor',
RENDERER = 'renderer'
@ -94,9 +86,7 @@ export const EditorPageContent: React.FC = () => {
useApplyDarkMode()
useEditorModeFromUrl()
const [error, loading] = useLoadNoteFromServer()
useUpdateLocalHistoryEntry(!error && !loading)
useUpdateLocalHistoryEntry()
const setRendererToScrollSource = useCallback(() => {
if (scrollSource.current !== ScrollSource.RENDERER) {
@ -146,22 +136,16 @@ export const EditorPageContent: React.FC = () => {
<MotdModal />
<div className={'d-flex flex-column vh-100'}>
<AppBar mode={AppBarMode.EDITOR} />
<div className={'container'}>
<ErrorWhileLoadingNoteAlert show={error} />
<LoadingNoteAlert show={loading} />
<div className={'flex-fill d-flex h-100 w-100 overflow-hidden flex-row'}>
<Splitter
showLeft={editorMode === EditorMode.EDITOR || editorMode === EditorMode.BOTH}
left={leftPane}
showRight={editorMode === EditorMode.PREVIEW || editorMode === EditorMode.BOTH}
right={rightPane}
additionalContainerClassName={'overflow-hidden'}
/>
<Sidebar />
</div>
<ShowIf condition={!error && !loading}>
<div className={'flex-fill d-flex h-100 w-100 overflow-hidden flex-row'}>
<Splitter
showLeft={editorMode === EditorMode.EDITOR || editorMode === EditorMode.BOTH}
left={leftPane}
showRight={editorMode === EditorMode.PREVIEW || editorMode === EditorMode.BOTH}
right={rightPane}
additionalContainerClassName={'overflow-hidden'}
/>
<Sidebar />
</div>
</ShowIf>
</div>
</Fragment>
)

View file

@ -1,36 +0,0 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { getNote } from '../../../api/notes'
import { setNoteDataFromServer } from '../../../redux/note-details/methods'
import { Logger } from '../../../utils/logger'
import { useRouter } from 'next/router'
import { useAsync } from 'react-use'
const log = new Logger('Load Note From Server')
export const useLoadNoteFromServer = (): [boolean, boolean] => {
const { query } = useRouter()
const { loading, error } = useAsync(() => {
const rawId = query.id
if (rawId === undefined) {
return Promise.reject('invalid id')
}
const id = typeof rawId === 'string' ? rawId : rawId[0]
return getNote(id)
.then((note) => {
setNoteDataFromServer(note)
})
.catch((error: Error) => {
log.error('Error while fetching note from server', error)
return Promise.reject(error)
})
}, [query])
return [error !== undefined, loading]
}

View file

@ -12,7 +12,7 @@ import { useApplicationState } from '../../../hooks/common/use-application-state
import type { HistoryEntryWithOrigin } from '../../../api/history/types'
import { HistoryEntryOrigin } from '../../../api/history/types'
export const useUpdateLocalHistoryEntry = (updateReady: boolean): void => {
export const useUpdateLocalHistoryEntry = (): void => {
const id = useApplicationState((state) => state.noteDetails.id)
const userExists = useApplicationState((state) => !!state.user)
const currentNoteTitle = useApplicationState((state) => state.noteDetails.title)
@ -22,7 +22,7 @@ export const useUpdateLocalHistoryEntry = (updateReady: boolean): void => {
const lastNoteTags = useRef<string[]>([])
useEffect(() => {
if (!updateReady || userExists) {
if (userExists) {
return
}
if (currentNoteTitle === lastNoteTitle.current && equal(currentNoteTags, lastNoteTags.current)) {
@ -46,5 +46,5 @@ export const useUpdateLocalHistoryEntry = (updateReady: boolean): void => {
updateLocalHistoryEntry(id, entry)
lastNoteTitle.current = currentNoteTitle
lastNoteTags.current = currentNoteTags
}, [updateReady, id, userExists, currentNoteTitle, currentNoteTags])
}, [id, userExists, currentNoteTitle, currentNoteTags])
}

View file

@ -120,7 +120,7 @@ export abstract class WindowPostMessageCommunicator<
* @param handler The handler that processes messages with the given message type.
*/
public setHandler<R extends RECEIVE_TYPE>(messageType: R, handler: MaybeHandler<MESSAGES, R>): void {
this.log.debug('Set handler for', messageType)
this.log.debug(handler === undefined ? 'Unset' : 'Set', 'handler for', messageType)
this.handlers[messageType] = handler as MaybeHandler<MESSAGES, RECEIVE_TYPE>
}

View file

@ -6,20 +6,16 @@
import { useMemo } from 'react'
import { useApplicationState } from './use-application-state'
import equal from 'fast-deep-equal'
/**
* Returns the markdown content from the global application state trimmed to the maximal note length and without the frontmatter lines.
*/
export const useTrimmedNoteMarkdownContentWithoutFrontmatter = (): string[] => {
const maxLength = useApplicationState((state) => state.config.maxDocumentLength)
const markdownContent = useApplicationState(
(state) => ({
lines: state.noteDetails.markdownContent.lines,
content: state.noteDetails.markdownContent.plain
}),
equal
)
const markdownContent = useApplicationState((state) => ({
lines: state.noteDetails.markdownContent.lines,
content: state.noteDetails.markdownContent.plain
}))
const lineOffset = useApplicationState((state) => state.noteDetails.frontmatterRendererInfo.lineOffset)
const trimmedLines = useMemo(() => {

View file

@ -8,15 +8,18 @@ import React from 'react'
import { EditorToRendererCommunicatorContextProvider } from '../../components/editor-page/render-context/editor-to-renderer-communicator-context-provider'
import type { NextPage } from 'next'
import { EditorPageContent } from '../../components/editor-page/editor-page-content'
import { NoteLoadingBoundary } from '../../components/common/note-loading-boundary/note-loading-boundary'
/**
* Renders a page that is used by the user to edit markdown notes. It contains the editor and a renderer.
*/
export const EditorPage: NextPage = () => {
return (
<EditorToRendererCommunicatorContextProvider>
<EditorPageContent />
</EditorToRendererCommunicatorContextProvider>
<NoteLoadingBoundary>
<EditorToRendererCommunicatorContextProvider>
<EditorPageContent />
</EditorToRendererCommunicatorContextProvider>
</NoteLoadingBoundary>
)
}

View file

@ -4,25 +4,20 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
import React, { Fragment } from 'react'
import { useLoadNoteFromServer } from '../../components/editor-page/hooks/useLoadNoteFromServer'
import { ShowIf } from '../../components/common/show-if/show-if'
import React from 'react'
import { EditorToRendererCommunicatorContextProvider } from '../../components/editor-page/render-context/editor-to-renderer-communicator-context-provider'
import { SlideShowPageContent } from '../../components/slide-show-page/slide-show-page-content'
import { NoteAndAppTitleHead } from '../../components/layout/note-and-app-title-head'
import { NoteLoadingBoundary } from '../../components/common/note-loading-boundary/note-loading-boundary'
export const SlideShowPage: React.FC = () => {
const [error, loading] = useLoadNoteFromServer()
return (
<Fragment>
<NoteLoadingBoundary>
<NoteAndAppTitleHead />
<EditorToRendererCommunicatorContextProvider>
<ShowIf condition={!error && !loading}>
<SlideShowPageContent />
</ShowIf>
<SlideShowPageContent />
</EditorToRendererCommunicatorContextProvider>
</Fragment>
</NoteLoadingBoundary>
)
}

View file

@ -7,37 +7,29 @@
import React from 'react'
import { useApplyDarkMode } from '../../hooks/common/use-apply-dark-mode'
import { MotdModal } from '../../components/common/motd-modal/motd-modal'
import { ShowIf } from '../../components/common/show-if/show-if'
import { AppBar, AppBarMode } from '../../components/editor-page/app-bar/app-bar'
import { useLoadNoteFromServer } from '../../components/editor-page/hooks/useLoadNoteFromServer'
import { ErrorWhileLoadingNoteAlert } from '../../components/document-read-only-page/ErrorWhileLoadingNoteAlert'
import { LoadingNoteAlert } from '../../components/document-read-only-page/LoadingNoteAlert'
import { EditorToRendererCommunicatorContextProvider } from '../../components/editor-page/render-context/editor-to-renderer-communicator-context-provider'
import { UiNotifications } from '../../components/notifications/ui-notifications'
import { DocumentReadOnlyPageContent } from '../../components/document-read-only-page/document-read-only-page-content'
import { NoteAndAppTitleHead } from '../../components/layout/note-and-app-title-head'
import { NoteLoadingBoundary } from '../../components/common/note-loading-boundary/note-loading-boundary'
/**
* Renders a page that contains only the rendered document without an editor or realtime updates.
*/
export const DocumentReadOnlyPage: React.FC = () => {
useApplyDarkMode()
const [error, loading] = useLoadNoteFromServer()
return (
<EditorToRendererCommunicatorContextProvider>
<NoteAndAppTitleHead />
<UiNotifications />
<MotdModal />
<div className={'d-flex flex-column mvh-100 bg-light'}>
<AppBar mode={AppBarMode.BASIC} />
<div className={'container'}>
<ErrorWhileLoadingNoteAlert show={error} />
<LoadingNoteAlert show={loading} />
</div>
<ShowIf condition={!error && !loading}>
<NoteLoadingBoundary>
<NoteAndAppTitleHead />
<UiNotifications />
<MotdModal />
<div className={'d-flex flex-column mvh-100 bg-light'}>
<AppBar mode={AppBarMode.BASIC} />
<DocumentReadOnlyPageContent />
</ShowIf>
</div>
</div>
</NoteLoadingBoundary>
</EditorToRendererCommunicatorContextProvider>
)
}