From d4de93e4204c431181c6e104a30eabf58577e34b Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Mon, 7 Feb 2022 13:03:35 -0800 Subject: [PATCH] Fix timeline crash when deleting the oldest visible message Co-authored-by: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> --- ts/components/conversation/Timeline.tsx | 39 ++++++++++++++----------- ts/state/smart/Timeline.tsx | 9 ++---- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/ts/components/conversation/Timeline.tsx b/ts/components/conversation/Timeline.tsx index a6f15ffdc..872fc9e16 100644 --- a/ts/components/conversation/Timeline.tsx +++ b/ts/components/conversation/Timeline.tsx @@ -115,7 +115,7 @@ type PropsHousekeepingType = { warning?: WarningType; contactSpoofingReview?: ContactSpoofingReviewPropType; - getTimestampForMessage: (_: string) => number; + getTimestampForMessage: (messageId: string) => undefined | number; getPreferredBadge: PreferredBadgeSelectorType; i18n: LocalizerType; theme: ThemeType; @@ -220,7 +220,7 @@ type StateType = { oneTimeScrollRow?: number; visibleRows?: { newestFullyVisible?: VisibleRowType; - oldestPartiallyVisible?: VisibleRowType; + oldestPartiallyVisibleMessageId?: string; oldestFullyVisible?: VisibleRowType; }; @@ -612,7 +612,7 @@ export class Timeline extends React.PureComponent { } let newestFullyVisible: undefined | VisibleRowType; - let oldestPartiallyVisible: undefined | VisibleRowType; + let oldestPartiallyVisibleMessageId: undefined | string; let oldestFullyVisible: undefined | VisibleRowType; const { children } = innerScrollContainer; @@ -646,20 +646,18 @@ export class Timeline extends React.PureComponent { continue; } - const thisRow = { - offsetTop, - row: parseInt(child.getAttribute('data-row') || '-1', 10), - id, - }; - const bottom = offsetTop + offsetHeight; - if (bottom >= visibleTop && !oldestPartiallyVisible) { - oldestPartiallyVisible = thisRow; + if (bottom >= visibleTop && !oldestPartiallyVisibleMessageId) { + oldestPartiallyVisibleMessageId = id; } if (offsetTop + AT_TOP_THRESHOLD >= visibleTop) { - oldestFullyVisible = thisRow; + oldestFullyVisible = { + offsetTop, + row: parseInt(child.getAttribute('data-row') || '-1', 10), + id, + }; break; } } @@ -667,7 +665,7 @@ export class Timeline extends React.PureComponent { this.setState(oldState => { const visibleRows = { newestFullyVisible, - oldestPartiallyVisible, + oldestPartiallyVisibleMessageId, oldestFullyVisible, }; @@ -1289,8 +1287,15 @@ export class Timeline extends React.PureComponent { } let floatingHeader: ReactNode; - const oldestPartiallyVisibleRow = visibleRows?.oldestPartiallyVisible; - if (oldestPartiallyVisibleRow) { + const oldestPartiallyVisibleMessageId = + visibleRows?.oldestPartiallyVisibleMessageId; + // It's possible that a message was removed from `items` but we still have its ID in + // state. `getTimestampForMessage` might return undefined in that case. + const oldestPartiallyVisibleMessageTimestamp = + oldestPartiallyVisibleMessageId + ? getTimestampForMessage(oldestPartiallyVisibleMessageId) + : undefined; + if (oldestPartiallyVisibleMessageTimestamp) { floatingHeader = ( { ? { marginTop: lastMeasuredWarningHeight } : undefined } - timestamp={getTimestampForMessage(oldestPartiallyVisibleRow.id)} + timestamp={oldestPartiallyVisibleMessageTimestamp} visible={ (hasRecentlyScrolled || isLoadingMessages) && - (!haveOldest || oldestPartiallyVisibleRow.id !== items[0]) + (!haveOldest || oldestPartiallyVisibleMessageId !== items[0]) } /> ); diff --git a/ts/state/smart/Timeline.tsx b/ts/state/smart/Timeline.tsx index e4db61b92..04d0ee16b 100644 --- a/ts/state/smart/Timeline.tsx +++ b/ts/state/smart/Timeline.tsx @@ -38,7 +38,7 @@ import { renderEmojiPicker } from './renderEmojiPicker'; import { renderReactionPicker } from './renderReactionPicker'; import { getOwn } from '../../util/getOwn'; -import { assert, strictAssert } from '../../util/assert'; +import { assert } from '../../util/assert'; import { missingCaseError } from '../../util/missingCaseError'; import { getGroupMemberships } from '../../util/getGroupMemberships'; import { @@ -295,11 +295,8 @@ const mapStateToProps = (state: StateType, props: ExternalProps) => { const selectedMessage = getSelectedMessage(state); const messageSelector = getMessageSelector(state); - const getTimestampForMessage = (messageId: string): number => { - const result = messageSelector(messageId)?.timestamp; - strictAssert(result, 'Expected a message'); - return result; - }; + const getTimestampForMessage = (messageId: string): undefined | number => + messageSelector(messageId)?.timestamp; return { id,