From 5f4c07eb7023cf50879d16a6a4ee9add5e3d5e71 Mon Sep 17 00:00:00 2001 From: mmso Date: Mon, 23 Dec 2019 21:01:45 +0100 Subject: [PATCH 01/12] Offline notification --- containers/api/ApiProvider.js | 45 +++++++++++++++++++++++++-- containers/api/OfflineNotification.js | 18 +++++++++++ containers/app/ProtonApp.js | 8 ++--- containers/notifications/Container.js | 4 +-- containers/notifications/manager.js | 12 +++++-- 5 files changed, 75 insertions(+), 12 deletions(-) create mode 100644 containers/api/OfflineNotification.js diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index 64bdfda53..4953d3484 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -1,6 +1,7 @@ import React, { useRef } from 'react'; import PropTypes from 'prop-types'; import xhr from 'proton-shared/lib/fetch/fetch'; +import { getUser } from 'proton-shared/lib/api/user'; import configureApi from 'proton-shared/lib/api'; import withApiHandlers, { CancelUnlockError, @@ -8,6 +9,7 @@ import withApiHandlers, { } from 'proton-shared/lib/api/helpers/withApiHandlers'; import { getError } from 'proton-shared/lib/apiHandlers'; import { getDateHeader } from 'proton-shared/lib/fetch/helpers'; +import { API_CUSTOM_ERROR_CODES } from 'proton-shared/lib/errors'; import { updateServerTime } from 'pmcrypto'; import { c } from 'ttag'; @@ -16,6 +18,7 @@ import useNotifications from '../notifications/useNotifications'; import useModals from '../modals/useModals'; import UnlockModal from '../login/UnlockModal'; import HumanVerificationModal from './HumanVerificationModal'; +import OfflineNotification from './OfflineNotification'; const getSilenced = ({ silence } = {}, code) => { if (Array.isArray(silence)) { @@ -25,29 +28,58 @@ const getSilenced = ({ silence } = {}, code) => { }; const ApiProvider = ({ config, onLogout, children, UID }) => { - const { createNotification } = useNotifications(); + const { createNotification, hideNotification } = useNotifications(); const { createModal } = useModals(); const apiRef = useRef(); + const offlineRef = useRef(); + const appVersionBad = useRef(); if (!apiRef.current) { const handleError = (e) => { const { message, code } = getError(e); + if (appVersionBad.current) { + return; + } + + if (code === API_CUSTOM_ERROR_CODES.APP_VERSION_BAD) { + appVersionBad.current = true; + // The only way to get out of this one is to refresh. + createNotification({ + type: 'error', + text: message || c('Info').t`Application upgrade required`, + expiration: -1 + }); + throw e; + } + if (e.name === 'InactiveSession') { onLogout(); throw e; } + // If the client knows it's offline, just ignore any other error + if (offlineRef.current) { + throw e; + } + if (e.name === 'CancelUnlock') { throw e; } if (e.name === 'OfflineError') { + /* const text = navigator.onLine ? c('Error').t`Could not connect to server.` : c('Error').t`No internet connection found`; - const isSilenced = getSilenced(e.config, code); - !isSilenced && createNotification({ type: 'error', text }); + */ + const id = createNotification({ + type: 'warning', + text: apiRef.current(getUser())} />, + expiration: -1, + disableAutoClose: true + }); + offlineRef.current = { id }; throw e; } @@ -99,11 +131,18 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { }); apiRef.current = ({ output = 'json', ...rest }) => { + if (appVersionBad.current) { + return Promise.reject(new Error(c('Error').t`Bad app version`)); + } return callWithApiHandlers(rest).then((response) => { const serverTime = getDateHeader(response.headers); if (serverTime) { updateServerTime(serverTime); } + if (offlineRef.current) { + hideNotification(offlineRef.current.id); + offlineRef.current = undefined; + } return output === 'stream' ? response.body : response[output](); }); }; diff --git a/containers/api/OfflineNotification.js b/containers/api/OfflineNotification.js new file mode 100644 index 000000000..684a459f7 --- /dev/null +++ b/containers/api/OfflineNotification.js @@ -0,0 +1,18 @@ +import { c } from 'ttag'; +import React from 'react'; +import { LinkButton, useLoading } from 'react-components'; + +const OfflineNotification = ({ onRetry }) => { + const [loading, withLoading] = useLoading(); + const retryNow = ( + withLoading(onRetry())}>{c('Action') + .t`Retry now`} + ); + return ( + <> + {c('Error').jt`Servers are unreachable.`} {retryNow} + + ); +}; + +export default OfflineNotification; diff --git a/containers/app/ProtonApp.js b/containers/app/ProtonApp.js index 0e0d3bd73..86fedb0b3 100644 --- a/containers/app/ProtonApp.js +++ b/containers/app/ProtonApp.js @@ -87,8 +87,8 @@ const ProtonApp = ({ config, children }) => { - - + + @@ -96,8 +96,8 @@ const ProtonApp = ({ config, children }) => { - - + + diff --git a/containers/notifications/Container.js b/containers/notifications/Container.js index 075600fd7..4ea21d2ec 100644 --- a/containers/notifications/Container.js +++ b/containers/notifications/Container.js @@ -4,13 +4,13 @@ import PropTypes from 'prop-types'; import Notification from './Notification'; const NotificationsContainer = ({ notifications, removeNotification, hideNotification }) => { - const list = notifications.map(({ id, type, text, isClosing }) => { + const list = notifications.map(({ id, type, text, isClosing, disableAutoClose }) => { return ( hideNotification(id)} + onClick={disableAutoClose ? undefined : () => hideNotification(id)} onExit={() => removeNotification(id)} > {text} diff --git a/containers/notifications/manager.js b/containers/notifications/manager.js index d41bd8891..b9dce474a 100644 --- a/containers/notifications/manager.js +++ b/containers/notifications/manager.js @@ -40,9 +40,11 @@ export default (setNotifications) => { idx = 0; } - intervalIds[id] = expiration === -1 ? -1 : setTimeout(() => hideNotification(id), expiration); - - return setNotifications((oldNotifications) => { + setNotifications((oldNotifications) => { + // Deduplicate notifications + if (oldNotifications.some(({ text }) => text === rest.text)) { + return oldNotifications; + } return [ ...oldNotifications, { @@ -54,6 +56,10 @@ export default (setNotifications) => { } ]; }); + + intervalIds[id] = expiration === -1 ? -1 : setTimeout(() => hideNotification(id), expiration); + + return id; }; const clearNotifications = () => { From 45d57449233a32922c97330e642f61fc5729bf7f Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 26 Dec 2019 21:26:52 +0100 Subject: [PATCH 02/12] Improve animation rest --- containers/notifications/manager.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/containers/notifications/manager.js b/containers/notifications/manager.js index b9dce474a..fc0fad381 100644 --- a/containers/notifications/manager.js +++ b/containers/notifications/manager.js @@ -57,7 +57,18 @@ export default (setNotifications) => { ]; }); - intervalIds[id] = expiration === -1 ? -1 : setTimeout(() => hideNotification(id), expiration); + intervalIds[id] = + expiration === -1 + ? -1 + : setTimeout(() => { + // If the page is hidden, don't hide the notification with an animation because they get stacked. + // This is to solve e.g. offline notifications appearing when the page is hidden, and when you focus + // the tab again, they would be visible for the animation out even if they happened a while ago. + if (document.hidden) { + return removeNotification(id); + } + hideNotification(id); + }, expiration); return id; }; From 0623737395f79a272ef88bceaaf88141655231ad Mon Sep 17 00:00:00 2001 From: mmso Date: Fri, 3 Jan 2020 17:59:33 +0100 Subject: [PATCH 03/12] Change hide behavior --- containers/api/OfflineNotification.js | 12 +++++++++--- containers/notifications/manager.js | 19 +++++++------------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/containers/api/OfflineNotification.js b/containers/api/OfflineNotification.js index 684a459f7..32ebf5764 100644 --- a/containers/api/OfflineNotification.js +++ b/containers/api/OfflineNotification.js @@ -1,18 +1,24 @@ import { c } from 'ttag'; import React from 'react'; import { LinkButton, useLoading } from 'react-components'; +import PropTypes from 'prop-types'; const OfflineNotification = ({ onRetry }) => { const [loading, withLoading] = useLoading(); const retryNow = ( - withLoading(onRetry())}>{c('Action') - .t`Retry now`} + withLoading(onRetry())}> + {c('Action').t`Retry now`} + ); return ( <> - {c('Error').jt`Servers are unreachable.`} {retryNow} + {c('Error').t`Servers are unreachable.`} {retryNow} ); }; +OfflineNotification.propTypes = { + onRetry: PropTypes.func +}; + export default OfflineNotification; diff --git a/containers/notifications/manager.js b/containers/notifications/manager.js index fc0fad381..38f6ddda0 100644 --- a/containers/notifications/manager.js +++ b/containers/notifications/manager.js @@ -19,6 +19,12 @@ export default (setNotifications) => { }; const hideNotification = (id) => { + // If the page is hidden, don't hide the notification with an animation because they get stacked. + // This is to solve e.g. offline notifications appearing when the page is hidden, and when you focus + // the tab again, they would be visible for the animation out even if they happened a while ago. + if (document.hidden) { + return removeNotification(id); + } return setNotifications((oldNotifications) => { return oldNotifications.map((oldNotification) => { if (oldNotification.id !== id) { @@ -57,18 +63,7 @@ export default (setNotifications) => { ]; }); - intervalIds[id] = - expiration === -1 - ? -1 - : setTimeout(() => { - // If the page is hidden, don't hide the notification with an animation because they get stacked. - // This is to solve e.g. offline notifications appearing when the page is hidden, and when you focus - // the tab again, they would be visible for the animation out even if they happened a while ago. - if (document.hidden) { - return removeNotification(id); - } - hideNotification(id); - }, expiration); + intervalIds[id] = expiration === -1 ? -1 : setTimeout(() => hideNotification(id), expiration); return id; }; From 7942cae231c858ea14a0d77fbf71cee13bc260ac Mon Sep 17 00:00:00 2001 From: mmso Date: Fri, 3 Jan 2020 18:19:49 +0100 Subject: [PATCH 04/12] Continue --- containers/api/ApiProvider.js | 39 ++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index 4953d3484..3136948b8 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -75,7 +75,14 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { */ const id = createNotification({ type: 'warning', - text: apiRef.current(getUser())} />, + text: ( + { + // TODO: What route when not logged in? + apiRef.current(getUser()); + }} + /> + ), expiration: -1, disableAutoClose: true }); @@ -134,17 +141,25 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { if (appVersionBad.current) { return Promise.reject(new Error(c('Error').t`Bad app version`)); } - return callWithApiHandlers(rest).then((response) => { - const serverTime = getDateHeader(response.headers); - if (serverTime) { - updateServerTime(serverTime); - } - if (offlineRef.current) { - hideNotification(offlineRef.current.id); - offlineRef.current = undefined; - } - return output === 'stream' ? response.body : response[output](); - }); + return callWithApiHandlers(rest) + .then((response) => { + const serverTime = getDateHeader(response.headers); + if (serverTime) { + updateServerTime(serverTime); + } + if (offlineRef.current) { + hideNotification(offlineRef.current.id); + offlineRef.current = undefined; + } + return output === 'stream' ? response.body : response[output](); + }) + .catch((e) => { + if (e.name !== 'OfflineError' && offlineRef.current) { + hideNotification(offlineRef.current.id); + offlineRef.current = undefined; + } + throw e; + }); }; } From f550938ff55539643a53d51fbb16e18e5d9c22dc Mon Sep 17 00:00:00 2001 From: mmso Date: Fri, 3 Jan 2020 18:27:34 +0100 Subject: [PATCH 05/12] Continue --- containers/api/ApiProvider.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index 3136948b8..d51bf8cb6 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -48,7 +48,8 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { createNotification({ type: 'error', text: message || c('Info').t`Application upgrade required`, - expiration: -1 + expiration: -1, + disableAutoClose: true }); throw e; } From 75cc3acd69f4609bb54a4a8dff951fede83cd1c0 Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 10:44:21 +0100 Subject: [PATCH 06/12] Fix ping in unauthenticated --- containers/api/ApiProvider.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index d51bf8cb6..41bbc89af 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -2,6 +2,7 @@ import React, { useRef } from 'react'; import PropTypes from 'prop-types'; import xhr from 'proton-shared/lib/fetch/fetch'; import { getUser } from 'proton-shared/lib/api/user'; +import { ping } from 'proton-shared/lib/api/tests'; import configureApi from 'proton-shared/lib/api'; import withApiHandlers, { CancelUnlockError, @@ -69,18 +70,16 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { } if (e.name === 'OfflineError') { - /* - const text = navigator.onLine - ? c('Error').t`Could not connect to server.` - : c('Error').t`No internet connection found`; - */ const id = createNotification({ type: 'warning', text: ( { - // TODO: What route when not logged in? - apiRef.current(getUser()); + hideNotification(id); + offlineRef.current = undefined; + // If there is a session, get user to validate it's still active after coming back online + // otherwise if signed out, call ping + apiRef.current(UID ? getUser() : ping()); }} /> ), From 66fb8fa54932cb3c806668ba9143c1f6ffc4919d Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 11:04:54 +0100 Subject: [PATCH 07/12] Add hide offline --- containers/api/ApiProvider.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index 41bbc89af..7b9562b23 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -35,6 +35,14 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { const offlineRef = useRef(); const appVersionBad = useRef(); + const hideOfflineNotification = () => { + if (!offlineRef.current) { + return; + } + hideNotification(offlineRef.current.id); + offlineRef.current = undefined; + }; + if (!apiRef.current) { const handleError = (e) => { const { message, code } = getError(e); @@ -75,8 +83,7 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { text: ( { - hideNotification(id); - offlineRef.current = undefined; + hideOfflineNotification(); // If there is a session, get user to validate it's still active after coming back online // otherwise if signed out, call ping apiRef.current(UID ? getUser() : ping()); @@ -147,16 +154,12 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { if (serverTime) { updateServerTime(serverTime); } - if (offlineRef.current) { - hideNotification(offlineRef.current.id); - offlineRef.current = undefined; - } + hideOfflineNotification(); return output === 'stream' ? response.body : response[output](); }) .catch((e) => { - if (e.name !== 'OfflineError' && offlineRef.current) { - hideNotification(offlineRef.current.id); - offlineRef.current = undefined; + if (e.name !== 'OfflineError' && e.name !== 'TimeoutError') { + hideOfflineNotification(); } throw e; }); From b7cb140a976d223095128b28b0fa45b77a8d8298 Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 11:06:07 +0100 Subject: [PATCH 08/12] Update comment --- containers/api/ApiProvider.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index 7b9562b23..e05933cee 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -85,7 +85,7 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { onRetry={() => { hideOfflineNotification(); // If there is a session, get user to validate it's still active after coming back online - // otherwise if signed out, call ping + // otherwise if it's not logged in, call ping apiRef.current(UID ? getUser() : ping()); }} /> From 26fd3fec7b748bb45f1e7b4668f0e8ed4eb4df1d Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 11:29:32 +0100 Subject: [PATCH 09/12] Change hiding notification on error --- containers/api/ApiProvider.js | 46 ++++++++++++++++------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/containers/api/ApiProvider.js b/containers/api/ApiProvider.js index e05933cee..332493152 100644 --- a/containers/api/ApiProvider.js +++ b/containers/api/ApiProvider.js @@ -48,7 +48,19 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { const { message, code } = getError(e); if (appVersionBad.current) { - return; + throw e; + } + + if (e.name === 'CancelUnlock') { + throw e; + } + + // If the client knows it's offline and it's another offline error, just ignore it + if (offlineRef.current && e.name === 'OfflineError') { + throw e; + } + if (offlineRef.current && e.name !== 'OfflineError') { + hideOfflineNotification(); } if (code === API_CUSTOM_ERROR_CODES.APP_VERSION_BAD) { @@ -68,15 +80,6 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { throw e; } - // If the client knows it's offline, just ignore any other error - if (offlineRef.current) { - throw e; - } - - if (e.name === 'CancelUnlock') { - throw e; - } - if (e.name === 'OfflineError') { const id = createNotification({ type: 'warning', @@ -148,21 +151,14 @@ const ApiProvider = ({ config, onLogout, children, UID }) => { if (appVersionBad.current) { return Promise.reject(new Error(c('Error').t`Bad app version`)); } - return callWithApiHandlers(rest) - .then((response) => { - const serverTime = getDateHeader(response.headers); - if (serverTime) { - updateServerTime(serverTime); - } - hideOfflineNotification(); - return output === 'stream' ? response.body : response[output](); - }) - .catch((e) => { - if (e.name !== 'OfflineError' && e.name !== 'TimeoutError') { - hideOfflineNotification(); - } - throw e; - }); + return callWithApiHandlers(rest).then((response) => { + const serverTime = getDateHeader(response.headers); + if (serverTime) { + updateServerTime(serverTime); + } + hideOfflineNotification(); + return output === 'stream' ? response.body : response[output](); + }); }; } From 07514326d417bfc9aa5f97de65aefd5b75aab980 Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 11:33:12 +0100 Subject: [PATCH 10/12] Revert order --- containers/app/ProtonApp.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/containers/app/ProtonApp.js b/containers/app/ProtonApp.js index 86fedb0b3..0e0d3bd73 100644 --- a/containers/app/ProtonApp.js +++ b/containers/app/ProtonApp.js @@ -87,8 +87,8 @@ const ProtonApp = ({ config, children }) => { - - + + @@ -96,8 +96,8 @@ const ProtonApp = ({ config, children }) => { - - + + From a28d0da410310f9b8f08adc632850b048eb858ad Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 11:35:47 +0100 Subject: [PATCH 11/12] Revert deduplication --- containers/notifications/manager.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/containers/notifications/manager.js b/containers/notifications/manager.js index 38f6ddda0..79f245472 100644 --- a/containers/notifications/manager.js +++ b/containers/notifications/manager.js @@ -47,10 +47,6 @@ export default (setNotifications) => { } setNotifications((oldNotifications) => { - // Deduplicate notifications - if (oldNotifications.some(({ text }) => text === rest.text)) { - return oldNotifications; - } return [ ...oldNotifications, { From c565e7f81c1339236da616ad4b40e77c5f718ba2 Mon Sep 17 00:00:00 2001 From: mmso Date: Thu, 23 Jan 2020 13:05:01 +0100 Subject: [PATCH 12/12] Update readme --- README.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 60ceeed2a..776321514 100644 --- a/README.md +++ b/README.md @@ -71,11 +71,22 @@ const { UID, login, logout, ...} = useAuthentication(); Create notifications to be displayed in the app. ``` js -const { createNotification, removeNotification } = useNotifications(); +const { createNotification, hideNotification } = useNotifications(); const handleClick = () => { createNotification({ type: 'error', text: 'Failed to update' }); } + +const handleClickPersistent = () => { + const id = createNotification({ + expiration: -1, // does not expire + type: 'error', + text: 'Failed to update' + }); + setTimeout(() => { + hideNotification(id); + }, 1000); +} ``` ### useModals