fix(team-messages): scope debounced sidebar scroll persistence to its team
A scroll within the 100ms persist debounce could otherwise have its queued update fire after the user switched teams, writing the previous team's offset under the new team and restoring the sidebar to the wrong position. Capture the team that scheduled the update and drop it on fire if the active team changed.
This commit is contained in:
parent
816ff210b7
commit
b9cfdb9323
2 changed files with 101 additions and 0 deletions
|
|
@ -507,6 +507,9 @@ export const MessagesPanel = memo(function MessagesPanel({
|
||||||
);
|
);
|
||||||
const messagesScrollTopRef = useRef(initialSidebarStateRef.current.messagesScrollTop);
|
const messagesScrollTopRef = useRef(initialSidebarStateRef.current.messagesScrollTop);
|
||||||
const messagesScrollPersistTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
const messagesScrollPersistTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||||
|
// Tracks which team the pending scroll persistence belongs to, so a debounced update
|
||||||
|
// scheduled before a team switch is never applied to or persisted under the new team.
|
||||||
|
const messagesScrollPersistTeamRef = useRef(teamName);
|
||||||
const [bottomSheetSnapIndex, setBottomSheetSnapIndex] = useState(
|
const [bottomSheetSnapIndex, setBottomSheetSnapIndex] = useState(
|
||||||
initialSidebarStateRef.current.bottomSheetSnapIndex
|
initialSidebarStateRef.current.bottomSheetSnapIndex
|
||||||
);
|
);
|
||||||
|
|
@ -522,6 +525,7 @@ export const MessagesPanel = memo(function MessagesPanel({
|
||||||
setMessagesSearchBarVisible(initialSidebarStateRef.current.messagesSearchBarVisible);
|
setMessagesSearchBarVisible(initialSidebarStateRef.current.messagesSearchBarVisible);
|
||||||
setExpandedItemKey(initialSidebarStateRef.current.expandedItemKey);
|
setExpandedItemKey(initialSidebarStateRef.current.expandedItemKey);
|
||||||
messagesScrollTopRef.current = initialSidebarStateRef.current.messagesScrollTop;
|
messagesScrollTopRef.current = initialSidebarStateRef.current.messagesScrollTop;
|
||||||
|
messagesScrollPersistTeamRef.current = teamName;
|
||||||
setMessagesScrollTop(initialSidebarStateRef.current.messagesScrollTop);
|
setMessagesScrollTop(initialSidebarStateRef.current.messagesScrollTop);
|
||||||
setBottomSheetSnapIndex(initialSidebarStateRef.current.bottomSheetSnapIndex);
|
setBottomSheetSnapIndex(initialSidebarStateRef.current.bottomSheetSnapIndex);
|
||||||
}, [teamName]);
|
}, [teamName]);
|
||||||
|
|
@ -551,11 +555,17 @@ export const MessagesPanel = memo(function MessagesPanel({
|
||||||
|
|
||||||
const persistMessagesScrollTop = useCallback((nextScrollTop: number): void => {
|
const persistMessagesScrollTop = useCallback((nextScrollTop: number): void => {
|
||||||
messagesScrollTopRef.current = nextScrollTop;
|
messagesScrollTopRef.current = nextScrollTop;
|
||||||
|
const scheduledTeamName = messagesScrollPersistTeamRef.current;
|
||||||
if (messagesScrollPersistTimerRef.current) {
|
if (messagesScrollPersistTimerRef.current) {
|
||||||
clearTimeout(messagesScrollPersistTimerRef.current);
|
clearTimeout(messagesScrollPersistTimerRef.current);
|
||||||
}
|
}
|
||||||
messagesScrollPersistTimerRef.current = setTimeout(() => {
|
messagesScrollPersistTimerRef.current = setTimeout(() => {
|
||||||
messagesScrollPersistTimerRef.current = null;
|
messagesScrollPersistTimerRef.current = null;
|
||||||
|
// Drop a queued update that outlived a team switch: it carries the previous team's
|
||||||
|
// offset and must not overwrite the scroll state the new team just restored.
|
||||||
|
if (messagesScrollPersistTeamRef.current !== scheduledTeamName) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
setMessagesScrollTop((current) =>
|
setMessagesScrollTop((current) =>
|
||||||
Math.abs(current - messagesScrollTopRef.current) < 1
|
Math.abs(current - messagesScrollTopRef.current) < 1
|
||||||
? current
|
? current
|
||||||
|
|
|
||||||
|
|
@ -570,6 +570,97 @@ describe('MessagesPanel idle summary invariants', () => {
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('flushes a pending scroll to the previous team without leaking it when switching teams mid-debounce', async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
|
||||||
|
const host = document.createElement('div');
|
||||||
|
document.body.appendChild(host);
|
||||||
|
const root = createRoot(host);
|
||||||
|
|
||||||
|
const messagesState = {
|
||||||
|
canonicalMessages: [makeMessage({ messageId: 'm-1', text: 'hello' })],
|
||||||
|
optimisticMessages: [],
|
||||||
|
feedRevision: 'rev-1',
|
||||||
|
nextCursor: null,
|
||||||
|
hasMore: false,
|
||||||
|
lastFetchedAt: Date.now(),
|
||||||
|
loadingHead: false,
|
||||||
|
loadingOlder: false,
|
||||||
|
headHydrated: true,
|
||||||
|
};
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
storeState.teamMessagesByName['atlas-hq'] = messagesState;
|
||||||
|
storeState.teamMessagesByName['beta-hq'] = messagesState;
|
||||||
|
root.render(
|
||||||
|
React.createElement(MessagesPanel, {
|
||||||
|
teamName: 'atlas-hq',
|
||||||
|
position: 'sidebar',
|
||||||
|
onPositionChange: vi.fn(),
|
||||||
|
members: [],
|
||||||
|
tasks: [],
|
||||||
|
timeWindow: null,
|
||||||
|
pendingRepliesByMember: {},
|
||||||
|
onPendingReplyChange: vi.fn(),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
const scrollContainer = host.querySelector('.overflow-y-auto') as HTMLDivElement | null;
|
||||||
|
expect(scrollContainer).not.toBeNull();
|
||||||
|
|
||||||
|
// Scroll, leaving the 100ms persist debounce pending (do not advance timers yet).
|
||||||
|
await act(async () => {
|
||||||
|
scrollContainer!.scrollTop = 320;
|
||||||
|
scrollContainer!.dispatchEvent(new Event('scroll', { bubbles: true }));
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mocked(setTeamMessagesSidebarUiState).mockClear();
|
||||||
|
|
||||||
|
// Switch teams while the debounced scroll update is still queued.
|
||||||
|
await act(async () => {
|
||||||
|
root.render(
|
||||||
|
React.createElement(MessagesPanel, {
|
||||||
|
teamName: 'beta-hq',
|
||||||
|
position: 'sidebar',
|
||||||
|
onPositionChange: vi.fn(),
|
||||||
|
members: [],
|
||||||
|
tasks: [],
|
||||||
|
timeWindow: null,
|
||||||
|
pendingRepliesByMember: {},
|
||||||
|
onPendingReplyChange: vi.fn(),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
vi.advanceTimersByTime(100);
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
// The pending offset is flushed to the team that actually owned it...
|
||||||
|
expect(setTeamMessagesSidebarUiState).toHaveBeenCalledWith(
|
||||||
|
'atlas-hq',
|
||||||
|
expect.objectContaining({ messagesScrollTop: 320 })
|
||||||
|
);
|
||||||
|
// ...and is never persisted under the team we switched into.
|
||||||
|
const leakedToNewTeam = vi
|
||||||
|
.mocked(setTeamMessagesSidebarUiState)
|
||||||
|
.mock.calls.some(
|
||||||
|
([name, state]) =>
|
||||||
|
name === 'beta-hq' && (state as { messagesScrollTop?: number }).messagesScrollTop === 320
|
||||||
|
);
|
||||||
|
expect(leakedToNewTeam).toBe(false);
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
root.unmount();
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it('hides passive peer summaries by default while unread badge only counts filtered unread messages', async () => {
|
it('hides passive peer summaries by default while unread badge only counts filtered unread messages', async () => {
|
||||||
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
|
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
|
||||||
const host = document.createElement('div');
|
const host = document.createElement('div');
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue