From ce1623691e8bfcd5539634c69f6353356bd381e5 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2019 12:46:38 +0000 Subject: [PATCH 01/11] Fix instantly sending RRs Splits UserActivity into a tristate of 'active' (last < 1s), 'passive' (lasts a couple of mins) and neither. Read receipts are sent when 'active', read markers are sent while 'passive'. Also fixed a document / window mix-up on the 'blur' handler. Also adds a unit test for UserActivity because it's quite complex now (and changes UserActivity to make it testable by accessing the singleton via sharedInstance() rather than exporting it directly). Fixes https://github.com/vector-im/riot-web/issues/9023 --- src/Lifecycle.js | 2 +- src/UserActivity.js | 157 +++++++++++++++------ src/components/structures/TimelinePanel.js | 8 +- test/UserActivity-test.js | 134 ++++++++++++++++++ 4 files changed, 250 insertions(+), 51 deletions(-) create mode 100644 test/UserActivity-test.js diff --git a/src/Lifecycle.js b/src/Lifecycle.js index 54ac605c65..e725e91044 100644 --- a/src/Lifecycle.js +++ b/src/Lifecycle.js @@ -439,7 +439,7 @@ async function startMatrixClient() { dis.dispatch({action: 'will_start_client'}, true); Notifier.start(); - UserActivity.start(); + UserActivity.sharedInstance().start(); Presence.start(); DMRoomMap.makeShared().start(); ActiveWidgetStore.start(); diff --git a/src/UserActivity.js b/src/UserActivity.js index b49bcf1a91..a4fba65f7f 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -23,16 +23,25 @@ import Timer from './utils/Timer'; // such as READ_MARKER_INVIEW_THRESHOLD_MS, // READ_MARKER_OUTOFVIEW_THRESHOLD_MS, // READ_RECEIPT_INTERVAL_MS in TimelinePanel -const CURRENTLY_ACTIVE_THRESHOLD_MS = 2 * 60 * 1000; +const CURRENTLY_ACTIVE_THRESHOLD_MS = 700; +const CURRENTLY_PASSIVE_THRESHOLD_MS = 2 * 60 * 1000; /** * This class watches for user activity (moving the mouse or pressing a key) * and starts/stops attached timers while the user is active. + * + * There are two classes of 'active' a user can be: 'active' and 'passive': + * see doc on the userCurrently* functions for what these mean. */ -class UserActivity { - constructor() { - this._attachedTimers = []; - this._activityTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS); +export default class UserActivity { + constructor(windowObj, documentObj) { + this._window = windowObj; + this._document = documentObj; + + this._attachedTimersActive = []; + this._attachedTimersPassive = []; + this._activeTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS); + this._passiveTimeout = new Timer(CURRENTLY_PASSIVE_THRESHOLD_MS); this._onUserActivity = this._onUserActivity.bind(this); this._onWindowBlurred = this._onWindowBlurred.bind(this); this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this); @@ -40,48 +49,76 @@ class UserActivity { this.lastScreenY = 0; } + static sharedInstance() { + if (global.mxUserActivity === undefined) { + global.mxUserActivity = new UserActivity(window, document); + } + return global.mxUserActivity; + } + /** - * Runs the given timer while the user is active, aborting when the user becomes inactive. + * Runs the given timer while the user is 'active', aborting when the user becomes 'passive'. + * See userCurrentlyActive() for what it means for a user to be 'active'. * Can be called multiple times with the same already running timer, which is a NO-OP. * Can be called before the user becomes active, in which case it is only started * later on when the user does become active. * @param {Timer} timer the timer to use */ timeWhileActive(timer) { + this._timeWhile(timer, this._attachedTimersActive); + if (this.userCurrentlyActive()) { + timer.start(); + } + } + + /** + * Runs the given timer while the user is 'active' or 'passive', aborting when the user becomes + * inactive. + * See userCurrentlyPassive() for what it means for a user to be 'passive'. + * Can be called multiple times with the same already running timer, which is a NO-OP. + * Can be called before the user becomes active, in which case it is only started + * later on when the user does become active. + * @param {Timer} timer the timer to use + */ + timeWhilePassive(timer) { + this._timeWhile(timer, this._attachedTimersPassive); + if (this.userCurrentlyPassive()) { + timer.start(); + } + } + + _timeWhile(timer, attachedTimers) { // important this happens first - const index = this._attachedTimers.indexOf(timer); + const index = attachedTimers.indexOf(timer); if (index === -1) { - this._attachedTimers.push(timer); + attachedTimers.push(timer); // remove when done or aborted timer.finished().finally(() => { - const index = this._attachedTimers.indexOf(timer); + const index = attachedTimers.indexOf(timer); if (index !== -1) { // should never be -1 - this._attachedTimers.splice(index, 1); + attachedTimers.splice(index, 1); } // as we fork the promise here, // avoid unhandled rejection warnings }).catch((err) => {}); } - if (this.userCurrentlyActive()) { - timer.start(); - } } /** * Start listening to user activity */ start() { - document.onmousedown = this._onUserActivity; - document.onmousemove = this._onUserActivity; - document.onkeydown = this._onUserActivity; - document.addEventListener("visibilitychange", this._onPageVisibilityChanged); - window.addEventListener("blur", this._onWindowBlurred); - window.addEventListener("focus", this._onUserActivity); + this._document.onmousedown = this._onUserActivity; + this._document.onmousemove = this._onUserActivity; + this._document.onkeydown = this._onUserActivity; + this._document.addEventListener("visibilitychange", this._onPageVisibilityChanged); + this._window.addEventListener("blur", this._onWindowBlurred); + this._window.addEventListener("focus", this._onUserActivity); // can't use document.scroll here because that's only the document // itself being scrolled. Need to use addEventListener's useCapture. // also this needs to be the wheel event, not scroll, as scroll is // fired when the view scrolls down for a new message. - window.addEventListener('wheel', this._onUserActivity, + this._window.addEventListener('wheel', this._onUserActivity, { passive: true, capture: true }); } @@ -89,39 +126,57 @@ class UserActivity { * Stop tracking user activity */ stop() { - document.onmousedown = undefined; - document.onmousemove = undefined; - document.onkeydown = undefined; - window.removeEventListener('wheel', this._onUserActivity, + this._document.onmousedown = undefined; + this._document.onmousemove = undefined; + this._document.onkeydown = undefined; + this._window.removeEventListener('wheel', this._onUserActivity, { passive: true, capture: true }); - document.removeEventListener("visibilitychange", this._onPageVisibilityChanged); - document.removeEventListener("blur", this._onDocumentBlurred); - document.removeEventListener("focus", this._onUserActivity); + this._document.removeEventListener("visibilitychange", this._onPageVisibilityChanged); + this._window.removeEventListener("blur", this._onWindowBlurred); + this._window.removeEventListener("focus", this._onUserActivity); } /** - * Return true if there has been user activity very recently - * (ie. within a few seconds) - * @returns {boolean} true if user is currently/very recently active + * Return true if the user is currently 'active' + * A user is 'active' while they are interacting with the app and for a very short (<1s) + * time after that. This is intended to give a strong indication that the app has the + * user's attention at any given moment. + * @returns {boolean} true if user is currently 'active' */ userCurrentlyActive() { - return this._activityTimeout.isRunning(); + return this._activeTimeout.isRunning(); + } + + /** + * Return true if the user is currently 'active' or 'passive' + * A user is 'passive' for a longer period of time (~2 mins) after they have been 'active' and + * while the app still has the focus. This is intended to indicate when the app may still have + * the user's attention (or they may have gone to make tea and left the window focused). + * @returns {boolean} true if user is currently 'active' or 'passive' + */ + userCurrentlyPassive() { + return this._passiveTimeout.isRunning(); } _onPageVisibilityChanged(e) { - if (document.visibilityState === "hidden") { - this._activityTimeout.abort(); + if (this._document.visibilityState === "hidden") { + this._activeTimeout.abort(); + this._passiveTimeout.abort(); } else { this._onUserActivity(e); } } _onWindowBlurred() { - this._activityTimeout.abort(); + this._activeTimeout.abort(); + this._passiveTimeout.abort(); } - async _onUserActivity(event) { + _onUserActivity(event) { + // ignore anything if the window isn't focused + if (!this._document.hasFocus()) return; + if (event.screenX && event.type === "mousemove") { if (event.screenX === this.lastScreenX && event.screenY === this.lastScreenY) { // mouse hasn't actually moved @@ -132,19 +187,29 @@ class UserActivity { } dis.dispatch({action: 'user_activity'}); - if (!this._activityTimeout.isRunning()) { - this._activityTimeout.start(); + if (!this._activeTimeout.isRunning()) { + this._activeTimeout.start(); dis.dispatch({action: 'user_activity_start'}); - this._attachedTimers.forEach((t) => t.start()); - try { - await this._activityTimeout.finished(); - } catch (_e) { /* aborted */ } - this._attachedTimers.forEach((t) => t.abort()); + + this._runTimersUntilTimeout(this._attachedTimersActive, this._activeTimeout); } else { - this._activityTimeout.restart(); + this._activeTimeout.restart(); + } + + if (!this._passiveTimeout.isRunning()) { + this._passiveTimeout.start(); + + this._runTimersUntilTimeout(this._attachedTimersPassive, this._passiveTimeout); + } else { + this._passiveTimeout.restart(); } } + + async _runTimersUntilTimeout(attachedTimers, timeout) { + attachedTimers.forEach((t) => t.start()); + try { + await timeout.finished(); + } catch (_e) { /* aborted */ } + attachedTimers.forEach((t) => t.abort()); + } } - - -module.exports = new UserActivity(); diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 911499e314..ca2d01de04 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -451,12 +451,12 @@ var TimelinePanel = React.createClass({ // // We ignore events we have sent ourselves; we don't want to see the // read-marker when a remote echo of an event we have just sent takes - // more than the timeout on userCurrentlyActive. + // more than the timeout on userCurrentlyPassive. // const myUserId = MatrixClientPeg.get().credentials.userId; const sender = ev.sender ? ev.sender.userId : null; var callRMUpdated = false; - if (sender != myUserId && !UserActivity.userCurrentlyActive()) { + if (sender != myUserId && !UserActivity.sharedInstance().userCurrentlyPassive()) { updatedState.readMarkerVisible = true; } else if (lastEv && this.getReadMarkerPosition() === 0) { // we know we're stuckAtBottom, so we can advance the RM @@ -562,7 +562,7 @@ var TimelinePanel = React.createClass({ this._readMarkerActivityTimer = new Timer(initialTimeout); while (this._readMarkerActivityTimer) { //unset on unmount - UserActivity.timeWhileActive(this._readMarkerActivityTimer); + UserActivity.sharedInstance().timeWhilePassive(this._readMarkerActivityTimer); try { await this._readMarkerActivityTimer.finished(); } catch(e) { continue; /* aborted */ } @@ -574,7 +574,7 @@ var TimelinePanel = React.createClass({ updateReadReceiptOnUserActivity: async function() { this._readReceiptActivityTimer = new Timer(READ_RECEIPT_INTERVAL_MS); while (this._readReceiptActivityTimer) { //unset on unmount - UserActivity.timeWhileActive(this._readReceiptActivityTimer); + UserActivity.sharedInstance().timeWhileActive(this._readReceiptActivityTimer); try { await this._readReceiptActivityTimer.finished(); } catch(e) { continue; /* aborted */ } diff --git a/test/UserActivity-test.js b/test/UserActivity-test.js new file mode 100644 index 0000000000..db15fe27c4 --- /dev/null +++ b/test/UserActivity-test.js @@ -0,0 +1,134 @@ +/* +Copyright 2019 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import expect from 'expect'; +import lolex from 'lolex'; +import jest from 'jest-mock'; +import EventEmitter from 'events'; +import UserActivity from '../src/UserActivity'; + +class FakeDomEventEmitter extends EventEmitter { + addEventListener(what, l) { + this.on(what, l); + } + + removeEventListener(what, l) { + this.removeListener(what, l); + } +}; + +describe('UserActivity', function() { + let fakeWindow; + let fakeDocument; + let userActivity; + let clock; + + beforeEach(function() { + fakeWindow = new FakeDomEventEmitter(), + fakeDocument = new FakeDomEventEmitter(), + userActivity = new UserActivity(fakeWindow, fakeDocument); + userActivity.start(); + clock = lolex.install(); + }); + + afterEach(function() { + userActivity.stop(); + userActivity = null; + clock.uninstall(); + clock = null; + }); + + it('should return the same shared instance', function() { + expect(UserActivity.sharedInstance()).toBe(UserActivity.sharedInstance()); + }); + + it('should consider user inactive if no activity', function() { + expect(userActivity.userCurrentlyActive()).toBe(false); + }); + + it('should consider user not passive if no activity', function() { + expect(userActivity.userCurrentlyPassive()).toBe(false); + }); + + it('should not consider user active after activity if no window focus', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(false); + + userActivity._onUserActivity({}); + expect(userActivity.userCurrentlyActive()).toBe(false); + expect(userActivity.userCurrentlyPassive()).toBe(false); + }); + + it('should consider user active shortly after activity', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(true); + + userActivity._onUserActivity({}); + expect(userActivity.userCurrentlyActive()).toBe(true); + expect(userActivity.userCurrentlyPassive()).toBe(true); + clock.tick(200); + expect(userActivity.userCurrentlyActive()).toBe(true); + expect(userActivity.userCurrentlyPassive()).toBe(true); + }); + + it('should consider user not active after 10s of no activity', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(true); + + userActivity._onUserActivity({}); + clock.tick(10000); + expect(userActivity.userCurrentlyActive()).toBe(false); + }); + + it('should consider user passive after 10s of no activity', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(true); + + userActivity._onUserActivity({}); + clock.tick(10000); + expect(userActivity.userCurrentlyPassive()).toBe(true); + }); + + it('should not consider user passive after 10s if window un-focused', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(true); + + userActivity._onUserActivity({}); + clock.tick(10000); + + fakeDocument.hasFocus = jest.fn().mockReturnValue(false); + fakeWindow.emit('blur', {}); + + expect(userActivity.userCurrentlyPassive()).toBe(false); + }); + + it('should not consider user passive after 3 mins', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(true); + + userActivity._onUserActivity({}); + clock.tick(3 * 60 * 1000); + + expect(userActivity.userCurrentlyPassive()).toBe(false); + }); + + it('should extend timer on activity', function() { + fakeDocument.hasFocus = jest.fn().mockReturnValue(true); + + userActivity._onUserActivity({}); + clock.tick(1 * 60 * 1000); + userActivity._onUserActivity({}); + clock.tick(1 * 60 * 1000); + userActivity._onUserActivity({}); + clock.tick(1 * 60 * 1000); + + expect(userActivity.userCurrentlyPassive()).toBe(true); + }); +}); From 61ebf4d92003166f7081ebe0103ba44da57dc79a Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2019 13:52:04 +0000 Subject: [PATCH 02/11] spurious semicolon --- test/UserActivity-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/UserActivity-test.js b/test/UserActivity-test.js index db15fe27c4..e3865e7ab6 100644 --- a/test/UserActivity-test.js +++ b/test/UserActivity-test.js @@ -28,7 +28,7 @@ class FakeDomEventEmitter extends EventEmitter { removeEventListener(what, l) { this.removeListener(what, l); } -}; +} describe('UserActivity', function() { let fakeWindow; From 7e424ce95b84cf000820a8d4af5027a9844e6233 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2019 15:09:44 +0000 Subject: [PATCH 03/11] Fix call to stop() --- src/Lifecycle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lifecycle.js b/src/Lifecycle.js index e725e91044..6a113db418 100644 --- a/src/Lifecycle.js +++ b/src/Lifecycle.js @@ -485,7 +485,7 @@ function _clearStorage() { */ export function stopMatrixClient() { Notifier.stop(); - UserActivity.stop(); + UserActivity.sharedInstance().stop(); Presence.stop(); ActiveWidgetStore.stop(); if (DMRoomMap.shared()) DMRoomMap.shared().stop(); From b404d21bbaea24dffb56d86cd7c49d355a24e844 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2019 17:41:04 +0000 Subject: [PATCH 04/11] PR feedback --- src/UserActivity.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/UserActivity.js b/src/UserActivity.js index a4fba65f7f..c556be8091 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -23,7 +23,11 @@ import Timer from './utils/Timer'; // such as READ_MARKER_INVIEW_THRESHOLD_MS, // READ_MARKER_OUTOFVIEW_THRESHOLD_MS, // READ_RECEIPT_INTERVAL_MS in TimelinePanel + +// 'Under a few seconds'. Must be less than 'CURRENTLY_PASSIVE_THRESHOLD_MS' const CURRENTLY_ACTIVE_THRESHOLD_MS = 700; + +// 'Under a few minutes'. const CURRENTLY_PASSIVE_THRESHOLD_MS = 2 * 60 * 1000; /** @@ -108,9 +112,9 @@ export default class UserActivity { * Start listening to user activity */ start() { - this._document.onmousedown = this._onUserActivity; - this._document.onmousemove = this._onUserActivity; - this._document.onkeydown = this._onUserActivity; + this._document.addEventListener('mousedown', this._onUserActivity); + this._document.addEventListener('mousemove', this._onUserActivity); + this._document.addEventListener('keydown', this._onUserActivity); this._document.addEventListener("visibilitychange", this._onPageVisibilityChanged); this._window.addEventListener("blur", this._onWindowBlurred); this._window.addEventListener("focus", this._onUserActivity); @@ -118,8 +122,9 @@ export default class UserActivity { // itself being scrolled. Need to use addEventListener's useCapture. // also this needs to be the wheel event, not scroll, as scroll is // fired when the view scrolls down for a new message. - this._window.addEventListener('wheel', this._onUserActivity, - { passive: true, capture: true }); + this._window.addEventListener('wheel', this._onUserActivity, { + passive: true, capture: true, + }); } /** From 999ebe6a19a1e0fff5d6affc6b4a42f94cdc47e2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2019 21:51:14 +0000 Subject: [PATCH 05/11] Missed the removes also fix more indenting --- src/UserActivity.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/UserActivity.js b/src/UserActivity.js index c556be8091..3a5880210f 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -131,11 +131,12 @@ export default class UserActivity { * Stop tracking user activity */ stop() { - this._document.onmousedown = undefined; - this._document.onmousemove = undefined; - this._document.onkeydown = undefined; - this._window.removeEventListener('wheel', this._onUserActivity, - { passive: true, capture: true }); + this._document.removeEventListener('mousedown', this._onUserActivity); + this._document.removeEventListener('mousemove', this._onUserActivity); + this._document.removeEventListener('keydown', this._onUserActivity); + this._window.removeEventListener('wheel', this._onUserActivity, { + passive: true, capture: true, + }); this._document.removeEventListener("visibilitychange", this._onPageVisibilityChanged); this._window.removeEventListener("blur", this._onWindowBlurred); From ce9f3d8a57a16810e61d08c70edb64028bd16f40 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 11 Mar 2019 11:38:54 +0000 Subject: [PATCH 06/11] Rename --- src/UserActivity.js | 66 +++++++++++----------- src/components/structures/TimelinePanel.js | 4 +- test/UserActivity-test.js | 28 ++++----- 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/src/UserActivity.js b/src/UserActivity.js index 3a5880210f..dd2a46d340 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -34,8 +34,8 @@ const CURRENTLY_PASSIVE_THRESHOLD_MS = 2 * 60 * 1000; * This class watches for user activity (moving the mouse or pressing a key) * and starts/stops attached timers while the user is active. * - * There are two classes of 'active' a user can be: 'active' and 'passive': - * see doc on the userCurrently* functions for what these mean. + * There are two classes of 'active': 'active now' and 'active recently' + * see doc on the userActive* functions for what these mean. */ export default class UserActivity { constructor(windowObj, documentObj) { @@ -44,8 +44,8 @@ export default class UserActivity { this._attachedTimersActive = []; this._attachedTimersPassive = []; - this._activeTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS); - this._passiveTimeout = new Timer(CURRENTLY_PASSIVE_THRESHOLD_MS); + this._activeNowTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS); + this._activeRecentlyTimeout = new Timer(CURRENTLY_PASSIVE_THRESHOLD_MS); this._onUserActivity = this._onUserActivity.bind(this); this._onWindowBlurred = this._onWindowBlurred.bind(this); this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this); @@ -61,8 +61,9 @@ export default class UserActivity { } /** - * Runs the given timer while the user is 'active', aborting when the user becomes 'passive'. - * See userCurrentlyActive() for what it means for a user to be 'active'. + * Runs the given timer while the user is 'active', aborting when the user is no longer + * considered currently active. + * See userActiveNow() for what it means for a user to be 'active'. * Can be called multiple times with the same already running timer, which is a NO-OP. * Can be called before the user becomes active, in which case it is only started * later on when the user does become active. @@ -70,15 +71,15 @@ export default class UserActivity { */ timeWhileActive(timer) { this._timeWhile(timer, this._attachedTimersActive); - if (this.userCurrentlyActive()) { + if (this.userActiveNow()) { timer.start(); } } /** - * Runs the given timer while the user is 'active' or 'passive', aborting when the user becomes - * inactive. - * See userCurrentlyPassive() for what it means for a user to be 'passive'. + * Runs the given timer while the user is 'active' now or recently, + * aborting when the user becomes inactive. + * See userActiveRecently() for what it means for a user to be 'active recently'. * Can be called multiple times with the same already running timer, which is a NO-OP. * Can be called before the user becomes active, in which case it is only started * later on when the user does become active. @@ -86,7 +87,7 @@ export default class UserActivity { */ timeWhilePassive(timer) { this._timeWhile(timer, this._attachedTimersPassive); - if (this.userCurrentlyPassive()) { + if (this.userActiveRecently()) { timer.start(); } } @@ -150,33 +151,34 @@ export default class UserActivity { * user's attention at any given moment. * @returns {boolean} true if user is currently 'active' */ - userCurrentlyActive() { - return this._activeTimeout.isRunning(); + userActiveNow() { + return this._activeNowTimeout.isRunning(); } /** - * Return true if the user is currently 'active' or 'passive' - * A user is 'passive' for a longer period of time (~2 mins) after they have been 'active' and - * while the app still has the focus. This is intended to indicate when the app may still have - * the user's attention (or they may have gone to make tea and left the window focused). - * @returns {boolean} true if user is currently 'active' or 'passive' + * Return true if the user is currently active or has been recently + * A user is 'active recently' for a longer period of time (~2 mins) after + * they have been 'active' and while the app still has the focus. This is + * intended to indicate when the app may still have the user's attention + * (or they may have gone to make tea and left the window focused). + * @returns {boolean} true if user has been active recently */ - userCurrentlyPassive() { - return this._passiveTimeout.isRunning(); + userActiveRecently() { + return this._activeRecentlyTimeout.isRunning(); } _onPageVisibilityChanged(e) { if (this._document.visibilityState === "hidden") { - this._activeTimeout.abort(); - this._passiveTimeout.abort(); + this._activeNowTimeout.abort(); + this._activeRecentlyTimeout.abort(); } else { this._onUserActivity(e); } } _onWindowBlurred() { - this._activeTimeout.abort(); - this._passiveTimeout.abort(); + this._activeNowTimeout.abort(); + this._activeRecentlyTimeout.abort(); } _onUserActivity(event) { @@ -193,21 +195,21 @@ export default class UserActivity { } dis.dispatch({action: 'user_activity'}); - if (!this._activeTimeout.isRunning()) { - this._activeTimeout.start(); + if (!this._activeNowTimeout.isRunning()) { + this._activeNowTimeout.start(); dis.dispatch({action: 'user_activity_start'}); - this._runTimersUntilTimeout(this._attachedTimersActive, this._activeTimeout); + this._runTimersUntilTimeout(this._attachedTimersActive, this._activeNowTimeout); } else { - this._activeTimeout.restart(); + this._activeNowTimeout.restart(); } - if (!this._passiveTimeout.isRunning()) { - this._passiveTimeout.start(); + if (!this._activeRecentlyTimeout.isRunning()) { + this._activeRecentlyTimeout.start(); - this._runTimersUntilTimeout(this._attachedTimersPassive, this._passiveTimeout); + this._runTimersUntilTimeout(this._attachedTimersPassive, this._activeRecentlyTimeout); } else { - this._passiveTimeout.restart(); + this._activeRecentlyTimeout.restart(); } } diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index ca2d01de04..dcb60f8970 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -451,12 +451,12 @@ var TimelinePanel = React.createClass({ // // We ignore events we have sent ourselves; we don't want to see the // read-marker when a remote echo of an event we have just sent takes - // more than the timeout on userCurrentlyPassive. + // more than the timeout on userRecentlyActive. // const myUserId = MatrixClientPeg.get().credentials.userId; const sender = ev.sender ? ev.sender.userId : null; var callRMUpdated = false; - if (sender != myUserId && !UserActivity.sharedInstance().userCurrentlyPassive()) { + if (sender != myUserId && !UserActivity.sharedInstance().userRecentlyActive()) { updatedState.readMarkerVisible = true; } else if (lastEv && this.getReadMarkerPosition() === 0) { // we know we're stuckAtBottom, so we can advance the RM diff --git a/test/UserActivity-test.js b/test/UserActivity-test.js index e3865e7ab6..6c684d25e9 100644 --- a/test/UserActivity-test.js +++ b/test/UserActivity-test.js @@ -56,30 +56,30 @@ describe('UserActivity', function() { }); it('should consider user inactive if no activity', function() { - expect(userActivity.userCurrentlyActive()).toBe(false); + expect(userActivity.userActiveNow()).toBe(false); }); - it('should consider user not passive if no activity', function() { - expect(userActivity.userCurrentlyPassive()).toBe(false); + it('should consider user not active recently if no activity', function() { + expect(userActivity.userActiveRecently()).toBe(false); }); it('should not consider user active after activity if no window focus', function() { fakeDocument.hasFocus = jest.fn().mockReturnValue(false); userActivity._onUserActivity({}); - expect(userActivity.userCurrentlyActive()).toBe(false); - expect(userActivity.userCurrentlyPassive()).toBe(false); + expect(userActivity.userActiveNow()).toBe(false); + expect(userActivity.userActiveRecently()).toBe(false); }); it('should consider user active shortly after activity', function() { fakeDocument.hasFocus = jest.fn().mockReturnValue(true); userActivity._onUserActivity({}); - expect(userActivity.userCurrentlyActive()).toBe(true); - expect(userActivity.userCurrentlyPassive()).toBe(true); + expect(userActivity.userActiveNow()).toBe(true); + expect(userActivity.userActiveRecently()).toBe(true); clock.tick(200); - expect(userActivity.userCurrentlyActive()).toBe(true); - expect(userActivity.userCurrentlyPassive()).toBe(true); + expect(userActivity.userActiveNow()).toBe(true); + expect(userActivity.userActiveRecently()).toBe(true); }); it('should consider user not active after 10s of no activity', function() { @@ -87,7 +87,7 @@ describe('UserActivity', function() { userActivity._onUserActivity({}); clock.tick(10000); - expect(userActivity.userCurrentlyActive()).toBe(false); + expect(userActivity.userActiveNow()).toBe(false); }); it('should consider user passive after 10s of no activity', function() { @@ -95,7 +95,7 @@ describe('UserActivity', function() { userActivity._onUserActivity({}); clock.tick(10000); - expect(userActivity.userCurrentlyPassive()).toBe(true); + expect(userActivity.userActiveRecently()).toBe(true); }); it('should not consider user passive after 10s if window un-focused', function() { @@ -107,7 +107,7 @@ describe('UserActivity', function() { fakeDocument.hasFocus = jest.fn().mockReturnValue(false); fakeWindow.emit('blur', {}); - expect(userActivity.userCurrentlyPassive()).toBe(false); + expect(userActivity.userActiveRecently()).toBe(false); }); it('should not consider user passive after 3 mins', function() { @@ -116,7 +116,7 @@ describe('UserActivity', function() { userActivity._onUserActivity({}); clock.tick(3 * 60 * 1000); - expect(userActivity.userCurrentlyPassive()).toBe(false); + expect(userActivity.userActiveRecently()).toBe(false); }); it('should extend timer on activity', function() { @@ -129,6 +129,6 @@ describe('UserActivity', function() { userActivity._onUserActivity({}); clock.tick(1 * 60 * 1000); - expect(userActivity.userCurrentlyPassive()).toBe(true); + expect(userActivity.userActiveRecently()).toBe(true); }); }); From 63d19a899b2eb426ef189d10b6f24ba1e15f79bd Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 12 Mar 2019 09:37:00 +0000 Subject: [PATCH 07/11] Rest of the naming changes --- src/UserActivity.js | 12 ++++++------ src/components/structures/TimelinePanel.js | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/UserActivity.js b/src/UserActivity.js index dd2a46d340..02f006f274 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -24,11 +24,11 @@ import Timer from './utils/Timer'; // READ_MARKER_OUTOFVIEW_THRESHOLD_MS, // READ_RECEIPT_INTERVAL_MS in TimelinePanel -// 'Under a few seconds'. Must be less than 'CURRENTLY_PASSIVE_THRESHOLD_MS' +// 'Under a few seconds'. Must be less than 'RECENTLY_ACTIVE_THRESHOLD_MS' const CURRENTLY_ACTIVE_THRESHOLD_MS = 700; // 'Under a few minutes'. -const CURRENTLY_PASSIVE_THRESHOLD_MS = 2 * 60 * 1000; +const RECENTLY_ACTIVE_THRESHOLD_MS = 2 * 60 * 1000; /** * This class watches for user activity (moving the mouse or pressing a key) @@ -45,7 +45,7 @@ export default class UserActivity { this._attachedTimersActive = []; this._attachedTimersPassive = []; this._activeNowTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS); - this._activeRecentlyTimeout = new Timer(CURRENTLY_PASSIVE_THRESHOLD_MS); + this._activeRecentlyTimeout = new Timer(RECENTLY_ACTIVE_THRESHOLD_MS); this._onUserActivity = this._onUserActivity.bind(this); this._onWindowBlurred = this._onWindowBlurred.bind(this); this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this); @@ -61,7 +61,7 @@ export default class UserActivity { } /** - * Runs the given timer while the user is 'active', aborting when the user is no longer + * Runs the given timer while the user is 'active now', aborting when the user is no longer * considered currently active. * See userActiveNow() for what it means for a user to be 'active'. * Can be called multiple times with the same already running timer, which is a NO-OP. @@ -69,7 +69,7 @@ export default class UserActivity { * later on when the user does become active. * @param {Timer} timer the timer to use */ - timeWhileActive(timer) { + timeWhileActiveNow(timer) { this._timeWhile(timer, this._attachedTimersActive); if (this.userActiveNow()) { timer.start(); @@ -85,7 +85,7 @@ export default class UserActivity { * later on when the user does become active. * @param {Timer} timer the timer to use */ - timeWhilePassive(timer) { + timeWhileActiveRecently(timer) { this._timeWhile(timer, this._attachedTimersPassive); if (this.userActiveRecently()) { timer.start(); diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index dcb60f8970..19adafbe4e 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -1,6 +1,7 @@ /* Copyright 2016 OpenMarket Ltd Copyright 2017 Vector Creations Ltd +Copyright 2019 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -562,7 +563,7 @@ var TimelinePanel = React.createClass({ this._readMarkerActivityTimer = new Timer(initialTimeout); while (this._readMarkerActivityTimer) { //unset on unmount - UserActivity.sharedInstance().timeWhilePassive(this._readMarkerActivityTimer); + UserActivity.sharedInstance().timeWhileActiveRecently(this._readMarkerActivityTimer); try { await this._readMarkerActivityTimer.finished(); } catch(e) { continue; /* aborted */ } @@ -574,7 +575,7 @@ var TimelinePanel = React.createClass({ updateReadReceiptOnUserActivity: async function() { this._readReceiptActivityTimer = new Timer(READ_RECEIPT_INTERVAL_MS); while (this._readReceiptActivityTimer) { //unset on unmount - UserActivity.sharedInstance().timeWhileActive(this._readReceiptActivityTimer); + UserActivity.sharedInstance().timeWhileActiveNow(this._readReceiptActivityTimer); try { await this._readReceiptActivityTimer.finished(); } catch(e) { continue; /* aborted */ } From ea0185323316f054b6f928324c00ee124b919788 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 12 Mar 2019 09:37:38 +0000 Subject: [PATCH 08/11] Rename Co-Authored-By: dbkr --- src/components/structures/TimelinePanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 19adafbe4e..c868cac048 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -452,7 +452,7 @@ var TimelinePanel = React.createClass({ // // We ignore events we have sent ourselves; we don't want to see the // read-marker when a remote echo of an event we have just sent takes - // more than the timeout on userRecentlyActive. + // more than the timeout on userActiveRecently. // const myUserId = MatrixClientPeg.get().credentials.userId; const sender = ev.sender ? ev.sender.userId : null; From 2d074d0de609052860b4db77756623d30059a9f4 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 12 Mar 2019 09:37:55 +0000 Subject: [PATCH 09/11] Rename Co-Authored-By: dbkr --- src/components/structures/TimelinePanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index c868cac048..f0feaf94c5 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -457,7 +457,7 @@ var TimelinePanel = React.createClass({ const myUserId = MatrixClientPeg.get().credentials.userId; const sender = ev.sender ? ev.sender.userId : null; var callRMUpdated = false; - if (sender != myUserId && !UserActivity.sharedInstance().userRecentlyActive()) { + if (sender != myUserId && !UserActivity.sharedInstance().userActiveRecently()) { updatedState.readMarkerVisible = true; } else if (lastEv && this.getReadMarkerPosition() === 0) { // we know we're stuckAtBottom, so we can advance the RM From 08e21ff5d411c51b59613586c0e21907829f42e1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 12 Mar 2019 09:40:17 +0000 Subject: [PATCH 10/11] Fix comment --- src/UserActivity.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/UserActivity.js b/src/UserActivity.js index 02f006f274..96b1f76d26 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -18,11 +18,11 @@ limitations under the License. import dis from './dispatcher'; import Timer from './utils/Timer'; -// important this is larger than the timeouts of timers -// used with UserActivity.timeWhileActive, -// such as READ_MARKER_INVIEW_THRESHOLD_MS, -// READ_MARKER_OUTOFVIEW_THRESHOLD_MS, -// READ_RECEIPT_INTERVAL_MS in TimelinePanel +// important these are larger than the timeouts of timers +// used with UserActivity.timeWhileActive*, +// such as READ_MARKER_INVIEW_THRESHOLD_MS (timeWhileActiveRecently), +// READ_MARKER_OUTOFVIEW_THRESHOLD_MS (timeWhileActiveRecently), +// READ_RECEIPT_INTERVAL_MS (timeWhileActiveNow) in TimelinePanel // 'Under a few seconds'. Must be less than 'RECENTLY_ACTIVE_THRESHOLD_MS' const CURRENTLY_ACTIVE_THRESHOLD_MS = 700; From 374be0b3b4ada93bbd6e1018d9d30085a6538d60 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 12 Mar 2019 10:28:47 +0000 Subject: [PATCH 11/11] Rename more things --- src/UserActivity.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/UserActivity.js b/src/UserActivity.js index 96b1f76d26..0d1b4d0cc0 100644 --- a/src/UserActivity.js +++ b/src/UserActivity.js @@ -42,8 +42,8 @@ export default class UserActivity { this._window = windowObj; this._document = documentObj; - this._attachedTimersActive = []; - this._attachedTimersPassive = []; + this._attachedActiveNowTimers = []; + this._attachedActiveRecentlyTimers = []; this._activeNowTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS); this._activeRecentlyTimeout = new Timer(RECENTLY_ACTIVE_THRESHOLD_MS); this._onUserActivity = this._onUserActivity.bind(this); @@ -70,7 +70,7 @@ export default class UserActivity { * @param {Timer} timer the timer to use */ timeWhileActiveNow(timer) { - this._timeWhile(timer, this._attachedTimersActive); + this._timeWhile(timer, this._attachedActiveNowTimers); if (this.userActiveNow()) { timer.start(); } @@ -86,7 +86,7 @@ export default class UserActivity { * @param {Timer} timer the timer to use */ timeWhileActiveRecently(timer) { - this._timeWhile(timer, this._attachedTimersPassive); + this._timeWhile(timer, this._attachedActiveRecentlyTimers); if (this.userActiveRecently()) { timer.start(); } @@ -199,7 +199,7 @@ export default class UserActivity { this._activeNowTimeout.start(); dis.dispatch({action: 'user_activity_start'}); - this._runTimersUntilTimeout(this._attachedTimersActive, this._activeNowTimeout); + this._runTimersUntilTimeout(this._attachedActiveNowTimers, this._activeNowTimeout); } else { this._activeNowTimeout.restart(); } @@ -207,7 +207,7 @@ export default class UserActivity { if (!this._activeRecentlyTimeout.isRunning()) { this._activeRecentlyTimeout.start(); - this._runTimersUntilTimeout(this._attachedTimersPassive, this._activeRecentlyTimeout); + this._runTimersUntilTimeout(this._attachedActiveRecentlyTimers, this._activeRecentlyTimeout); } else { this._activeRecentlyTimeout.restart(); }