Skip to content

Commit 5970b58

Browse files
fix chat jump/mark read/header issues (#28615)
1 parent 66731be commit 5970b58

File tree

5 files changed

+74
-46
lines changed

5 files changed

+74
-46
lines changed

shared/chat/conversation/list-area/hooks.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ export const useActions = (p: {conversationIDKey: T.Chat.ConversationIDKey}) =>
1212
logger.info('mark intially as read bail on not looking at this thread anymore?')
1313
return
1414
}
15-
C.getConvoState(conversationIDKey).dispatch.markThreadAsRead()
15+
// Force mark as read since this is triggered by navigation (user action)
16+
C.getConvoState(conversationIDKey).dispatch.markThreadAsRead(true)
1617
}, [conversationIDKey])
1718

1819
return {markInitiallyLoadedThreadAsRead}

shared/chat/conversation/list-area/index.desktop.tsx

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -222,35 +222,39 @@ const useScrolling = (p: {
222222
}
223223
}, [cleanupDebounced])
224224

225-
const initScrollRef = React.useRef(false)
226225
const [didFirstLoad, setDidFirstLoad] = React.useState(false)
227226

228-
// handle initial load scrolling only. Either scroll to bottom or scroll to centered
229-
React.useLayoutEffect(() => {
230-
if (!loaded) return
231-
if (!initScrollRef.current) {
232-
initScrollRef.current = true
233-
if (!markedInitiallyLoaded) {
234-
markedInitiallyLoaded = true
235-
markInitiallyLoadedThreadAsRead()
236-
}
227+
// Ensure didFirstLoad is true whenever we're loaded (even if we skipped reload)
228+
React.useEffect(() => {
229+
if (loaded && !didFirstLoad) {
230+
requestAnimationFrame(() => {
231+
setDidFirstLoad(true)
232+
})
233+
}
234+
}, [loaded, didFirstLoad])
237235

238-
if (centeredOrdinal) {
239-
lockedToBottomRef.current = false
240-
scrollToCentered()
241-
} else {
242-
scrollToBottom()
243-
}
236+
// Handle scrolling when loaded becomes true. Scroll to centered ordinal if present, else bottom
237+
const prevLoadedRef = React.useRef(loaded)
238+
React.useLayoutEffect(() => {
239+
const justLoaded = loaded && !prevLoadedRef.current
240+
prevLoadedRef.current = loaded
241+
242+
if (!justLoaded) return
243+
244+
if (!markedInitiallyLoaded) {
245+
markedInitiallyLoaded = true
246+
markInitiallyLoadedThreadAsRead()
244247
}
245248

246-
requestAnimationFrame(() => {
247-
setDidFirstLoad(true)
248-
})
249+
if (centeredOrdinal) {
250+
lockedToBottomRef.current = false
251+
scrollToCentered()
252+
} else {
253+
scrollToBottom()
254+
}
249255
}, [
250-
listRef,
251256
loaded,
252257
centeredOrdinal,
253-
isLockedToBottom,
254258
markInitiallyLoadedThreadAsRead,
255259
scrollToBottom,
256260
scrollToCentered,
@@ -292,26 +296,27 @@ const useScrolling = (p: {
292296
// we want this to fire when the ordinals change
293297
}, [centeredOrdinal, ordinalsLength, isLockedToBottom, isMounted, listRef, firstOrdinal])
294298

295-
// Check to see if our centered ordinal has changed, and if so, scroll to it
296-
const [lastCenteredOrdinal, setLastCenteredOrdinal] = React.useState(centeredOrdinal)
299+
// Also handle centered ordinal changing while already loaded (e.g. from thread search results)
300+
const prevCenteredOrdinal = React.useRef(centeredOrdinal)
301+
const wasLoadedRef = React.useRef(loaded)
297302
React.useEffect(() => {
298-
if (lastCenteredOrdinal === centeredOrdinal) return
303+
const wasLoaded = wasLoadedRef.current
304+
const changed = prevCenteredOrdinal.current !== centeredOrdinal
305+
prevCenteredOrdinal.current = centeredOrdinal
306+
wasLoadedRef.current = loaded
307+
308+
// Only scroll if we were already loaded and ordinal changed
309+
// (the load effect handles scrolling when loaded transitions to true)
310+
if (!wasLoaded || !loaded || !changed) return
311+
299312
if (centeredOrdinal) {
300313
lockedToBottomRef.current = false
301314
scrollToCentered()
302-
} else if (lastCenteredOrdinal && !centeredOrdinal && containsLatestMessage) {
315+
} else if (containsLatestMessage) {
303316
lockedToBottomRef.current = true
304317
scrollToBottom()
305318
}
306-
setLastCenteredOrdinal(centeredOrdinal)
307-
}, [
308-
centeredOrdinal,
309-
scrollToCentered,
310-
setLastCenteredOrdinal,
311-
lastCenteredOrdinal,
312-
containsLatestMessage,
313-
scrollToBottom,
314-
])
319+
}, [centeredOrdinal, loaded, containsLatestMessage, scrollToCentered, scrollToBottom])
315320

316321
const {setScrollRef} = React.useContext(ScrollContext)
317322
React.useEffect(() => {

shared/chat/conversation/search.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ const useCommon = (ownProps: OwnProps) => {
109109
if (hasHits && !hadHitsRef.current) {
110110
hadHitsRef.current = true
111111
selectResult(0)
112+
} else if (!hasHits) {
113+
hadHitsRef.current = false
112114
}
113115
}, [hasHits, selectResult])
114116

shared/chat/inbox-and-conversation-header.tsx

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ const Header2 = () => {
251251
/>
252252
{description ? (
253253
<>
254-
<Kb.Text type="BodySmall" style={styles.desc}>
254+
<Kb.Text type="BodySmall" style={styles.descDot}>
255255
&nbsp;•&nbsp;
256256
</Kb.Text>
257257
{description}
@@ -271,15 +271,17 @@ const Header2 = () => {
271271
<Kb.Box2
272272
direction="horizontal"
273273
style={styles.right}
274-
fullHeight={!renderDescription}
274+
fullHeight={true}
275275
gap="small"
276276
alignItems="flex-end"
277277
alignSelf="flex-end"
278278
>
279-
<Kb.Box2 direction="vertical" style={styles.headerTitle}>
280-
{topRow}
281-
{bottomRow}
282-
</Kb.Box2>
279+
<Kb.BoxGrow2 style={{height: '100%'}}>
280+
<Kb.Box2 direction="vertical" style={styles.headerTitle}>
281+
{topRow}
282+
{bottomRow}
283+
</Kb.Box2>
284+
</Kb.BoxGrow2>
283285
{rightIcons}
284286
</Kb.Box2>
285287
</Kb.Box2>
@@ -298,7 +300,15 @@ const styles = Kb.Styles.styleSheetCreate(
298300
width: '100%',
299301
},
300302
desc: {
301-
...Kb.Styles.platformStyles({isElectron: {...Kb.Styles.desktopStyles.windowDraggingClickable}}),
303+
...Kb.Styles.platformStyles({
304+
isElectron: {...Kb.Styles.desktopStyles.windowDraggingClickable, width: '100%'},
305+
}),
306+
color: Kb.Styles.globalColors.black_50,
307+
},
308+
descDot: {
309+
...Kb.Styles.platformStyles({
310+
isElectron: {...Kb.Styles.desktopStyles.windowDraggingClickable},
311+
}),
302312
color: Kb.Styles.globalColors.black_50,
303313
},
304314
descriptionContainer: {
@@ -312,14 +322,15 @@ const styles = Kb.Styles.styleSheetCreate(
312322
descriptionTooltip: {alignItems: 'flex-start'},
313323
headerTitle: Kb.Styles.platformStyles({
314324
common: {
315-
flexGrow: 1,
316325
paddingBottom: Kb.Styles.globalMargins.xtiny,
326+
width: '100%',
317327
},
318328
isElectron: Kb.Styles.desktopStyles.windowDraggingClickable,
319329
isTablet: {flex: 1},
320330
}),
321331
left: Kb.Styles.platformStyles({
322332
common: {
333+
flexShrink: 0,
323334
height: Kb.Styles.isTablet ? 36 : 32,
324335
width: Kb.Styles.globalStyles.mediumSubNavWidth,
325336
},

shared/constants/chat2/convostate.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ export interface ConvoState extends ConvoStore {
225225
loadNewerMessagesDueToScroll: (numOrdinals: number) => void
226226
loadMoreMessages: DebouncedFunc<(p: LoadMoreMessagesParams) => void>
227227
loadNextAttachment: (from: T.Chat.Ordinal, backInTime: boolean) => Promise<T.Chat.Ordinal>
228-
markThreadAsRead: (unreadLineMessageID?: number) => void
228+
markThreadAsRead: (force?: boolean) => void
229229
markTeamAsRead: (teamID: T.Teams.TeamID) => void
230230
messageAttachmentNativeSave: (ordinal: T.Chat.Ordinal) => void
231231
messageAttachmentNativeShare: (ordinal: T.Chat.Ordinal) => void
@@ -1594,6 +1594,15 @@ const createSlice: Z.ImmerStateCreator<ConvoState> = (set, get) => {
15941594
})
15951595
}
15961596
}
1597+
1598+
// Force mark as read for user-initiated navigations (not auto-selection by service)
1599+
const isUserNavigation =
1600+
reason !== 'findNewestConversation' &&
1601+
reason !== 'findNewestConversationFromLayout' &&
1602+
reason !== 'tab selected'
1603+
if (isUserNavigation) {
1604+
get().dispatch.markThreadAsRead(true)
1605+
}
15971606
}
15981607

15991608
const pagination = messageIDControl ? null : scrollDirectionToPagination(sd, numberOfMessagesToLoad)
@@ -1743,7 +1752,7 @@ const createSlice: Z.ImmerStateCreator<ConvoState> = (set, get) => {
17431752
}
17441753
C.ignorePromise(f())
17451754
},
1746-
markThreadAsRead: () => {
1755+
markThreadAsRead: force => {
17471756
const f = async () => {
17481757
if (!C.useConfigState.getState().loggedIn) {
17491758
logger.info('mark read bail on not logged in')
@@ -1754,7 +1763,7 @@ const createSlice: Z.ImmerStateCreator<ConvoState> = (set, get) => {
17541763
logger.info('mark read bail on no selected conversation')
17551764
return
17561765
}
1757-
if (!Common.isUserActivelyLookingAtThisThread(conversationIDKey)) {
1766+
if (!force && !Common.isUserActivelyLookingAtThisThread(conversationIDKey)) {
17581767
logger.info('mark read bail on not looking at this thread')
17591768
return
17601769
}

0 commit comments

Comments
 (0)