mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-03-28 17:43:05 +01:00
Fix message dual render and get the jump to unread link out of the way on visible unread boundaries. Closes #57.
This commit is contained in:
@@ -204,11 +204,9 @@ export function MessageList({
|
||||
const resendTimersRef = useRef<Map<number, ReturnType<typeof setTimeout>>>(new Map());
|
||||
const [highlightedMessageId, setHighlightedMessageId] = useState<number | null>(null);
|
||||
const [showJumpToUnread, setShowJumpToUnread] = useState(false);
|
||||
const [jumpToUnreadDismissed, setJumpToUnreadDismissed] = useState(false);
|
||||
const targetScrolledRef = useRef(false);
|
||||
const unreadMarkerRef = useRef<HTMLButtonElement | HTMLDivElement | null>(null);
|
||||
const setUnreadMarkerElement = useCallback((node: HTMLButtonElement | HTMLDivElement | null) => {
|
||||
unreadMarkerRef.current = node;
|
||||
}, []);
|
||||
|
||||
// Capture scroll state in the scroll handler BEFORE any state updates
|
||||
const scrollStateRef = useRef({
|
||||
@@ -330,68 +328,6 @@ export function MessageList({
|
||||
};
|
||||
}, [messages, onResendChannelMessage]);
|
||||
|
||||
// Refs for scroll handler to read without causing callback recreation
|
||||
const onLoadOlderRef = useRef(onLoadOlder);
|
||||
const loadingOlderRef = useRef(loadingOlder);
|
||||
const hasOlderMessagesRef = useRef(hasOlderMessages);
|
||||
const onLoadNewerRef = useRef(onLoadNewer);
|
||||
const loadingNewerRef = useRef(loadingNewer);
|
||||
const hasNewerMessagesRef = useRef(hasNewerMessages);
|
||||
onLoadOlderRef.current = onLoadOlder;
|
||||
loadingOlderRef.current = loadingOlder;
|
||||
hasOlderMessagesRef.current = hasOlderMessages;
|
||||
onLoadNewerRef.current = onLoadNewer;
|
||||
loadingNewerRef.current = loadingNewer;
|
||||
hasNewerMessagesRef.current = hasNewerMessages;
|
||||
|
||||
// Handle scroll - capture state and detect when user is near top/bottom
|
||||
// Stable callback: reads changing values from refs, never recreated.
|
||||
const handleScroll = useCallback(() => {
|
||||
if (!listRef.current) return;
|
||||
|
||||
const { scrollTop, scrollHeight, clientHeight } = listRef.current;
|
||||
const distanceFromBottom = scrollHeight - scrollTop - clientHeight;
|
||||
|
||||
// Always capture current scroll state (needed for scroll preservation)
|
||||
scrollStateRef.current = {
|
||||
scrollTop,
|
||||
scrollHeight,
|
||||
clientHeight,
|
||||
wasNearTop: scrollTop < 150,
|
||||
wasNearBottom: distanceFromBottom < 100,
|
||||
};
|
||||
|
||||
// Show scroll-to-bottom button when not near the bottom (more than 100px away)
|
||||
setShowScrollToBottom(distanceFromBottom > 100);
|
||||
|
||||
if (!onLoadOlderRef.current || loadingOlderRef.current || !hasOlderMessagesRef.current) {
|
||||
// skip older load
|
||||
} else if (scrollTop < 100) {
|
||||
onLoadOlderRef.current();
|
||||
}
|
||||
|
||||
// Trigger load newer when within 100px of bottom
|
||||
if (
|
||||
onLoadNewerRef.current &&
|
||||
!loadingNewerRef.current &&
|
||||
hasNewerMessagesRef.current &&
|
||||
distanceFromBottom < 100
|
||||
) {
|
||||
onLoadNewerRef.current();
|
||||
}
|
||||
}, []);
|
||||
|
||||
// Scroll to bottom handler (or jump to bottom if viewing historical messages)
|
||||
const scrollToBottom = useCallback(() => {
|
||||
if (hasNewerMessages && onJumpToBottom) {
|
||||
onJumpToBottom();
|
||||
return;
|
||||
}
|
||||
if (listRef.current) {
|
||||
listRef.current.scrollTop = listRef.current.scrollHeight;
|
||||
}
|
||||
}, [hasNewerMessages, onJumpToBottom]);
|
||||
|
||||
// Sort messages by received_at ascending (oldest first)
|
||||
// Note: Deduplication is handled by useConversationMessages.addMessageIfNew()
|
||||
// and the database UNIQUE constraint on (type, conversation_key, text, sender_timestamp)
|
||||
@@ -408,10 +344,117 @@ export function MessageList({
|
||||
return sortedMessages.findIndex((msg) => !msg.outgoing && msg.received_at > boundary);
|
||||
}, [sortedMessages, unreadMarkerLastReadAt]);
|
||||
|
||||
const syncJumpToUnreadVisibility = useCallback(() => {
|
||||
if (unreadMarkerIndex === -1 || jumpToUnreadDismissed) {
|
||||
setShowJumpToUnread(false);
|
||||
return;
|
||||
}
|
||||
|
||||
const marker = unreadMarkerRef.current;
|
||||
const list = listRef.current;
|
||||
if (!marker || !list) {
|
||||
setShowJumpToUnread(true);
|
||||
return;
|
||||
}
|
||||
|
||||
const markerRect = marker.getBoundingClientRect();
|
||||
const listRect = list.getBoundingClientRect();
|
||||
|
||||
if (
|
||||
markerRect.width === 0 ||
|
||||
markerRect.height === 0 ||
|
||||
listRect.width === 0 ||
|
||||
listRect.height === 0
|
||||
) {
|
||||
setShowJumpToUnread(true);
|
||||
return;
|
||||
}
|
||||
|
||||
const markerVisible =
|
||||
markerRect.top >= listRect.top &&
|
||||
markerRect.bottom <= listRect.bottom &&
|
||||
markerRect.left >= listRect.left &&
|
||||
markerRect.right <= listRect.right;
|
||||
|
||||
setShowJumpToUnread(!markerVisible);
|
||||
}, [jumpToUnreadDismissed, unreadMarkerIndex]);
|
||||
|
||||
// Refs for scroll handler to read without causing callback recreation
|
||||
const onLoadOlderRef = useRef(onLoadOlder);
|
||||
const loadingOlderRef = useRef(loadingOlder);
|
||||
const hasOlderMessagesRef = useRef(hasOlderMessages);
|
||||
const onLoadNewerRef = useRef(onLoadNewer);
|
||||
const loadingNewerRef = useRef(loadingNewer);
|
||||
const hasNewerMessagesRef = useRef(hasNewerMessages);
|
||||
onLoadOlderRef.current = onLoadOlder;
|
||||
loadingOlderRef.current = loadingOlder;
|
||||
hasOlderMessagesRef.current = hasOlderMessages;
|
||||
onLoadNewerRef.current = onLoadNewer;
|
||||
loadingNewerRef.current = loadingNewer;
|
||||
hasNewerMessagesRef.current = hasNewerMessages;
|
||||
|
||||
const setUnreadMarkerElement = useCallback(
|
||||
(node: HTMLButtonElement | HTMLDivElement | null) => {
|
||||
unreadMarkerRef.current = node;
|
||||
syncJumpToUnreadVisibility();
|
||||
},
|
||||
[syncJumpToUnreadVisibility]
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
setShowJumpToUnread(unreadMarkerIndex !== -1);
|
||||
setJumpToUnreadDismissed(false);
|
||||
}, [unreadMarkerIndex]);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
syncJumpToUnreadVisibility();
|
||||
}, [messages, syncJumpToUnreadVisibility]);
|
||||
|
||||
// Handle scroll - capture state and detect when user is near top/bottom
|
||||
// Stable callback: reads changing values from refs, never recreated.
|
||||
const handleScroll = useCallback(() => {
|
||||
if (!listRef.current) return;
|
||||
|
||||
const { scrollTop, scrollHeight, clientHeight } = listRef.current;
|
||||
const distanceFromBottom = scrollHeight - scrollTop - clientHeight;
|
||||
|
||||
scrollStateRef.current = {
|
||||
scrollTop,
|
||||
scrollHeight,
|
||||
clientHeight,
|
||||
wasNearTop: scrollTop < 150,
|
||||
wasNearBottom: distanceFromBottom < 100,
|
||||
};
|
||||
|
||||
setShowScrollToBottom(distanceFromBottom > 100);
|
||||
|
||||
if (!onLoadOlderRef.current || loadingOlderRef.current || !hasOlderMessagesRef.current) {
|
||||
// skip older load
|
||||
} else if (scrollTop < 100) {
|
||||
onLoadOlderRef.current();
|
||||
}
|
||||
|
||||
if (
|
||||
onLoadNewerRef.current &&
|
||||
!loadingNewerRef.current &&
|
||||
hasNewerMessagesRef.current &&
|
||||
distanceFromBottom < 100
|
||||
) {
|
||||
onLoadNewerRef.current();
|
||||
}
|
||||
syncJumpToUnreadVisibility();
|
||||
}, [syncJumpToUnreadVisibility]);
|
||||
|
||||
// Scroll to bottom handler (or jump to bottom if viewing historical messages)
|
||||
const scrollToBottom = useCallback(() => {
|
||||
if (hasNewerMessages && onJumpToBottom) {
|
||||
onJumpToBottom();
|
||||
return;
|
||||
}
|
||||
if (listRef.current) {
|
||||
listRef.current.scrollTop = listRef.current.scrollHeight;
|
||||
}
|
||||
}, [hasNewerMessages, onJumpToBottom]);
|
||||
|
||||
// Sender info for outgoing messages (used by path modal on own messages)
|
||||
const selfSenderInfo = useMemo<SenderInfo>(
|
||||
() => ({
|
||||
@@ -841,6 +884,7 @@ export function MessageList({
|
||||
type="button"
|
||||
onClick={() => {
|
||||
unreadMarkerRef.current?.scrollIntoView?.({ block: 'center' });
|
||||
setJumpToUnreadDismissed(true);
|
||||
setShowJumpToUnread(false);
|
||||
}}
|
||||
className="pointer-events-auto h-9 rounded-full bg-card hover:bg-accent border border-border px-3 text-sm font-medium shadow-lg transition-all hover:scale-105 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
|
||||
|
||||
@@ -86,6 +86,30 @@ function isMessageConversation(conversation: Conversation | null): conversation
|
||||
return !!conversation && !['raw', 'map', 'visualizer', 'search'].includes(conversation.type);
|
||||
}
|
||||
|
||||
function appendUniqueMessages(current: Message[], incoming: Message[]): Message[] {
|
||||
if (incoming.length === 0) return current;
|
||||
|
||||
const seenIds = new Set(current.map((msg) => msg.id));
|
||||
const seenContent = new Set(current.map((msg) => getMessageContentKey(msg)));
|
||||
const additions: Message[] = [];
|
||||
|
||||
for (const msg of incoming) {
|
||||
const contentKey = getMessageContentKey(msg);
|
||||
if (seenIds.has(msg.id) || seenContent.has(contentKey)) {
|
||||
continue;
|
||||
}
|
||||
seenIds.add(msg.id);
|
||||
seenContent.add(contentKey);
|
||||
additions.push(msg);
|
||||
}
|
||||
|
||||
if (additions.length === 0) {
|
||||
return current;
|
||||
}
|
||||
|
||||
return [...current, ...additions];
|
||||
}
|
||||
|
||||
export function useConversationMessages(
|
||||
activeConversation: Conversation | null,
|
||||
targetMessageId?: number | null
|
||||
@@ -139,6 +163,7 @@ export function useConversationMessages(
|
||||
const fetchingConversationIdRef = useRef<string | null>(null);
|
||||
const latestReconcileRequestIdRef = useRef(0);
|
||||
const messagesRef = useRef<Message[]>([]);
|
||||
const loadingOlderRef = useRef(false);
|
||||
const hasOlderMessagesRef = useRef(false);
|
||||
const hasNewerMessagesRef = useRef(false);
|
||||
const prevConversationIdRef = useRef<string | null>(null);
|
||||
@@ -147,6 +172,10 @@ export function useConversationMessages(
|
||||
messagesRef.current = messages;
|
||||
}, [messages]);
|
||||
|
||||
useEffect(() => {
|
||||
loadingOlderRef.current = loadingOlder;
|
||||
}, [loadingOlder]);
|
||||
|
||||
useEffect(() => {
|
||||
hasOlderMessagesRef.current = hasOlderMessages;
|
||||
}, [hasOlderMessages]);
|
||||
@@ -252,7 +281,13 @@ export function useConversationMessages(
|
||||
);
|
||||
|
||||
const fetchOlderMessages = useCallback(async () => {
|
||||
if (!isMessageConversation(activeConversation) || loadingOlder || !hasOlderMessages) return;
|
||||
if (
|
||||
!isMessageConversation(activeConversation) ||
|
||||
loadingOlderRef.current ||
|
||||
!hasOlderMessagesRef.current
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const conversationId = activeConversation.id;
|
||||
const oldestMessage = messages.reduce(
|
||||
@@ -266,6 +301,7 @@ export function useConversationMessages(
|
||||
);
|
||||
if (!oldestMessage) return;
|
||||
|
||||
loadingOlderRef.current = true;
|
||||
setLoadingOlder(true);
|
||||
try {
|
||||
const data = await api.getMessages({
|
||||
@@ -281,9 +317,17 @@ export function useConversationMessages(
|
||||
const dataWithPendingAck = data.map((msg) => applyPendingAck(msg));
|
||||
|
||||
if (dataWithPendingAck.length > 0) {
|
||||
setMessages((prev) => [...prev, ...dataWithPendingAck]);
|
||||
for (const msg of dataWithPendingAck) {
|
||||
seenMessageContent.current.add(getMessageContentKey(msg));
|
||||
let nextMessages: Message[] | null = null;
|
||||
setMessages((prev) => {
|
||||
const merged = appendUniqueMessages(prev, dataWithPendingAck);
|
||||
if (merged !== prev) {
|
||||
nextMessages = merged;
|
||||
}
|
||||
return merged;
|
||||
});
|
||||
if (nextMessages) {
|
||||
messagesRef.current = nextMessages;
|
||||
syncSeenContent(nextMessages);
|
||||
}
|
||||
}
|
||||
setHasOlderMessages(dataWithPendingAck.length >= MESSAGE_PAGE_SIZE);
|
||||
@@ -293,9 +337,10 @@ export function useConversationMessages(
|
||||
description: err instanceof Error ? err.message : 'Check your connection',
|
||||
});
|
||||
} finally {
|
||||
loadingOlderRef.current = false;
|
||||
setLoadingOlder(false);
|
||||
}
|
||||
}, [activeConversation, applyPendingAck, hasOlderMessages, loadingOlder, messages]);
|
||||
}, [activeConversation, applyPendingAck, messages, syncSeenContent]);
|
||||
|
||||
const fetchNewerMessages = useCallback(async () => {
|
||||
if (!isMessageConversation(activeConversation) || loadingNewer || !hasNewerMessages) return;
|
||||
@@ -379,6 +424,7 @@ export function useConversationMessages(
|
||||
}
|
||||
|
||||
setLoadingOlder(false);
|
||||
loadingOlderRef.current = false;
|
||||
setLoadingNewer(false);
|
||||
if (conversationChanged) {
|
||||
setHasNewerMessages(false);
|
||||
|
||||
@@ -7,6 +7,7 @@ import { MessageList } from '../components/MessageList';
|
||||
import type { Message } from '../types';
|
||||
|
||||
const scrollIntoViewMock = vi.fn();
|
||||
const originalGetBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
|
||||
|
||||
function createMessage(overrides: Partial<Message> = {}): Message {
|
||||
return {
|
||||
@@ -35,6 +36,11 @@ describe('MessageList channel sender rendering', () => {
|
||||
value: scrollIntoViewMock,
|
||||
writable: true,
|
||||
});
|
||||
Object.defineProperty(HTMLElement.prototype, 'getBoundingClientRect', {
|
||||
configurable: true,
|
||||
value: originalGetBoundingClientRect,
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('renders explicit corrupt placeholder and warning avatar for unnamed corrupt channel packets', () => {
|
||||
@@ -155,4 +161,68 @@ describe('MessageList channel sender rendering', () => {
|
||||
expect(screen.getByText('Unread messages')).toBeInTheDocument();
|
||||
expect(scrollIntoViewMock).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('hides the jump-to-unread button when the unread marker is already visible', () => {
|
||||
Object.defineProperty(HTMLElement.prototype, 'getBoundingClientRect', {
|
||||
configurable: true,
|
||||
writable: true,
|
||||
value: function () {
|
||||
const element = this as HTMLElement;
|
||||
if (element.textContent?.includes('Unread messages')) {
|
||||
return {
|
||||
top: 200,
|
||||
bottom: 240,
|
||||
left: 0,
|
||||
right: 300,
|
||||
width: 300,
|
||||
height: 40,
|
||||
x: 0,
|
||||
y: 200,
|
||||
toJSON: () => '',
|
||||
};
|
||||
}
|
||||
if (element.className.includes('overflow-y-auto')) {
|
||||
return {
|
||||
top: 100,
|
||||
bottom: 500,
|
||||
left: 0,
|
||||
right: 400,
|
||||
width: 400,
|
||||
height: 400,
|
||||
x: 0,
|
||||
y: 100,
|
||||
toJSON: () => '',
|
||||
};
|
||||
}
|
||||
return {
|
||||
top: 0,
|
||||
bottom: 0,
|
||||
left: 0,
|
||||
right: 0,
|
||||
width: 0,
|
||||
height: 0,
|
||||
x: 0,
|
||||
y: 0,
|
||||
toJSON: () => '',
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
const messages = [
|
||||
createMessage({ id: 1, received_at: 1700000001, text: 'Alice: older' }),
|
||||
createMessage({ id: 2, received_at: 1700000010, text: 'Alice: newer' }),
|
||||
];
|
||||
|
||||
render(
|
||||
<MessageList
|
||||
messages={messages}
|
||||
contacts={[]}
|
||||
loading={false}
|
||||
unreadMarkerLastReadAt={1700000005}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText('Unread messages')).toBeInTheDocument();
|
||||
expect(screen.queryByRole('button', { name: 'Jump to unread' })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -323,6 +323,102 @@ describe('useConversationMessages background reconcile ordering', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('useConversationMessages older-page dedup and reentry', () => {
|
||||
beforeEach(() => {
|
||||
mockGetMessages.mockReset();
|
||||
messageCache.clear();
|
||||
});
|
||||
|
||||
it('prevents duplicate overlapping older-page fetches in the same tick', async () => {
|
||||
const conv: Conversation = { type: 'contact', id: 'conv_a', name: 'Contact A' };
|
||||
|
||||
const fullPage = Array.from({ length: 200 }, (_, i) =>
|
||||
createMessage({
|
||||
id: i + 1,
|
||||
conversation_key: 'conv_a',
|
||||
text: `msg-${i + 1}`,
|
||||
sender_timestamp: 1700000000 + i,
|
||||
received_at: 1700000000 + i,
|
||||
})
|
||||
);
|
||||
mockGetMessages.mockResolvedValueOnce(fullPage);
|
||||
|
||||
const olderDeferred = createDeferred<Message[]>();
|
||||
mockGetMessages.mockReturnValueOnce(olderDeferred.promise);
|
||||
|
||||
const { result } = renderHook(() => useConversationMessages(conv));
|
||||
|
||||
await waitFor(() => expect(result.current.messagesLoading).toBe(false));
|
||||
expect(result.current.messages).toHaveLength(200);
|
||||
expect(result.current.hasOlderMessages).toBe(true);
|
||||
|
||||
act(() => {
|
||||
void result.current.fetchOlderMessages();
|
||||
void result.current.fetchOlderMessages();
|
||||
});
|
||||
|
||||
expect(mockGetMessages).toHaveBeenCalledTimes(2); // initial page + one older fetch
|
||||
|
||||
olderDeferred.resolve([
|
||||
createMessage({
|
||||
id: 0,
|
||||
conversation_key: 'conv_a',
|
||||
text: 'older-msg',
|
||||
sender_timestamp: 1699999999,
|
||||
received_at: 1699999999,
|
||||
}),
|
||||
]);
|
||||
|
||||
await waitFor(() => expect(result.current.loadingOlder).toBe(false));
|
||||
expect(result.current.messages).toHaveLength(201);
|
||||
expect(result.current.messages.filter((msg) => msg.id === 0)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not append duplicate messages from an overlapping older page', async () => {
|
||||
const conv: Conversation = { type: 'contact', id: 'conv_a', name: 'Contact A' };
|
||||
|
||||
const fullPage = Array.from({ length: 200 }, (_, i) =>
|
||||
createMessage({
|
||||
id: i + 1,
|
||||
conversation_key: 'conv_a',
|
||||
text: `msg-${i + 1}`,
|
||||
sender_timestamp: 1700000000 + i,
|
||||
received_at: 1700000000 + i,
|
||||
})
|
||||
);
|
||||
mockGetMessages.mockResolvedValueOnce(fullPage);
|
||||
mockGetMessages.mockResolvedValueOnce([
|
||||
createMessage({
|
||||
id: 1,
|
||||
conversation_key: 'conv_a',
|
||||
text: 'msg-1',
|
||||
sender_timestamp: 1700000000,
|
||||
received_at: 1700000000,
|
||||
}),
|
||||
createMessage({
|
||||
id: 0,
|
||||
conversation_key: 'conv_a',
|
||||
text: 'older-msg',
|
||||
sender_timestamp: 1699999999,
|
||||
received_at: 1699999999,
|
||||
}),
|
||||
]);
|
||||
|
||||
const { result } = renderHook(() => useConversationMessages(conv));
|
||||
|
||||
await waitFor(() => expect(result.current.messagesLoading).toBe(false));
|
||||
expect(result.current.messages).toHaveLength(200);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.fetchOlderMessages();
|
||||
});
|
||||
|
||||
expect(result.current.messages.filter((msg) => msg.id === 1)).toHaveLength(1);
|
||||
expect(result.current.messages.filter((msg) => msg.id === 0)).toHaveLength(1);
|
||||
expect(result.current.messages).toHaveLength(201);
|
||||
});
|
||||
});
|
||||
|
||||
describe('useConversationMessages forward pagination', () => {
|
||||
beforeEach(() => {
|
||||
mockGetMessages.mockReset();
|
||||
|
||||
Reference in New Issue
Block a user