Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Fix instantly sending RRs #2770

Merged
merged 12 commits into from
Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 34 additions & 32 deletions src/UserActivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These thresholds should likely also be renamed to match as well.

this._onUserActivity = this._onUserActivity.bind(this);
this._onWindowBlurred = this._onWindowBlurred.bind(this);
this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this);
Expand All @@ -61,32 +61,33 @@ 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.
* @param {Timer} timer the timer to use
*/
timeWhileActive(timer) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, shouldn't we carry the new naming through to this API as well? We'd then have timeWhileActiveNow and timeWhileActiveRecently.

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.
* @param {Timer} timer the timer to use
*/
timeWhilePassive(timer) {
this._timeWhile(timer, this._attachedTimersPassive);
if (this.userCurrentlyPassive()) {
if (this.userActiveRecently()) {
timer.start();
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/components/structures/TimelinePanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 14 additions & 14 deletions test/UserActivity-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,46 +56,46 @@ 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() {
fakeDocument.hasFocus = jest.fn().mockReturnValue(true);

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() {
fakeDocument.hasFocus = jest.fn().mockReturnValue(true);

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() {
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -129,6 +129,6 @@ describe('UserActivity', function() {
userActivity._onUserActivity({});
clock.tick(1 * 60 * 1000);

expect(userActivity.userCurrentlyPassive()).toBe(true);
expect(userActivity.userActiveRecently()).toBe(true);
});
});