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

Debounce read marker update on scroll #6774

Merged
merged 2 commits into from
Sep 9, 2021
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
Next Next commit
Debounce read marker update on scroll
Reverts #6751 in
favour of debouncing the updates to read markers, because it seems
allowing the scroll to be 1px away from the bottom was important for
some browsers and meant they never got to the bottom.

We can fix the problem instead by debouncing the update to read
markers, because the scroll state gets reset back to the bottom when
componentDidUpdate() runs which happens after the read marker code
does a setState(). This should probably be debounced anyway since
it doesn't need to be run that frequently.

Fixes element-hq/element-web#18961
Type: bugfix
  • Loading branch information
dbkr committed Sep 9, 2021
commit eba815afda03225957957283242cf9ca86d0c6cc
2 changes: 1 addition & 1 deletion src/components/structures/ScrollPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ export default class ScrollPanel extends React.Component<IProps> {
// for scrollTop happen on certain browsers/platforms
// when scrolled all the way down. E.g. Chrome 72 on debian.
// so check difference < 1;
return Math.abs(sn.scrollHeight - (sn.scrollTop + sn.clientHeight)) < 1;
return Math.abs(sn.scrollHeight - (sn.scrollTop + sn.clientHeight)) <= 1;
};

// returns the vertical height in the given direction that can be removed from
Expand Down
42 changes: 29 additions & 13 deletions src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,14 @@ import { RoomPermalinkCreator } from "../../utils/permalinks/Permalinks";
import Spinner from "../views/elements/Spinner";
import EditorStateTransfer from '../../utils/EditorStateTransfer';
import ErrorDialog from '../views/dialogs/ErrorDialog';
import { debounce } from 'lodash';

const PAGINATE_SIZE = 20;
const INITIAL_SIZE = 20;
const READ_RECEIPT_INTERVAL_MS = 500;

const READ_MARKER_DEBOUNCE_MS = 100;

const DEBUG = false;

let debuglog = function(...s: any[]) {};
Expand Down Expand Up @@ -475,22 +478,35 @@ class TimelinePanel extends React.Component<IProps, IState> {
}

if (this.props.manageReadMarkers) {
const rmPosition = this.getReadMarkerPosition();
// we hide the read marker when it first comes onto the screen, but if
// it goes back off the top of the screen (presumably because the user
// clicks on the 'jump to bottom' button), we need to re-enable it.
if (rmPosition < 0) {
this.setState({ readMarkerVisible: true });
}

// if read marker position goes between 0 and -1/1,
// (and user is active), switch timeout
const timeout = this.readMarkerTimeout(rmPosition);
// NO-OP when timeout already has set to the given value
this.readMarkerActivityTimer.changeTimeout(timeout);
this.doManageReadMarkers();
}
};

/*
* Debounced function to manage read markers because we don't need to
* do this on every tiny scroll update. It also sets state which causes
* a component update, which can in turn reset the scroll position, so
* it's important we allow the browser to scroll a bit before running this
* (hence trailing edge only and debounce rather than throttle because
* we really only need to update this once the user has finished scrolling,
* not periodically while they scroll).
*/
private doManageReadMarkers = debounce(() => {
const rmPosition = this.getReadMarkerPosition();
// we hide the read marker when it first comes onto the screen, but if
// it goes back off the top of the screen (presumably because the user
// clicks on the 'jump to bottom' button), we need to re-enable it.
if (rmPosition < 0) {
this.setState({ readMarkerVisible: true });
}

// if read marker position goes between 0 and -1/1,
// (and user is active), switch timeout
const timeout = this.readMarkerTimeout(rmPosition);
// NO-OP when timeout already has set to the given value
this.readMarkerActivityTimer.changeTimeout(timeout);
}, READ_MARKER_DEBOUNCE_MS, { leading: false, trailing: true });

private onAction = (payload: ActionPayload): void => {
switch (payload.action) {
case "ignore_state_changed":
Expand Down