fix(web): code review round 3 -- UI bugs and remaining design fidelity
- FileAttachment never surfaced a failed authenticated download -- the card looked identically clickable whether it worked or not, and clicking while broken silently no-op'd. Added an error state/retry (useAuthedBlobUrl gained a retry() escape hatch for this). ImageAttachment/AudioAttachment already handled this; only the file card didn't. - --shadow-btn was one token serving two different specs: round 2's fix (0 6px 16px, from the 50px auth CTAs) actually regressed the 36px messaging send button (ConversationView/VoiceRecorder), which needs 0 4px 12px per 02-messages.dc.html. Split into --shadow-btn / --shadow-btn-sm. - loadOlder's scroll-restore guard compared topicId by value, which false-positives on an A->B->A round trip within one request's lifetime (same string, but the DOM was torn down and rebuilt by the intervening open/close). Switched to a generation counter, same pattern as topics.ts's leave-race fix. - errorToast wasn't included in ConversationView/DetailColumn's topicId-reset effects -- a failed action's toast could keep showing over a conversation/contact switched to afterward. Also switched those effects (and MessagesLayout's) from useEffect to useLayoutEffect -- since none of these components remount on topicId change, a post-paint effect could let one frame render with the previous topicId's stale UI still showing. - VoiceRecorder never listened for MediaRecorder's own onerror -- a mid-recording failure left the "录音中" timer ticking forever with no indication the recording had already died. - VerificationCodeButton's phone-change countdown reset (round 2) only cleared the button's own state; RegisterPage/ResetPasswordPage's challengeId was still tied to whatever phone the code was actually issued for, with nothing cross-checking it against a since-edited phone number. - Agreement's checkbox used bare accent-color on a native input, which only tints the checked fill and leaves size/radius/border to the browser/OS; replaced with the spec's exact 16x16 border-radius:4px box + check glyph. - Added a catch-all route (App.tsx), TenantFeedbackPage image-picker validation feedback + corrected remove-button style, and a handful of smaller token/spacing corrections against screens/*.dc.html (NavRail font-weight, LoginPage/TenantEntryPage logo size+shadow+color, ChatListColumn row padding/icon size, --shadow-modal alpha). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
8ec74dffcc
commit
72b7b79caf
@@ -47,7 +47,12 @@ export function FileAttachment({ data }: { data: FileData }) {
|
||||
href={href}
|
||||
download={data.name}
|
||||
onClick={(e) => {
|
||||
if (!href) e.preventDefault();
|
||||
if (remote.error) {
|
||||
e.preventDefault();
|
||||
remote.retry();
|
||||
} else if (!href) {
|
||||
e.preventDefault();
|
||||
}
|
||||
}}
|
||||
className="flex w-[280px] items-center gap-3 rounded-lg bg-white px-3.5 py-3 no-underline shadow-bubble"
|
||||
>
|
||||
@@ -56,11 +61,15 @@ export function FileAttachment({ data }: { data: FileData }) {
|
||||
</span>
|
||||
<span className="flex min-w-0 flex-col">
|
||||
<span className="truncate text-sm font-medium text-text-body">{data.name || '文件'}</span>
|
||||
<span className="text-xs text-text-tertiary-2">
|
||||
{typeof data.size === 'number' ? formatFileSize(data.size) : ''} · {remote.loading ? '加载中…' : '点击下载'}
|
||||
<span className={`text-xs ${remote.error ? 'text-danger-text' : 'text-text-tertiary-2'}`}>
|
||||
{typeof data.size === 'number' ? formatFileSize(data.size) : ''} ·{' '}
|
||||
{remote.error ? '下载失败,点击重试' : remote.loading ? '加载中…' : '点击下载'}
|
||||
</span>
|
||||
</span>
|
||||
<Icon name="download" className="ml-auto flex-none text-[20px] text-text-placeholder-2" />
|
||||
<Icon
|
||||
name={remote.error ? 'error_outline' : 'download'}
|
||||
className={`ml-auto flex-none text-[20px] ${remote.error ? 'text-danger-text' : 'text-text-placeholder-2'}`}
|
||||
/>
|
||||
</a>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -73,10 +73,10 @@ export function ChatListColumn() {
|
||||
<Link
|
||||
key={entry.key}
|
||||
to={entry.route}
|
||||
className="grid min-h-[68px] grid-cols-[44px_minmax(0,1fr)_auto] items-center gap-3 border-b border-list-item bg-white px-4 py-2.5 no-underline hover:bg-[#f6f8fb]"
|
||||
className="grid min-h-[68px] grid-cols-[44px_minmax(0,1fr)_auto] items-center gap-3 border-b border-list-item bg-white py-2.5 pl-4 pr-3 no-underline hover:bg-[#f6f8fb]"
|
||||
>
|
||||
<span className={`grid h-11 w-11 place-items-center rounded-lg ${entry.colorClass}`}>
|
||||
<Icon name={entry.icon} className="text-[22px]" />
|
||||
<Icon name={entry.icon} className="text-2xl" />
|
||||
</span>
|
||||
<span className="flex min-w-0 flex-col gap-1">
|
||||
<strong className="truncate text-[15px] font-medium text-[#272b31]">{entry.label}</strong>
|
||||
@@ -105,7 +105,7 @@ export function ChatListColumn() {
|
||||
<Link
|
||||
key={contact.topic}
|
||||
to={`/messages/${contact.topic}`}
|
||||
className={`grid min-h-[68px] grid-cols-[44px_minmax(0,1fr)_auto] items-center gap-3 border-b border-list-item px-4 py-2.5 no-underline ${
|
||||
className={`grid min-h-[68px] grid-cols-[44px_minmax(0,1fr)_auto] items-center gap-3 border-b border-list-item py-2.5 pl-4 pr-3 no-underline ${
|
||||
active ? 'bg-accent-selected-bg' : 'bg-white hover:bg-[#f6f8fb]'
|
||||
}`}
|
||||
>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { useEffect, useLayoutEffect, useRef, useState } from 'react';
|
||||
|
||||
import { Link, useOutletContext, useParams } from 'react-router-dom';
|
||||
import type { StoredMessage } from 'tinode-sdk';
|
||||
@@ -72,10 +72,13 @@ export function ConversationView() {
|
||||
const [errorToast, setErrorToast] = useState('');
|
||||
const streamRef = useRef<HTMLDivElement>(null);
|
||||
const autoScrollRef = useRef(true);
|
||||
const topicIdRef = useRef(topicId);
|
||||
useEffect(() => {
|
||||
topicIdRef.current = topicId;
|
||||
});
|
||||
// Bumped every time a conversation is (re)opened below -- NOT the same as comparing topicId by
|
||||
// value, which would false-positive if the user opens A, then B, then A again within a single
|
||||
// loadOlder() round trip: topicId ends up back at "A" even though the DOM has since been torn
|
||||
// down and rebuilt by the intervening open/close cycle, so a same-topicId check alone would wrongly
|
||||
// treat that as "still the same open" and apply a scroll-restore computed against content that
|
||||
// isn't there anymore.
|
||||
const openGenerationRef = useRef(0);
|
||||
|
||||
// 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
|
||||
@@ -91,7 +94,12 @@ export function ConversationView() {
|
||||
};
|
||||
}, [topicId]);
|
||||
|
||||
useEffect(() => {
|
||||
// useLayoutEffect, not useEffect -- this component never remounts on topicId change (route is
|
||||
// `:topicId`), so a plain useEffect (which fires after paint) can let one frame render with the
|
||||
// PREVIOUS conversation's select-mode/menu/etc. still showing over the new conversation's message
|
||||
// list underneath, self-correcting only on the next tick.
|
||||
useLayoutEffect(() => {
|
||||
openGenerationRef.current += 1;
|
||||
setSelectMode(false);
|
||||
setSelectedSeqs(new Set());
|
||||
setMenu(null);
|
||||
@@ -110,6 +118,9 @@ export function ConversationView() {
|
||||
// NEW conversation and silently skips scrolling it to the bottom on open.
|
||||
setLoadingOlder(false);
|
||||
autoScrollRef.current = true;
|
||||
// An error toast from the previous conversation (e.g. "图片发送失败") would otherwise keep
|
||||
// showing over whichever conversation is now open, misattributing the failure.
|
||||
setErrorToast('');
|
||||
}, [topicId]);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -139,7 +150,7 @@ export function ConversationView() {
|
||||
*/
|
||||
const loadOlder = async () => {
|
||||
if (loadingOlder) return;
|
||||
const calledForTopicId = topicId;
|
||||
const calledForGeneration = openGenerationRef.current;
|
||||
const el = streamRef.current;
|
||||
const min = topic.minMsgSeq();
|
||||
const max = topic.maxMsgSeq();
|
||||
@@ -151,12 +162,13 @@ export function ConversationView() {
|
||||
try {
|
||||
await topic.getMessagesPage(MESSAGES_PAGE, gaps, min, max);
|
||||
} finally {
|
||||
// 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) {
|
||||
// If the conversation was closed and reopened (even back to this SAME topicId) while this was
|
||||
// in flight, `el` now shows content from a different open/close cycle -- prevTop/prevHeight
|
||||
// were measured against content that isn't there anymore, so applying them would jump the
|
||||
// current conversation's scroll to a meaningless spot. Comparing topicId alone would miss an
|
||||
// A->B->A round trip (same string, different generation); only the specific open this request
|
||||
// was made for should ever clear its own loading flag or touch the scroll position.
|
||||
if (openGenerationRef.current === calledForGeneration) {
|
||||
setLoadingOlder(false);
|
||||
requestAnimationFrame(() => {
|
||||
if (el) el.scrollTop = prevTop + (el.scrollHeight - prevHeight);
|
||||
@@ -446,7 +458,7 @@ export function ConversationView() {
|
||||
type="button"
|
||||
onClick={submit}
|
||||
disabled={!draft.trim()}
|
||||
className="h-9 rounded-lg bg-btn px-5.5 text-sm font-medium text-white shadow-btn disabled:bg-btn-disabled disabled:shadow-none"
|
||||
className="h-9 rounded-lg bg-btn px-5.5 text-sm font-medium text-white shadow-btn-sm disabled:bg-btn-disabled disabled:shadow-none"
|
||||
>
|
||||
发送
|
||||
</button>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useLayoutEffect, useState } from 'react';
|
||||
|
||||
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
@@ -56,10 +56,14 @@ export function DetailColumn({ topicId, peerName, onClose, onOpenHistory }: Deta
|
||||
|
||||
// DetailColumn is reused across every contact when docked (no `key={topicId}`, same instance),
|
||||
// so without this an in-progress remark edit for the PREVIOUS contact stays open and could be
|
||||
// saved against the NEW one.
|
||||
useEffect(() => {
|
||||
// saved against the NEW one, and a leftover error toast would keep showing over the new contact
|
||||
// misattributing the failure. useLayoutEffect (not useEffect) so this resolves before paint --
|
||||
// this component never remounts on topicId change, so a post-paint effect can let one frame
|
||||
// render with the previous contact's stale edit box still open.
|
||||
useLayoutEffect(() => {
|
||||
setRemarkEditing(false);
|
||||
setRemarkDraft('');
|
||||
setErrorToast('');
|
||||
}, [topicId]);
|
||||
|
||||
const friendsQuery = useQuery({ queryKey: ['friends'], queryFn: () => fetchAllPages((cursor) => getFriends('', cursor)) });
|
||||
@@ -98,7 +102,7 @@ export function DetailColumn({ topicId, peerName, onClose, onOpenHistory }: Deta
|
||||
});
|
||||
|
||||
const blocksQuery = useQuery({ queryKey: ['blocks'], queryFn: () => fetchAllPages((cursor) => getBlocks(cursor)) });
|
||||
const isBlocked = !!blocksQuery.data?.some((b) => b.user_id === topicId);
|
||||
const isBlocked = !!blocksQuery.data?.some((b) => b.user.user_id === topicId);
|
||||
|
||||
const deleteFriendMutation = useMutation({
|
||||
mutationFn: () => deleteFriend(topicId),
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useLayoutEffect, useState } from 'react';
|
||||
|
||||
import { Outlet, useParams } from 'react-router-dom';
|
||||
|
||||
@@ -37,7 +37,7 @@ export function MessagesLayout() {
|
||||
// MessagesLayout is one long-lived instance reused across every conversation (route is
|
||||
// `:topicId`) -- a history-search modal left open while switching topics would otherwise keep
|
||||
// showing results for whichever topic it was opened against, not the one now on screen.
|
||||
useEffect(() => setHistoryTab(null), [topicId]);
|
||||
useLayoutEffect(() => setHistoryTab(null), [topicId]);
|
||||
|
||||
return (
|
||||
<div className="relative flex h-full w-full">
|
||||
|
||||
@@ -74,6 +74,16 @@ export function VoiceRecorder({ onSend, onCancel }: VoiceRecorderProps) {
|
||||
recorder.ondataavailable = (e) => {
|
||||
if (e.data.size > 0) chunksRef.current.push(e.data);
|
||||
};
|
||||
// Without this, a mid-recording failure (mic unplugged, OS audio error) auto-transitions
|
||||
// the recorder to 'inactive' with nothing telling the UI -- the "录音中" timer just keeps
|
||||
// ticking forever with no way to know the recording already died until the user manually
|
||||
// hits 取消/发送 and finish()'s own `recorder.state === 'inactive'` check silently no-ops.
|
||||
recorder.onerror = () => {
|
||||
if (finishedRef.current) return;
|
||||
finishedRef.current = true;
|
||||
stopStream();
|
||||
setError('录音失败,请重试');
|
||||
};
|
||||
recorder.start();
|
||||
recorderRef.current = recorder;
|
||||
startedAtRef.current = Date.now();
|
||||
@@ -125,7 +135,7 @@ export function VoiceRecorder({ onSend, onCancel }: VoiceRecorderProps) {
|
||||
type="button"
|
||||
onClick={() => finish(true)}
|
||||
disabled={!ready}
|
||||
className="flex h-8 items-center gap-1 rounded-lg bg-btn px-4 text-sm font-medium text-white shadow-btn disabled:bg-btn-disabled"
|
||||
className="flex h-8 items-center gap-1 rounded-lg bg-btn px-4 text-sm font-medium text-white shadow-btn-sm disabled:bg-btn-disabled"
|
||||
>
|
||||
<Icon name="send" className="text-[16px]" />
|
||||
发送
|
||||
|
||||
Reference in New Issue
Block a user