This repository was archived by the owner on Sep 11, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 827
Fix instantly sending RRs #2770
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ce16236
Fix instantly sending RRs
dbkr 61ebf4d
spurious semicolon
dbkr 7e424ce
Fix call to stop()
dbkr b404d21
PR feedback
dbkr 999ebe6
Missed the removes
dbkr ce9f3d8
Rename
dbkr 63d19a8
Rest of the naming changes
dbkr ea01853
Rename
jryans 2d074d0
Rename
jryans 08e21ff
Fix comment
dbkr 374be0b
Rename more things
dbkr 9f4de60
Merge branch 'dbkr/fix_instant_rrs_pt2' of github.com:matrix-org/matr…
dbkr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,110 +18,173 @@ 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 | ||
const CURRENTLY_ACTIVE_THRESHOLD_MS = 2 * 60 * 1000; | ||
// 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; | ||
jryans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// 'Under a few minutes'. | ||
const RECENTLY_ACTIVE_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': 'active now' and 'active recently' | ||
* see doc on the userActive* 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._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); | ||
this._onWindowBlurred = this._onWindowBlurred.bind(this); | ||
this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this); | ||
this.lastScreenX = 0; | ||
this.lastScreenY = 0; | ||
} | ||
|
||
static sharedInstance() { | ||
if (global.mxUserActivity === undefined) { | ||
global.mxUserActivity = new UserActivity(window, document); | ||
jryans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
return global.mxUserActivity; | ||
} | ||
|
||
/** | ||
* 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. | ||
* 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 | ||
*/ | ||
timeWhileActiveNow(timer) { | ||
this._timeWhile(timer, this._attachedActiveNowTimers); | ||
if (this.userActiveNow()) { | ||
timer.start(); | ||
} | ||
} | ||
|
||
/** | ||
* Runs the given timer while the user is active, aborting when the user becomes inactive. | ||
* 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. | ||
* @param {Timer} timer the timer to use | ||
*/ | ||
timeWhileActive(timer) { | ||
timeWhileActiveRecently(timer) { | ||
this._timeWhile(timer, this._attachedActiveRecentlyTimers); | ||
if (this.userActiveRecently()) { | ||
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.addEventListener('mousedown', this._onUserActivity); | ||
this._document.addEventListener('mousemove', this._onUserActivity); | ||
this._document.addEventListener('keydown', this._onUserActivity); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for doing this! 😁 We should also change the ones in |
||
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, | ||
{ passive: true, capture: true }); | ||
this._window.addEventListener('wheel', this._onUserActivity, { | ||
passive: true, capture: true, | ||
}); | ||
} | ||
|
||
/** | ||
* Stop tracking user activity | ||
*/ | ||
stop() { | ||
document.onmousedown = undefined; | ||
document.onmousemove = undefined; | ||
document.onkeydown = undefined; | ||
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('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); | ||
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(); | ||
userActiveNow() { | ||
return this._activeNowTimeout.isRunning(); | ||
} | ||
|
||
/** | ||
* 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 | ||
*/ | ||
userActiveRecently() { | ||
return this._activeRecentlyTimeout.isRunning(); | ||
} | ||
|
||
_onPageVisibilityChanged(e) { | ||
if (document.visibilityState === "hidden") { | ||
this._activityTimeout.abort(); | ||
if (this._document.visibilityState === "hidden") { | ||
this._activeNowTimeout.abort(); | ||
this._activeRecentlyTimeout.abort(); | ||
} else { | ||
this._onUserActivity(e); | ||
} | ||
} | ||
|
||
_onWindowBlurred() { | ||
this._activityTimeout.abort(); | ||
this._activeNowTimeout.abort(); | ||
this._activeRecentlyTimeout.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 +195,29 @@ class UserActivity { | |
} | ||
|
||
dis.dispatch({action: 'user_activity'}); | ||
jryans marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!this._activityTimeout.isRunning()) { | ||
this._activityTimeout.start(); | ||
if (!this._activeNowTimeout.isRunning()) { | ||
this._activeNowTimeout.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._attachedActiveNowTimers, this._activeNowTimeout); | ||
} else { | ||
this._activityTimeout.restart(); | ||
this._activeNowTimeout.restart(); | ||
} | ||
} | ||
} | ||
|
||
if (!this._activeRecentlyTimeout.isRunning()) { | ||
this._activeRecentlyTimeout.start(); | ||
|
||
this._runTimersUntilTimeout(this._attachedActiveRecentlyTimers, this._activeRecentlyTimeout); | ||
} else { | ||
this._activeRecentlyTimeout.restart(); | ||
} | ||
} | ||
|
||
module.exports = new UserActivity(); | ||
async _runTimersUntilTimeout(attachedTimers, timeout) { | ||
attachedTimers.forEach((t) => t.start()); | ||
try { | ||
await timeout.finished(); | ||
} catch (_e) { /* aborted */ } | ||
attachedTimers.forEach((t) => t.abort()); | ||
} | ||
} |
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
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess
DMRoomMap
below reveals another, slightly different convention for shared instances. In general, the approach ofsharedInstance
(to create one if it doesn't exist and return) without a separate getter seems simpler to me.Maybe there's value in filing an issue to unify approaches for simplicity? The peg style is also related, though not quite the same. Anyway, might be worth having a consistent style, especially if want to add this kind of thing more for testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'd forgotten about DMRoomMap's approach to singletons. Have filed element-hq/element-web#9090