forked from zulip/zulip-mobile
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
presence: replace all presence-reporting logic
Replace the existing presence-reporting logic with HeartbeatComponent, instantiated as part of a new zero-display React component located under AppEventHandlers. In particular, strip the throttling logic from `reportPresence`. This fixes at least three issues: * If the user logged in and out, or otherwise performed an additional "initial fetch" without killing the app, the presence timer would be started multiple times. (The negative effects of this were previously concealed by the throttling logic.) * `idle` events were not treated differently from `active` events by the throttling logic, and would almost invariably be swallowed. * If the user went idle (e.g. by hitting the Home button), the presence timer would still run, and would still attempt to send `active` notifications. (These attempts were often suppressed while the app was in the background, but this was never guaranteed.) Fixes (the original version of) zulip#3699, and is work towards zulip#3659. Unfortunately, there is no user-visible change yet: modifications will be needed on the Zulip server side as well.
- Loading branch information
1 parent
c59f2ad
commit 158fff1
Showing
4 changed files
with
80 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
// @flow strict-local | ||
import { PureComponent } from 'react'; | ||
import { AppState } from 'react-native'; | ||
import type { Auth, Dispatch } from '../types'; | ||
|
||
import { connect } from '../react-redux'; | ||
import { tryGetAuth } from '../account/accountsSelectors'; | ||
import { reportPresence } from '../actions'; | ||
import Heartbeat from './heartbeat'; | ||
|
||
type Props = $ReadOnly<{| | ||
dispatch: Dispatch, | ||
auth: Auth | void, | ||
|}>; | ||
|
||
/** | ||
* Component providing a recurrent `presence` signal. | ||
*/ | ||
// The name "PureComponent" is potentially misleading here, as this component is | ||
// in no reasonable sense "pure" -- its entire purpose is to emit network calls | ||
// for their observable side effects as a side effect of being rendered. | ||
// | ||
// (This is merely a misnomer on React's part, rather than a functional error. A | ||
// `PureComponent` is simply one which never updates except when its props have | ||
// changed -- which is exactly what we want.) | ||
class PresenceHeartbeat extends PureComponent<Props> { | ||
/** Callback for Heartbeat object. */ | ||
onHeartbeat = (state: boolean) => { | ||
// N.B.: If `auth` changes, we do not send out a final `false` presence | ||
// status for the previous `auth`. It's the responsibility of our logout | ||
// handler to determine whether that's necessary. | ||
// | ||
// (TODO: ensure that our logout handlers actually do that.) | ||
if (this.props.auth) { | ||
this.props.dispatch(reportPresence(state)); | ||
} | ||
}; | ||
|
||
heartbeat: Heartbeat = new Heartbeat(this.onHeartbeat, 1000 * 60); | ||
|
||
componentDidMount() { | ||
this.updateHeartbeatState(); // conditional start | ||
AppState.addEventListener('change', this.updateHeartbeatState); | ||
} | ||
|
||
componentWillUnmount() { | ||
AppState.removeEventListener('change', this.updateHeartbeatState); | ||
this.heartbeat.stop(); // unconditional stop | ||
} | ||
|
||
// React to any state change. | ||
updateHeartbeatState = () => { | ||
// heartbeat.toState is idempotent | ||
this.heartbeat.toState(AppState.currentState === 'active' && !!this.props.auth); | ||
}; | ||
|
||
// React to props changes. | ||
// | ||
// Not dependent on `render()`'s return value, although the docs may not yet | ||
// be clear on that. See: https://github.com/reactjs/reactjs.org/pull/1230. | ||
componentDidUpdate() { | ||
this.updateHeartbeatState(); | ||
} | ||
|
||
render() { | ||
return null; | ||
} | ||
} | ||
|
||
export default connect(state => ({ | ||
auth: tryGetAuth(state), | ||
}))(PresenceHeartbeat); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters