Timeline: When scrolling far into history, discard newest messages

This commit is contained in:
Scott Nonnenberg 2022-05-10 13:19:58 -07:00 committed by GitHub
parent 0ca66d6e95
commit 6b4bea6330
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 173 additions and 35 deletions

View file

@ -33,12 +33,12 @@ import type { PropsType as SmartContactSpoofingReviewDialogPropsType } from '../
import type { GroupNameCollisionsWithIdsByTitle } from '../../util/groupMemberNameCollisions'; import type { GroupNameCollisionsWithIdsByTitle } from '../../util/groupMemberNameCollisions';
import { hasUnacknowledgedCollisions } from '../../util/groupMemberNameCollisions'; import { hasUnacknowledgedCollisions } from '../../util/groupMemberNameCollisions';
import { TimelineFloatingHeader } from './TimelineFloatingHeader'; import { TimelineFloatingHeader } from './TimelineFloatingHeader';
import type { TimelineMessageLoadingState } from '../../util/timelineUtil';
import { import {
ScrollAnchor,
UnreadIndicatorPlacement,
getScrollAnchorBeforeUpdate, getScrollAnchorBeforeUpdate,
getWidthBreakpoint, getWidthBreakpoint,
ScrollAnchor,
TimelineMessageLoadingState,
UnreadIndicatorPlacement,
} from '../../util/timelineUtil'; } from '../../util/timelineUtil';
import { import {
getScrollBottom, getScrollBottom,
@ -53,6 +53,7 @@ const AT_BOTTOM_DETECTOR_STYLE = { height: AT_BOTTOM_THRESHOLD };
const MIN_ROW_HEIGHT = 18; const MIN_ROW_HEIGHT = 18;
const SCROLL_DOWN_BUTTON_THRESHOLD = 8; const SCROLL_DOWN_BUTTON_THRESHOLD = 8;
const LOAD_NEWER_THRESHOLD = 5;
export type WarningType = export type WarningType =
| { | {
@ -109,7 +110,13 @@ type PropsHousekeepingType = {
contactSpoofingReview?: ContactSpoofingReviewPropType; contactSpoofingReview?: ContactSpoofingReviewPropType;
discardMessages: ( discardMessages: (
_: Readonly<{ conversationId: string; numberToKeepAtBottom: number }> _: Readonly<
| {
conversationId: string;
numberToKeepAtBottom: number;
}
| { conversationId: string; numberToKeepAtTop: number }
>
) => void; ) => void;
getTimestampForMessage: (messageId: string) => undefined | number; getTimestampForMessage: (messageId: string) => undefined | number;
getPreferredBadge: PreferredBadgeSelectorType; getPreferredBadge: PreferredBadgeSelectorType;
@ -487,10 +494,15 @@ export class Timeline extends React.Component<
if (newestBottomVisibleMessageId) { if (newestBottomVisibleMessageId) {
this.markNewestBottomVisibleMessageRead(); this.markNewestBottomVisibleMessageRead();
const rowIndex = getRowIndexFromElement(newestBottomVisible);
const maxRowIndex = items.length - 1;
if ( if (
!messageLoadingState && !messageLoadingState &&
!haveNewest && !haveNewest &&
newestBottomVisibleMessageId === last(items) isNumber(rowIndex) &&
maxRowIndex >= 0 &&
rowIndex >= maxRowIndex - LOAD_NEWER_THRESHOLD
) { ) {
loadNewerMessages(newestBottomVisibleMessageId); loadNewerMessages(newestBottomVisibleMessageId);
} }
@ -633,8 +645,16 @@ export class Timeline extends React.Component<
_prevState: Readonly<StateType>, _prevState: Readonly<StateType>,
snapshot: Readonly<SnapshotType> snapshot: Readonly<SnapshotType>
): void { ): void {
const { items: oldItems } = prevProps; const {
const { discardMessages, id, items: newItems } = this.props; items: oldItems,
messageLoadingState: previousMessageLoadingState,
} = prevProps;
const {
discardMessages,
id,
items: newItems,
messageLoadingState,
} = this.props;
const containerEl = this.containerRef.current; const containerEl = this.containerRef.current;
if (containerEl && snapshot) { if (containerEl && snapshot) {
@ -662,12 +682,33 @@ export class Timeline extends React.Component<
this.updateIntersectionObserver(); this.updateIntersectionObserver();
// This condition is somewhat arbitrary. // This condition is somewhat arbitrary.
const numberToKeepAtBottom = this.maxVisibleRows * 2;
const shouldDiscardOlderMessages: boolean = const shouldDiscardOlderMessages: boolean =
this.isAtBottom() && newItems.length >= this.maxVisibleRows * 1.5; this.isAtBottom() && newItems.length > numberToKeepAtBottom;
if (shouldDiscardOlderMessages) { if (shouldDiscardOlderMessages) {
discardMessages({ discardMessages({
conversationId: id, conversationId: id,
numberToKeepAtBottom: this.maxVisibleRows, numberToKeepAtBottom,
});
}
const loadingStateThatJustFinished:
| undefined
| TimelineMessageLoadingState =
!messageLoadingState && previousMessageLoadingState
? previousMessageLoadingState
: undefined;
const numberToKeepAtTop = this.maxVisibleRows * 5;
const shouldDiscardNewerMessages: boolean =
!this.isAtBottom() &&
loadingStateThatJustFinished ===
TimelineMessageLoadingState.LoadingOlderMessages &&
newItems.length > numberToKeepAtTop;
if (shouldDiscardNewerMessages) {
discardMessages({
conversationId: id,
numberToKeepAtTop,
}); });
} }
} else { } else {
@ -1195,6 +1236,14 @@ function getMessageIdFromElement(
return element instanceof HTMLElement ? element.dataset.messageId : undefined; return element instanceof HTMLElement ? element.dataset.messageId : undefined;
} }
function getRowIndexFromElement(
element: undefined | Element
): undefined | number {
return element instanceof HTMLElement && element.dataset.itemIndex
? parseInt(element.dataset.itemIndex, 10)
: undefined;
}
function showDebugLog() { function showDebugLog() {
window.showDebugLog(); window.showDebugLog();
} }

View file

@ -80,6 +80,7 @@ import type { NoopActionType } from './noop';
import { conversationJobQueue } from '../../jobs/conversationJobQueue'; import { conversationJobQueue } from '../../jobs/conversationJobQueue';
import type { TimelineMessageLoadingState } from '../../util/timelineUtil'; import type { TimelineMessageLoadingState } from '../../util/timelineUtil';
import { isGroup } from '../../util/whatTypeOfConversation'; import { isGroup } from '../../util/whatTypeOfConversation';
import { missingCaseError } from '../../util/missingCaseError';
// State // State
@ -465,7 +466,13 @@ type CustomColorRemovedActionType = {
}; };
type DiscardMessagesActionType = { type DiscardMessagesActionType = {
type: typeof DISCARD_MESSAGES; type: typeof DISCARD_MESSAGES;
payload: { conversationId: string; numberToKeepAtBottom: number }; payload: Readonly<
| {
conversationId: string;
numberToKeepAtBottom: number;
}
| { conversationId: string; numberToKeepAtTop: number }
>;
}; };
type SetPreJoinConversationActionType = { type SetPreJoinConversationActionType = {
type: 'SET_PRE_JOIN_CONVERSATION'; type: 'SET_PRE_JOIN_CONVERSATION';
@ -2195,35 +2202,70 @@ export function reducer(
} }
if (action.type === DISCARD_MESSAGES) { if (action.type === DISCARD_MESSAGES) {
const { conversationId, numberToKeepAtBottom } = action.payload; const { conversationId } = action.payload;
if ('numberToKeepAtBottom' in action.payload) {
const { numberToKeepAtBottom } = action.payload;
const conversationMessages = getOwn(
state.messagesByConversation,
conversationId
);
if (!conversationMessages) {
return state;
}
const conversationMessages = getOwn( const { messageIds: oldMessageIds } = conversationMessages;
state.messagesByConversation, if (oldMessageIds.length <= numberToKeepAtBottom) {
conversationId return state;
); }
if (!conversationMessages) {
return state;
}
const { messageIds: oldMessageIds } = conversationMessages; const messageIdsToRemove = oldMessageIds.slice(0, -numberToKeepAtBottom);
if (oldMessageIds.length <= numberToKeepAtBottom) { const messageIdsToKeep = oldMessageIds.slice(-numberToKeepAtBottom);
return state;
}
const messageIdsToRemove = oldMessageIds.slice(0, -numberToKeepAtBottom); return {
const messageIdsToKeep = oldMessageIds.slice(-numberToKeepAtBottom); ...state,
messagesLookup: omit(state.messagesLookup, messageIdsToRemove),
return { messagesByConversation: {
...state, ...state.messagesByConversation,
messagesLookup: omit(state.messagesLookup, messageIdsToRemove), [conversationId]: {
messagesByConversation: { ...conversationMessages,
...state.messagesByConversation, messageIds: messageIdsToKeep,
[conversationId]: { },
...conversationMessages,
messageIds: messageIdsToKeep,
}, },
}, };
}; }
if ('numberToKeepAtTop' in action.payload) {
const { numberToKeepAtTop } = action.payload;
const conversationMessages = getOwn(
state.messagesByConversation,
conversationId
);
if (!conversationMessages) {
return state;
}
const { messageIds: oldMessageIds } = conversationMessages;
if (oldMessageIds.length <= numberToKeepAtTop) {
return state;
}
const messageIdsToRemove = oldMessageIds.slice(numberToKeepAtTop);
const messageIdsToKeep = oldMessageIds.slice(0, numberToKeepAtTop);
return {
...state,
messagesLookup: omit(state.messagesLookup, messageIdsToRemove),
messagesByConversation: {
...state.messagesByConversation,
[conversationId]: {
...conversationMessages,
messageIds: messageIdsToKeep,
},
},
};
}
throw missingCaseError(action.payload);
} }
if (action.type === 'SET_PRE_JOIN_CONVERSATION') { if (action.type === 'SET_PRE_JOIN_CONVERSATION') {

View file

@ -54,6 +54,7 @@ const {
closeRecommendedGroupSizeModal, closeRecommendedGroupSizeModal,
conversationStoppedByMissingVerification, conversationStoppedByMissingVerification,
createGroup, createGroup,
discardMessages,
openConversationInternal, openConversationInternal,
repairNewestMessage, repairNewestMessage,
repairOldestMessage, repairOldestMessage,
@ -1306,6 +1307,52 @@ describe('both/state/ducks/conversations', () => {
}); });
}); });
describe('DISCARD_MESSAGES', () => {
const startState: ConversationsStateType = {
...getEmptyState(),
messagesLookup: {
[messageId]: getDefaultMessage(messageId),
[messageIdTwo]: getDefaultMessage(messageIdTwo),
[messageIdThree]: getDefaultMessage(messageIdThree),
},
messagesByConversation: {
[conversationId]: {
metrics: {
totalUnseen: 0,
},
scrollToMessageCounter: 0,
messageIds: [messageId, messageIdTwo, messageIdThree],
},
},
};
it('eliminates older messages', () => {
const toDiscard = {
conversationId,
numberToKeepAtBottom: 2,
};
const state = reducer(startState, discardMessages(toDiscard));
assert.deepEqual(
state.messagesByConversation[conversationId]?.messageIds,
[messageIdTwo, messageIdThree]
);
});
it('eliminates newer messages', () => {
const toDiscard = {
conversationId,
numberToKeepAtTop: 2,
};
const state = reducer(startState, discardMessages(toDiscard));
assert.deepEqual(
state.messagesByConversation[conversationId]?.messageIds,
[messageId, messageIdTwo]
);
});
});
describe('SET_PRE_JOIN_CONVERSATION', () => { describe('SET_PRE_JOIN_CONVERSATION', () => {
const startState = { const startState = {
...getEmptyState(), ...getEmptyState(),