From 59d2b73fad4c7ba13e5a44a098fe097110e590a8 Mon Sep 17 00:00:00 2001 From: HenryNguyen5 Date: Mon, 11 Jun 2018 18:43:39 -0400 Subject: [PATCH] Revamp Offline Status Checker (#1917) * Revamp app status to be event listener based * Update lockfile * Update snapshot * Show invalid only if .has-blurred * revert yarn.lock changes * Clean up input classes and types (#1925) * Show invalid for Nonce Field when empty (#1930) --- common/Root.tsx | 4 - common/actions/config/actionCreators.ts | 7 - common/actions/config/actionTypes.ts | 6 - common/actions/config/constants.ts | 2 +- common/components/NonceField.tsx | 1 + common/components/ui/Input.scss | 4 +- common/components/ui/Input.tsx | 35 +++-- common/sagas/config/node.ts | 130 ++++++++++-------- .../sass/styles/overrides/react-select.scss | 9 +- .../config/__snapshots__/config.spec.ts.snap | 2 +- spec/reducers/config/config.spec.ts | 86 +----------- 11 files changed, 106 insertions(+), 180 deletions(-) diff --git a/common/Root.tsx b/common/Root.tsx index e1d6fac3..fed42b3f 100644 --- a/common/Root.tsx +++ b/common/Root.tsx @@ -19,7 +19,6 @@ import OnboardModal from 'containers/OnboardModal'; import WelcomeModal from 'components/WelcomeModal'; import NewAppReleaseModal from 'components/NewAppReleaseModal'; import { Store } from 'redux'; -import { pollOfflineStatus, TPollOfflineStatus } from 'actions/config'; import { AppState } from 'reducers'; import { RouteNotFound } from 'components/RouteNotFound'; import { RedirectWithQuery } from 'components/RedirectWithQuery'; @@ -36,7 +35,6 @@ interface StateProps { } interface DispatchProps { - pollOfflineStatus: TPollOfflineStatus; setUnitMeta: TSetUnitMeta; } @@ -52,7 +50,6 @@ class RootClass extends Component { }; public componentDidMount() { - this.props.pollOfflineStatus(); this.props.setUnitMeta(this.props.networkUnit); this.addBodyClasses(); } @@ -190,6 +187,5 @@ const mapStateToProps = (state: AppState) => { }; export default connect(mapStateToProps, { - pollOfflineStatus, setUnitMeta })(RootClass); diff --git a/common/actions/config/actionCreators.ts b/common/actions/config/actionCreators.ts index c1e6c26a..2b8bf434 100644 --- a/common/actions/config/actionCreators.ts +++ b/common/actions/config/actionCreators.ts @@ -28,13 +28,6 @@ export function changeLanguage(sign: string): interfaces.ChangeLanguageAction { }; } -export type TPollOfflineStatus = typeof pollOfflineStatus; -export function pollOfflineStatus(): interfaces.PollOfflineStatus { - return { - type: TypeKeys.CONFIG_POLL_OFFLINE_STATUS - }; -} - export type TChangeNodeRequested = typeof changeNodeRequested; export function changeNodeRequested(payload: string): interfaces.ChangeNodeRequestedAction { return { diff --git a/common/actions/config/actionTypes.ts b/common/actions/config/actionTypes.ts index bd2c11a1..426a6321 100644 --- a/common/actions/config/actionTypes.ts +++ b/common/actions/config/actionTypes.ts @@ -20,11 +20,6 @@ export interface ChangeLanguageAction { payload: string; } -/*** Poll offline status ***/ -export interface PollOfflineStatus { - type: TypeKeys.CONFIG_POLL_OFFLINE_STATUS; -} - /*** Change Node Requested ***/ export interface ChangeNodeRequestedAction { type: TypeKeys.CONFIG_CHANGE_NODE_REQUESTED; @@ -120,7 +115,6 @@ export type MetaAction = | SetOnlineAction | SetOfflineAction | ToggleAutoGasLimitAction - | PollOfflineStatus | SetLatestBlockAction; /*** Union Type ***/ diff --git a/common/actions/config/constants.ts b/common/actions/config/constants.ts index 7a720c6c..66a4fb95 100644 --- a/common/actions/config/constants.ts +++ b/common/actions/config/constants.ts @@ -6,7 +6,7 @@ export enum TypeKeys { CONFIG_TOGGLE_OFFLINE = 'CONFIG_TOGGLE_OFFLINE', CONFIG_TOGGLE_AUTO_GAS_LIMIT = 'CONFIG_TOGGLE_AUTO_GAS_LIMIT', - CONFIG_POLL_OFFLINE_STATUS = 'CONFIG_POLL_OFFLINE_STATUS', + CONFIG_SET_LATEST_BLOCK = 'CONFIG_SET_LATEST_BLOCK', CONFIG_NODE_WEB3_SET = 'CONFIG_NODE_WEB3_SET', diff --git a/common/components/NonceField.tsx b/common/components/NonceField.tsx index ccffae9b..416304a8 100644 --- a/common/components/NonceField.tsx +++ b/common/components/NonceField.tsx @@ -50,6 +50,7 @@ class NonceField extends React.Component { readOnly={readOnly} onChange={onChange} disabled={noncePending} + showInvalidWithoutValue={true} /> {noncePending ? (
diff --git a/common/components/ui/Input.scss b/common/components/ui/Input.scss index 27c63028..b7a84038 100644 --- a/common/components/ui/Input.scss +++ b/common/components/ui/Input.scss @@ -81,11 +81,11 @@ color: $input-color-placeholder; } &:not([disabled]):not([readonly]) { - &.invalid.has-blurred.has-value { + &.invalid { border-color: $brand-danger; box-shadow: inset 0px 0px 0px 1px $brand-danger; } - &.valid.has-value { + &.valid { border-color: #8dd17b; box-shadow: inset 0px 0px 0px 1px #8dd17b; } diff --git a/common/components/ui/Input.tsx b/common/components/ui/Input.tsx index d4293ba1..34e31c0d 100644 --- a/common/components/ui/Input.tsx +++ b/common/components/ui/Input.tsx @@ -3,7 +3,10 @@ import classnames from 'classnames'; import './Input.scss'; interface OwnProps extends HTMLProps { + isValid?: boolean; showInvalidBeforeBlur?: boolean; + showInvalidWithoutValue?: boolean; + showValidAsPlain?: boolean; setInnerRef?(ref: HTMLInputElement | null): void; } @@ -16,12 +19,9 @@ interface State { isStateless: boolean; } -interface OwnProps extends HTMLProps { - isValid: boolean; - showValidAsPlain?: boolean; -} +type Props = OwnProps & HTMLProps; -class Input extends React.Component { +class Input extends React.Component { public state: State = { hasBlurred: false, isStateless: true @@ -31,18 +31,29 @@ class Input extends React.Component { const { setInnerRef, showInvalidBeforeBlur, + showInvalidWithoutValue, showValidAsPlain, isValid, ...htmlProps } = this.props; + const { hasBlurred, isStateless } = this.state; const hasValue = !!this.props.value && this.props.value.toString().length > 0; - const classname = classnames( - this.props.className, - 'input-group-input', - this.state.isStateless ? '' : isValid ? (showValidAsPlain ? '' : '') : `invalid`, - (showInvalidBeforeBlur || this.state.hasBlurred) && 'has-blurred', - hasValue && 'has-value' - ); + + // Currently we don't ever highlight valid, so go empty string instead + let validClass = isValid ? '' : 'invalid'; + if (isStateless) { + validClass = ''; + } + if (!hasValue && !showInvalidWithoutValue) { + validClass = ''; + } else if (!hasBlurred && !showInvalidBeforeBlur) { + validClass = ''; + } + if (!hasValue && showInvalidWithoutValue) { + validClass = 'invalid'; + } + + const classname = classnames('input-group-input', this.props.className, validClass); return ( { + const getShepherdStatus = () => ({ + pending: getShepherdPending(), + isOnline: !getShepherdOffline() + }); - const restoreNotif = showNotification( - 'success', - 'Your connection to the network has been restored!', - 3000 - ); - const lostNetworkNotif = showNotification( - 'danger', - `You’ve lost your connection to the network, check your internet - connection or try changing networks from the dropdown at the - top right of the page.`, - Infinity - ); - const offlineNotif = showNotification( - 'info', - 'You are currently offline. Some features will be unavailable.', - 5000 + const { online, offline, lostNetworkNotif, offlineNotif, restoreNotif } = bindActionCreators( + { + offline: setOffline, + online: setOnline, + restoreNotif: () => + showNotification('success', 'Your connection to the network has been restored!', 3000), + lostNetworkNotif: () => + showNotification( + 'danger', + `You’ve lost your connection to the network, check your internet +connection or try changing networks from the dropdown at the +top right of the page.`, + Infinity + ), + + offlineNotif: () => + showNotification( + 'info', + 'You are currently offline. Some features will be unavailable.', + 5000 + ) + }, + store.dispatch ); - while (true) { - yield call(delay, 2500); + const getAppOnline = () => !getOffline(store.getState()); - const pending: ReturnType = yield call(getShepherdPending); - if (pending) { - continue; + /** + * @description Repeatedly polls itself to check for online state conflict occurs, implemented in recursive style for flexible polling times + * as network requests take a variable amount of time. + * + * Whenever an app online state conflict occurs, it resolves the conflict with the following priority: + * * If shepherd is online but app is offline -> do a ping request via shepherd provider, with the result of the ping being the set app state + * * If shepherd is offline but app is online -> set app to offline as it wont be able to make requests anyway + */ + async function detectOnlineStateConflict() { + const shepherdStatus = getShepherdStatus(); + const appOffline = getAppOnline(); + const onlineStateConflict = shepherdStatus.isOnline !== appOffline; + + if (shepherdStatus.pending || !onlineStateConflict) { + return setTimeout(detectOnlineStateConflict, 1000); } - const isOffline: boolean = yield select(getOffline); - const balancerOffline = yield call(getShepherdOffline); - - if (!balancerOffline && isOffline) { - // If we were able to ping but redux says we're offline, mark online - yield put(restoreNotif); - yield put(setOnline()); - } else if (balancerOffline && !isOffline) { - // If we were unable to ping but redux says we're online, mark offline - // If they had been online, show an error. - // If they hadn't been online, just inform them with a warning. - yield put(setOffline()); - if (hasCheckedOnline) { - yield put(lostNetworkNotif); - } else { - yield put(offlineNotif); + // if app reports online but shepherd offline, then set app offline + if (appOffline && !shepherdStatus.isOnline) { + lostNetworkNotif(); + offline(); + } else if (!appOffline && shepherdStatus.isOnline) { + // if app reports offline but shepherd reports online + // send a request to shepherd provider to see if we can still send out requests + const success = await shepherdProvider.ping().catch(() => false); + if (success) { + restoreNotif(); + online(); } } - hasCheckedOnline = true; + detectOnlineStateConflict(); } -} + detectOnlineStateConflict(); -// Fork our recurring API call, watch for the need to cancel. -export function* handlePollOfflineStatus(): SagaIterator { - const pollOfflineStatusTask = yield fork(pollOfflineStatus); - yield take('CONFIG_STOP_POLL_OFFLINE_STATE'); - yield cancel(pollOfflineStatusTask); -} + window.addEventListener('offline', () => { + const previouslyOnline = getAppOnline(); + + // if browser reports as offline and we were previously online + // then set offline without checking balancer state + if (!navigator.onLine && previouslyOnline) { + offlineNotif(); + offline(); + } + }); +}); // @HACK For now we reload the app when doing a language swap to force non-connected // data to reload. Also the use of timeout to avoid using additional actions for now. @@ -283,7 +296,6 @@ export const node = [ takeEvery(TypeKeys.CONFIG_CHANGE_NODE_REQUESTED, handleChangeNodeRequested), takeEvery(TypeKeys.CONFIG_CHANGE_NODE_FORCE, handleNodeChangeForce), takeEvery(TypeKeys.CONFIG_CHANGE_NETWORK_REQUESTED, handleChangeNetworkRequested), - takeLatest(TypeKeys.CONFIG_POLL_OFFLINE_STATUS, handlePollOfflineStatus), takeEvery(TypeKeys.CONFIG_LANGUAGE_CHANGE, reload), takeEvery(TypeKeys.CONFIG_ADD_CUSTOM_NODE, handleAddCustomNode), takeEvery(TypeKeys.CONFIG_REMOVE_CUSTOM_NODE, handleRemoveCustomNode) diff --git a/common/sass/styles/overrides/react-select.scss b/common/sass/styles/overrides/react-select.scss index 91278fbd..e83025b2 100644 --- a/common/sass/styles/overrides/react-select.scss +++ b/common/sass/styles/overrides/react-select.scss @@ -65,10 +65,11 @@ &-menu { max-height: 10.0625rem; } - &.invalid.has-blurred { - border-color: $brand-danger; - box-shadow: inset 0px 0px 0px 1px $brand-danger; - } + // Selects should never have invalid input + // &.invalid { + // border-color: $brand-danger; + // box-shadow: inset 0px 0px 0px 1px $brand-danger; + // } &.is-focused { border-color: #4295bc; box-shadow: inset 0px 0px 0px 1px #4295bc; diff --git a/spec/reducers/config/__snapshots__/config.spec.ts.snap b/spec/reducers/config/__snapshots__/config.spec.ts.snap index 5ddb4aa6..d649e139 100644 --- a/spec/reducers/config/__snapshots__/config.spec.ts.snap +++ b/spec/reducers/config/__snapshots__/config.spec.ts.snap @@ -5,7 +5,7 @@ Object { "@@redux-saga/IO": true, "SELECT": Object { "args": Array [ - "ELLA", + "CLO", ], "selector": [Function], }, diff --git a/spec/reducers/config/config.spec.ts b/spec/reducers/config/config.spec.ts index eba96681..03691223 100644 --- a/spec/reducers/config/config.spec.ts +++ b/spec/reducers/config/config.spec.ts @@ -1,10 +1,8 @@ import { configuredStore } from 'store'; import { delay, SagaIterator } from 'redux-saga'; -import { call, cancel, fork, put, take, select, apply } from 'redux-saga/effects'; -import { cloneableGenerator, createMockTask } from 'redux-saga/utils'; +import { call, fork, put, take, select, apply } from 'redux-saga/effects'; +import { cloneableGenerator } from 'redux-saga/utils'; import { - setOffline, - setOnline, changeNodeSucceeded, changeNodeRequested, changeNodeFailed, @@ -16,8 +14,6 @@ import { } from 'actions/config'; import { handleChangeNodeRequested, - handlePollOfflineStatus, - pollOfflineStatus, handleNewNetwork, handleChangeNodeRequestedOneTime } from 'sagas/config/node'; @@ -39,88 +35,10 @@ import { selectedNodeExpectedState } from './nodes/selectedNode.spec'; import { customNodesExpectedState, firstCustomNode } from './nodes/customNodes.spec'; import { unsetWeb3Node, unsetWeb3NodeOnWalletEvent } from 'sagas/config/web3'; import { shepherd } from 'mycrypto-shepherd'; -import { getShepherdOffline, getShepherdPending } from 'libs/nodes'; // init module configuredStore.getState(); -describe('pollOfflineStatus*', () => { - const restoreNotif = 'Your connection to the network has been restored!'; - - const lostNetworkNotif = `You’ve lost your connection to the network, check your internet - connection or try changing networks from the dropdown at the - top right of the page.`; - - const offlineNotif = 'You are currently offline. Some features will be unavailable.'; - - const offlineOnFirstTimeCase = pollOfflineStatus(); - it('should delay by 2.5 seconds', () => { - expect(offlineOnFirstTimeCase.next().value).toEqual(call(delay, 2500)); - }); - - it('should skip if a node change is pending', () => { - expect(offlineOnFirstTimeCase.next().value).toEqual(call(getShepherdPending)); - expect(offlineOnFirstTimeCase.next(true).value).toEqual(call(delay, 2500)); - expect(offlineOnFirstTimeCase.next().value).toEqual(call(getShepherdPending)); - }); - - it('should select offline', () => { - expect(offlineOnFirstTimeCase.next(false).value).toEqual(select(getOffline)); - }); - - it('should select shepherd"s offline', () => { - expect(offlineOnFirstTimeCase.next(false).value).toEqual(call(getShepherdOffline)); - }); - - // .PUT.action.payload.msg is used because the action creator uses an random ID, cant to a showNotif comparision - it('should put a different notif if online for the first time ', () => { - expect(offlineOnFirstTimeCase.next(true).value).toEqual(put(setOffline())); - expect((offlineOnFirstTimeCase.next().value as any).PUT.action.payload.msg).toEqual( - offlineNotif - ); - }); - - it('should loop around then go back online, putting a restore msg', () => { - expect(offlineOnFirstTimeCase.next().value).toEqual(call(delay, 2500)); - expect(offlineOnFirstTimeCase.next().value).toEqual(call(getShepherdPending)); - expect(offlineOnFirstTimeCase.next(false).value).toEqual(select(getOffline)); - expect(offlineOnFirstTimeCase.next(true).value).toEqual(call(getShepherdOffline)); - expect((offlineOnFirstTimeCase.next().value as any).PUT.action.payload.msg).toEqual( - restoreNotif - ); - expect(offlineOnFirstTimeCase.next(false).value).toEqual(put(setOnline())); - }); - - it('should put a generic lost connection notif on every time afterwards', () => { - expect(offlineOnFirstTimeCase.next().value).toEqual(call(delay, 2500)); - expect(offlineOnFirstTimeCase.next().value).toEqual(call(getShepherdPending)); - expect(offlineOnFirstTimeCase.next(false).value).toEqual(select(getOffline)); - expect(offlineOnFirstTimeCase.next(false).value).toEqual(call(getShepherdOffline)); - expect(offlineOnFirstTimeCase.next(true).value).toEqual(put(setOffline())); - expect((offlineOnFirstTimeCase.next().value as any).PUT.action.payload.msg).toEqual( - lostNetworkNotif - ); - }); -}); - -describe('handlePollOfflineStatus*', () => { - const gen = handlePollOfflineStatus(); - const mockTask = createMockTask(); - - it('should fork pollOffineStatus', () => { - const expectedForkYield = fork(pollOfflineStatus); - expect(gen.next().value).toEqual(expectedForkYield); - }); - - it('should take CONFIG_STOP_POLL_OFFLINE_STATE', () => { - expect(gen.next(mockTask).value).toEqual(take('CONFIG_STOP_POLL_OFFLINE_STATE')); - }); - - it('should cancel pollOfflineStatus', () => { - expect(gen.next().value).toEqual(cancel(mockTask)); - }); -}); - describe('handleChangeNodeRequested*', () => { let originalRandom: any;