fix(web): code review round 2 -- messaging UI bugs and design fidelity
- DetailColumn: remark-edit draft and the history-search modal's open
state weren't reset on contact switch (no key={topicId}, same long-
lived instance) -- could silently save a typed remark against the
WRONG contact. History-search state lifted to MessagesLayout so
ConversationView's header search icon (previously wired to nothing)
can open the same modal
- ConversationView: loadOlder's in-flight "restore scroll position"
callback and its loading flag weren't guarded against a conversation
switch happening mid-request, and the scroll-to-bottom effect keyed
only on messages.length so two conversations that happened to load
the same page size never got scrolled down on open
- ChatListColumn: pinned 系统消息/文件助手 rows used solid-color-square
avatars like regular contacts; the actual design-code source
(ChatList.dc.html) and reference screenshots use a light tinted
background with a colored icon instead
- MessageBubble: own ("我") messages hash-colored their avatar like any
contact instead of the design's fixed accent blue
- Every send/recall/favorite/forward action swallowed its own failure
silently (empty .catch or, for forward, no catch at all) -- moved
Toast from features/auth into app/ (shared) and wired real error
feedback through it
- MessageContextMenu: added the recall countdown label, Escape-to-close,
and viewport-edge clamping (a right-click near the window edge could
render partially off-screen)
- EmojiPanel now defaults to 最近 per the reference (was defaulting to
表情)
- HistorySearchModal: pagination (searchTopicMessages/getTopicMedia both
supported cursor/next_cursor already, nothing used it -- silently
truncated at one page), Escape-to-close, and a brief "没有找到" flash
on the first keystroke before the debounce caught up
- A handful of design-token corrections (--shadow-btn, NavRail's border/
spacing, LoginPage's logo radius/brand-panel padding, a placeholder
icon color) verified against screens/*.dc.html's exact pixel values
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
d3a0eb0dbc
commit
c9f4615c40
@@ -5,16 +5,17 @@ import type { StoredMessage } from 'tinode-sdk';
|
||||
import { Drafty } from 'tinode-sdk';
|
||||
|
||||
import { Icon } from '@/app/Icon';
|
||||
import { Toast } from '@/app/Toast';
|
||||
import { EmojiPanel } from '@/features/messages/EmojiPanel';
|
||||
import { ForwardPickerModal } from '@/features/messages/ForwardPickerModal';
|
||||
import { MessageContextMenu } from '@/features/messages/MessageContextMenu';
|
||||
import { MessageRow } from '@/features/messages/MessageBubble';
|
||||
import { VoiceRecorder } from '@/features/messages/VoiceRecorder';
|
||||
import { useBootstrap } from '@/lib/api/hooks';
|
||||
import { favoriteMessages, recallMessage } from '@/lib/api/messages';
|
||||
import { favoriteMessages, type MediaType, recallMessage } from '@/lib/api/messages';
|
||||
import { avatarBgClass, avatarLetter } from '@/lib/avatar';
|
||||
import { FETCH_PAGE_TRIGGER, MESSAGES_PAGE } from '@/lib/config';
|
||||
import { availableActions, type MessageAction } from '@/lib/messages/actions';
|
||||
import { availableActions, recallRemainingSeconds, type MessageAction } from '@/lib/messages/actions';
|
||||
import { renderDrafty } from '@/lib/messages/drafty';
|
||||
import { sendFile, sendImage, sendVoice } from '@/lib/tinode/attachments';
|
||||
import { useChatList, useTinode, useTopicMessages } from '@/lib/tinode/hooks';
|
||||
@@ -30,6 +31,9 @@ interface OutletCtx {
|
||||
* 960-1279px regardless of the drawer's real state, always grey at >=1280px despite the panel
|
||||
* being permanently open there) rather than this actual state. */
|
||||
detailVisible: boolean;
|
||||
/** Opens the shared history-search modal (owned by MessagesLayout, the closest common ancestor of
|
||||
* this view and DetailColumn, which can also open it from its own "查找聊天记录" row). */
|
||||
onOpenHistory: (tab: 'text' | MediaType) => void;
|
||||
}
|
||||
|
||||
interface ContextMenuState {
|
||||
@@ -40,7 +44,7 @@ interface ContextMenuState {
|
||||
|
||||
export function ConversationView() {
|
||||
const { topicId } = useParams();
|
||||
const { onToggleDetail, detailVisible } = useOutletContext<OutletCtx>();
|
||||
const { onToggleDetail, detailVisible, onOpenHistory } = useOutletContext<OutletCtx>();
|
||||
const myUserId = useSessionStore((s) => s.myUserId);
|
||||
const tinode = useTinode();
|
||||
const contacts = useChatList();
|
||||
@@ -65,13 +69,26 @@ export function ConversationView() {
|
||||
const [replyTo, setReplyTo] = useState<StoredMessage | null>(null);
|
||||
const [forwardMsg, setForwardMsg] = useState<StoredMessage | null>(null);
|
||||
const [emojiOpen, setEmojiOpen] = useState(false);
|
||||
const [errorToast, setErrorToast] = useState('');
|
||||
const streamRef = useRef<HTMLDivElement>(null);
|
||||
const autoScrollRef = useRef(true);
|
||||
const topicIdRef = useRef(topicId);
|
||||
useEffect(() => {
|
||||
topicIdRef.current = topicId;
|
||||
});
|
||||
|
||||
// Every "fire and forget" send/recall/favorite/forward action below used to swallow its own
|
||||
// failure with an empty `.catch(() => {})` (or, for forward, no catch at all) -- e.g. an
|
||||
// over-size-limit attachment would throw a friendly Chinese error from uploadFile() that then
|
||||
// just vanished, leaving the user staring at nothing happening with no idea why.
|
||||
const showError = (err: unknown, fallback: string) => setErrorToast(err instanceof Error ? err.message : fallback);
|
||||
|
||||
useEffect(() => {
|
||||
if (!topicId) return undefined;
|
||||
openConversation(topicId).catch(() => {});
|
||||
return () => closeConversation(topicId);
|
||||
return () => {
|
||||
closeConversation(topicId).catch(() => {});
|
||||
};
|
||||
}, [topicId]);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -87,13 +104,22 @@ export function ConversationView() {
|
||||
// onSend callback (re-created every render off the CURRENT topicId) would publish the eventual
|
||||
// recording into B instead of the conversation the user was actually recording in.
|
||||
setRecording(false);
|
||||
// Without these two, a "加载更多" in flight when the user switches conversations leaves a
|
||||
// stale spinner showing (never resolves against the topic it started on) and, if the PREVIOUS
|
||||
// conversation had been scrolled up when it was left, the ref carries that `false` into the
|
||||
// NEW conversation and silently skips scrolling it to the bottom on open.
|
||||
setLoadingOlder(false);
|
||||
autoScrollRef.current = true;
|
||||
}, [topicId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (autoScrollRef.current) {
|
||||
streamRef.current?.scrollTo({ top: streamRef.current.scrollHeight });
|
||||
}
|
||||
}, [messages.length]);
|
||||
// `topicId` is included so switching between two conversations that both happen to have
|
||||
// already loaded the same number of messages (e.g. both exactly MESSAGES_PAGE on first open)
|
||||
// still scrolls the newly-opened one to the bottom -- `messages.length` alone wouldn't change.
|
||||
}, [messages.length, topicId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!topicId || messages.length === 0) return;
|
||||
@@ -113,6 +139,7 @@ export function ConversationView() {
|
||||
*/
|
||||
const loadOlder = async () => {
|
||||
if (loadingOlder) return;
|
||||
const calledForTopicId = topicId;
|
||||
const el = streamRef.current;
|
||||
const min = topic.minMsgSeq();
|
||||
const max = topic.maxMsgSeq();
|
||||
@@ -124,10 +151,17 @@ export function ConversationView() {
|
||||
try {
|
||||
await topic.getMessagesPage(MESSAGES_PAGE, gaps, min, max);
|
||||
} finally {
|
||||
setLoadingOlder(false);
|
||||
requestAnimationFrame(() => {
|
||||
if (el) el.scrollTop = prevTop + (el.scrollHeight - prevHeight);
|
||||
});
|
||||
// If the user switched to a different conversation while this was in flight, `el` is now
|
||||
// showing that OTHER topic's messages -- prevTop/prevHeight were measured against content
|
||||
// that isn't there anymore, so applying them would jump the NEW conversation's scroll
|
||||
// position to a meaningless spot. Only the topic that actually asked for more history should
|
||||
// ever clear its own loading flag or touch the scroll position.
|
||||
if (topicIdRef.current === calledForTopicId) {
|
||||
setLoadingOlder(false);
|
||||
requestAnimationFrame(() => {
|
||||
if (el) el.scrollTop = prevTop + (el.scrollHeight - prevHeight);
|
||||
});
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@@ -186,7 +220,7 @@ export function ConversationView() {
|
||||
|
||||
const bulkFavorite = () => {
|
||||
if (selectedSeqs.size === 0) return;
|
||||
favoriteMessages([...selectedSeqs].map((seq) => ({ topic: topicId, seq }))).catch(() => {});
|
||||
favoriteMessages([...selectedSeqs].map((seq) => ({ topic: topicId, seq }))).catch((err) => showError(err, '收藏失败'));
|
||||
exitSelectMode();
|
||||
};
|
||||
|
||||
@@ -194,7 +228,7 @@ export function ConversationView() {
|
||||
const target = tinode.getTopic(targetTopic);
|
||||
for (const seq of selectedSeqs) {
|
||||
const msg = topic.findMessage(seq);
|
||||
if (msg) forwardMessage(topicId, msg, target);
|
||||
if (msg) forwardMessage(topicId, msg, target).catch((err) => showError(err, '转发失败'));
|
||||
}
|
||||
setBulkForwardOpen(false);
|
||||
exitSelectMode();
|
||||
@@ -211,7 +245,7 @@ export function ConversationView() {
|
||||
case 'recall':
|
||||
recallMessage(topicId, msg.seq)
|
||||
.then(() => topic.delMessagesList([msg.seq], true))
|
||||
.catch(() => {});
|
||||
.catch((err) => showError(err, '撤回失败'));
|
||||
break;
|
||||
case 'delete':
|
||||
topic.delMessagesList([msg.seq], false);
|
||||
@@ -224,7 +258,7 @@ export function ConversationView() {
|
||||
setReplyTo(msg);
|
||||
break;
|
||||
case 'favorite':
|
||||
favoriteMessages([{ topic: topicId, seq: msg.seq }]).catch(() => {});
|
||||
favoriteMessages([{ topic: topicId, seq: msg.seq }]).catch((err) => showError(err, '收藏失败'));
|
||||
break;
|
||||
case 'forward':
|
||||
setForwardMsg(msg);
|
||||
@@ -235,13 +269,13 @@ export function ConversationView() {
|
||||
const handlePickImage = (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
const file = e.target.files?.[0];
|
||||
e.target.value = '';
|
||||
if (file) sendImage(topic, file).catch(() => {});
|
||||
if (file) sendImage(topic, file).catch((err) => showError(err, '图片发送失败'));
|
||||
};
|
||||
|
||||
const handlePickFile = (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
const file = e.target.files?.[0];
|
||||
e.target.value = '';
|
||||
if (file) sendFile(topic, file).catch(() => {});
|
||||
if (file) sendFile(topic, file).catch((err) => showError(err, '文件发送失败'));
|
||||
};
|
||||
|
||||
return (
|
||||
@@ -279,7 +313,11 @@ export function ConversationView() {
|
||||
<button type="button" className="grid h-9.5 w-9.5 place-items-center rounded-lg text-text-tertiary-1">
|
||||
<Icon name="videocam" />
|
||||
</button>
|
||||
<button type="button" className="grid h-9.5 w-9.5 place-items-center rounded-lg text-text-tertiary-1">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onOpenHistory('text')}
|
||||
className="grid h-9.5 w-9.5 place-items-center rounded-lg text-text-tertiary-1"
|
||||
>
|
||||
<Icon name="search" />
|
||||
</button>
|
||||
<button
|
||||
@@ -304,7 +342,6 @@ export function ConversationView() {
|
||||
mine={!!myUserId && msg.from === myUserId}
|
||||
isGroup={isGroup}
|
||||
peerName={peerName}
|
||||
myUserId={myUserId}
|
||||
selectMode={selectMode}
|
||||
selected={typeof msg.seq === 'number' && selectedSeqs.has(msg.seq)}
|
||||
menuActive={typeof msg.seq === 'number' && menu?.msg.seq === msg.seq}
|
||||
@@ -392,7 +429,7 @@ export function ConversationView() {
|
||||
onCancel={() => setRecording(false)}
|
||||
onSend={(blob, durationMs) => {
|
||||
setRecording(false);
|
||||
sendVoice(topic, blob, durationMs).catch(() => {});
|
||||
sendVoice(topic, blob, durationMs).catch((err) => showError(err, '语音发送失败'));
|
||||
}}
|
||||
/>
|
||||
) : (
|
||||
@@ -424,6 +461,7 @@ export function ConversationView() {
|
||||
x={menu.x}
|
||||
y={menu.y}
|
||||
actions={availableActions(menu.msg, myUserId, bootstrap.data?.message_policy)}
|
||||
recallRemainingSeconds={recallRemainingSeconds(menu.msg, myUserId, bootstrap.data?.message_policy)}
|
||||
onAction={handleAction}
|
||||
onClose={() => setMenu(null)}
|
||||
/>
|
||||
@@ -433,13 +471,15 @@ export function ConversationView() {
|
||||
<ForwardPickerModal
|
||||
onClose={() => setForwardMsg(null)}
|
||||
onPick={(targetTopic) => {
|
||||
forwardMessage(topicId, forwardMsg, tinode.getTopic(targetTopic));
|
||||
forwardMessage(topicId, forwardMsg, tinode.getTopic(targetTopic)).catch((err) => showError(err, '转发失败'));
|
||||
setForwardMsg(null);
|
||||
}}
|
||||
/>
|
||||
) : null}
|
||||
|
||||
{bulkForwardOpen ? <ForwardPickerModal onClose={() => setBulkForwardOpen(false)} onPick={bulkForward} /> : null}
|
||||
|
||||
<Toast message={errorToast} variant="error" onDone={() => setErrorToast('')} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user