From 1aeac5102aa3609ee1e8b55fb61f3ec0ba5f64ec Mon Sep 17 00:00:00 2001 From: Till Prochaska Date: Tue, 3 Jan 2023 21:25:15 +0100 Subject: [PATCH 1/3] Fix "in progress" and "error" states of `UpdateStatus` component This bug was likely introduced when upgrading to React Router 6. The `UpdateStatus` component needed access to the router's state in order to block navigation (and show a confirmation dialog to the user) when there are unsaved changes. With the upgrade to React Router 6, this required wrapping class components in a custom `withRouter` HOC. The `UpdateStatus` component exposes the different states (`SUCCESS`, `ERROR`, `IN_PROGRESS`) as static class fields (e.g. `UpdateStatus.IN_PROGRESS`). When wrapping the component in `withRouter`, those static class fields aren't forwarded, i.e. `UpdateStatus.IN_PROGRESS` is undefined. However, at the moment, blocking navigation isn't possible anyway (at least temporarily), as React Router v6 has removed support for relevant features required to implement this (although it will hopefully return soon [1]). So for now, I have simply removed `withRouter` altogether. Once blocking navigation is possible again, we should refactor this component to a function component and use the hooks provided by React Router. [1] https://github.com/remix-run/react-router/issues/8139 --- ui/src/components/common/UpdateStatus.jsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ui/src/components/common/UpdateStatus.jsx b/ui/src/components/common/UpdateStatus.jsx index cda6f4c6f2..2f07d12846 100644 --- a/ui/src/components/common/UpdateStatus.jsx +++ b/ui/src/components/common/UpdateStatus.jsx @@ -2,14 +2,10 @@ // future support is planned in future v6 releases // see https://reactrouter.com/docs/en/v6/upgrading/v5#prompt-is-not-currently-supported for reference -import React, { PureComponent } from 'react'; -import { compose } from 'redux'; +import { PureComponent } from 'react'; import { defineMessages, injectIntl } from 'react-intl'; -// import { Prompt } from 'react-router'; import { Intent, Spinner, Tag } from '@blueprintjs/core'; -import withRouter from 'app/withRouter'; - const messages = defineMessages({ status_success: { id: 'entity.status.success', @@ -100,4 +96,4 @@ class UpdateStatus extends PureComponent { } } -export default compose(withRouter, injectIntl)(UpdateStatus); +export default injectIntl(UpdateStatus); From 342c526cb7f884f996e2cda4f6b30ace700f355d Mon Sep 17 00:00:00 2001 From: Till Prochaska Date: Wed, 4 Jan 2023 16:19:53 +0100 Subject: [PATCH 2/3] Add tests for `UpdateStatus` and refactor to function component Doing this as preparation to also display the browser online status. --- ui/src/components/common/UpdateStatus.jsx | 99 ------------------- .../components/common/UpdateStatus.test.tsx | 22 +++++ ui/src/components/common/UpdateStatus.tsx | 57 +++++++++++ 3 files changed, 79 insertions(+), 99 deletions(-) delete mode 100644 ui/src/components/common/UpdateStatus.jsx create mode 100644 ui/src/components/common/UpdateStatus.test.tsx create mode 100644 ui/src/components/common/UpdateStatus.tsx diff --git a/ui/src/components/common/UpdateStatus.jsx b/ui/src/components/common/UpdateStatus.jsx deleted file mode 100644 index 2f07d12846..0000000000 --- a/ui/src/components/common/UpdateStatus.jsx +++ /dev/null @@ -1,99 +0,0 @@ -// Prompt component was temporarily unsupported as of react-router v6.2.1 -// future support is planned in future v6 releases -// see https://reactrouter.com/docs/en/v6/upgrading/v5#prompt-is-not-currently-supported for reference - -import { PureComponent } from 'react'; -import { defineMessages, injectIntl } from 'react-intl'; -import { Intent, Spinner, Tag } from '@blueprintjs/core'; - -const messages = defineMessages({ - status_success: { - id: 'entity.status.success', - defaultMessage: 'Saved', - }, - status_error: { - id: 'entity.status.error', - defaultMessage: 'Error saving', - }, - status_in_progress: { - id: 'entity.status.in_progress', - defaultMessage: 'Saving...', - }, - // error_warning: { - // id: 'entity.status.error_warning', - // defaultMessage: 'There was an error saving your latest changes, are you sure you want to leave?', - // }, - // in_progress_warning: { - // id: 'entity.status.in_progress_warning', - // defaultMessage: 'Changes are still being saved, are you sure you want to leave?', - // }, -}); - -class UpdateStatus extends PureComponent { - static SUCCESS = 'SUCCESS'; - static ERROR = 'ERROR'; - static IN_PROGRESS = 'IN_PROGRESS'; - - getStatusValue() { - switch (this.props.status) { - case 'IN_PROGRESS': - return { - text: messages.status_in_progress, - intent: Intent.PRIMARY, - icon: , - }; - case 'ERROR': - return { - text: messages.status_error, - intent: Intent.DANGER, - icon: 'error', - }; - default: - return { - text: messages.status_success, - intent: Intent.SUCCESS, - icon: 'tick', - }; - } - } - - // showPrompt(location) { - // const { pathname } = window.location; - // return location.pathname !== pathname - // } - - render() { - const { intl } = this.props; - const { text, intent, icon } = this.getStatusValue(); - - return ( - <> - {/* - { - if (this.showPrompt(location)) { - return intl.formatMessage(messages.in_progress_warning); - } - }} - /> - */} - {/* - { - if (this.showPrompt(location)) { - return intl.formatMessage(messages.error_warning); - } - }} - /> - */} - - {intl.formatMessage(text)} - - - ); - } -} - -export default injectIntl(UpdateStatus); diff --git a/ui/src/components/common/UpdateStatus.test.tsx b/ui/src/components/common/UpdateStatus.test.tsx new file mode 100644 index 0000000000..0c72860186 --- /dev/null +++ b/ui/src/components/common/UpdateStatus.test.tsx @@ -0,0 +1,22 @@ +import { render } from 'testUtils'; +import UpdateStatus from './UpdateStatus'; + +it('renders success by default', () => { + render(); + expect(document.body).toHaveTextContent('Saved'); +}); + +it('supports "success" status', () => { + render(); + expect(document.body).toHaveTextContent('Saved'); +}); + +it('supports "in progress" status', () => { + render(); + expect(document.body).toHaveTextContent('Saving'); +}); + +it('supports "error" status', () => { + render(); + expect(document.body).toHaveTextContent('Error saving'); +}); diff --git a/ui/src/components/common/UpdateStatus.tsx b/ui/src/components/common/UpdateStatus.tsx new file mode 100644 index 0000000000..a5e52dc13d --- /dev/null +++ b/ui/src/components/common/UpdateStatus.tsx @@ -0,0 +1,57 @@ +import { FormattedMessage } from 'react-intl'; +import { Intent, Spinner, Tag } from '@blueprintjs/core'; + +enum Status { + SUCCESS = 'SUCCESS', + ERROR = 'ERROR', + IN_PROGRESS = 'IN_PROGRESS', +} + +type UpdateStatusProps = { + status?: Status; +}; + +function UpdateStatus({ status }: UpdateStatusProps) { + const commonProps = { className: 'UpdateStatus', large: true, minimal: true }; + + if (status === Status.ERROR) { + return ( + + + + ); + } + + if (status === Status.IN_PROGRESS) { + return ( + } + {...commonProps} + > + + + ); + } + + // If no status or an unknown status is passed to the component, it will + // display "Saved". It would be better to be strict and always require a + // valid status. Keeping this behavior to prevent breaking API changes. + return ( + + + + ); +} + +UpdateStatus.SUCCESS = Status.SUCCESS; +UpdateStatus.ERROR = Status.ERROR; +UpdateStatus.IN_PROGRESS = Status.IN_PROGRESS; + +export default UpdateStatus; From 7836abf52ff12f90282f375e98122fe710809944 Mon Sep 17 00:00:00 2001 From: Till Prochaska Date: Wed, 4 Jan 2023 16:48:32 +0100 Subject: [PATCH 3/3] Warn users when their browser is offline --- .../components/common/UpdateStatus.test.tsx | 23 +++++++++++++++- ui/src/components/common/UpdateStatus.tsx | 27 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/ui/src/components/common/UpdateStatus.test.tsx b/ui/src/components/common/UpdateStatus.test.tsx index 0c72860186..239dcf8aee 100644 --- a/ui/src/components/common/UpdateStatus.test.tsx +++ b/ui/src/components/common/UpdateStatus.test.tsx @@ -1,4 +1,4 @@ -import { render } from 'testUtils'; +import { render, act } from 'testUtils'; import UpdateStatus from './UpdateStatus'; it('renders success by default', () => { @@ -20,3 +20,24 @@ it('supports "error" status', () => { render(); expect(document.body).toHaveTextContent('Error saving'); }); + +it('shows message when browser is offline', async () => { + render(); + expect(document.body).toHaveTextContent('Saved'); + + jest.spyOn(navigator, 'onLine', 'get').mockReturnValue(false); + + await act(async () => { + window.dispatchEvent(new Event('offline')); + }); + + expect(document.body).toHaveTextContent('Offline'); + + jest.spyOn(navigator, 'onLine', 'get').mockReturnValue(true); + + await act(async () => { + window.dispatchEvent(new Event('online')); + }); + + expect(document.body).toHaveTextContent('Saved'); +}); diff --git a/ui/src/components/common/UpdateStatus.tsx b/ui/src/components/common/UpdateStatus.tsx index a5e52dc13d..c6eb89251e 100644 --- a/ui/src/components/common/UpdateStatus.tsx +++ b/ui/src/components/common/UpdateStatus.tsx @@ -1,3 +1,4 @@ +import { useEffect, useState } from 'react'; import { FormattedMessage } from 'react-intl'; import { Intent, Spinner, Tag } from '@blueprintjs/core'; @@ -13,6 +14,15 @@ type UpdateStatusProps = { function UpdateStatus({ status }: UpdateStatusProps) { const commonProps = { className: 'UpdateStatus', large: true, minimal: true }; + const isOnline = useOnlineStatus(); + + if (!isOnline) { + return ( + + + + ); + } if (status === Status.ERROR) { return ( @@ -54,4 +64,21 @@ UpdateStatus.SUCCESS = Status.SUCCESS; UpdateStatus.ERROR = Status.ERROR; UpdateStatus.IN_PROGRESS = Status.IN_PROGRESS; +function useOnlineStatus() { + const [isOnline, setIsOnline] = useState(navigator.onLine); + const onChange = () => setIsOnline(navigator.onLine); + + useEffect(() => { + window.addEventListener('online', onChange); + window.addEventListener('offline', onChange); + + return () => { + window.removeEventListener('online', onChange); + window.removeEventListener('offline', onChange); + }; + }, []); + + return isOnline; +} + export default UpdateStatus;