perf: sync comment reply target during render (#4281)
This commit is contained in:
parent
f3ba73d845
commit
d2c179a872
|
|
@ -109,6 +109,7 @@ import {
|
|||
updateCommentCodeContextExpansionState,
|
||||
type CommentCodeContextLineUpdate
|
||||
} from '@/components/comment-code-context-state'
|
||||
import { resolveCommentReplyTarget } from '@/components/comment-reply-target-state'
|
||||
import { useAppStore } from '@/store'
|
||||
import { useAllWorktrees } from '@/store/selectors'
|
||||
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
|
||||
|
|
@ -2380,12 +2381,13 @@ function ConversationTab({
|
|||
[commentFilter, comments]
|
||||
)
|
||||
const visibleCommentGroups = useMemo(() => groupPRComments(visibleComments), [visibleComments])
|
||||
const resolvedReplyingTo = resolveCommentReplyTarget(replyingTo, visibleComments)
|
||||
|
||||
useEffect(() => {
|
||||
if (replyingTo !== null && !visibleComments.some((comment) => comment.id === replyingTo)) {
|
||||
setReplyingTo(null)
|
||||
}
|
||||
}, [replyingTo, visibleComments])
|
||||
if (resolvedReplyingTo !== replyingTo) {
|
||||
// Why: comment filters/refetches can hide the active reply target; clear it
|
||||
// before paint so a stale composer does not flash for the wrong comment set.
|
||||
setReplyingTo(resolvedReplyingTo)
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
if (!bodyEditing) {
|
||||
|
|
@ -2590,7 +2592,7 @@ function ConversationTab({
|
|||
className="min-w-0 max-w-full overflow-hidden break-words text-[13px] leading-relaxed [&_a]:break-all [&_code]:break-words [&_pre]:max-w-full"
|
||||
/>
|
||||
<CommentReactions reactions={comment.reactions} />
|
||||
{replyingTo === comment.id && (
|
||||
{resolvedReplyingTo === comment.id && (
|
||||
<CommentReplyForm
|
||||
className="mt-3"
|
||||
placeholder={
|
||||
|
|
|
|||
|
|
@ -108,6 +108,7 @@ import {
|
|||
updateCommentCodeContextExpansionState,
|
||||
type CommentCodeContextLineUpdate
|
||||
} from '@/components/comment-code-context-state'
|
||||
import { resolveCommentReplyTarget } from '@/components/comment-reply-target-state'
|
||||
import { useAppStore } from '@/store'
|
||||
import { useAllWorktrees } from '@/store/selectors'
|
||||
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
|
||||
|
|
@ -2571,6 +2572,7 @@ function ConversationTab({
|
|||
[commentFilter, comments]
|
||||
)
|
||||
const visibleCommentGroups = useMemo(() => groupPRComments(visibleComments), [visibleComments])
|
||||
const resolvedReplyingTo = resolveCommentReplyTarget(replyingTo, visibleComments)
|
||||
const mentionOptions = useMemo(
|
||||
() =>
|
||||
buildMentionOptions({
|
||||
|
|
@ -2589,11 +2591,11 @@ function ConversationTab({
|
|||
}
|
||||
}, [])
|
||||
|
||||
useEffect(() => {
|
||||
if (replyingTo !== null && !visibleComments.some((comment) => comment.id === replyingTo)) {
|
||||
setReplyingTo(null)
|
||||
}
|
||||
}, [replyingTo, visibleComments])
|
||||
if (resolvedReplyingTo !== replyingTo) {
|
||||
// Why: comment filters/refetches can hide the active reply target; clear it
|
||||
// before paint so a stale composer does not flash for the wrong comment set.
|
||||
setReplyingTo(resolvedReplyingTo)
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
if (!bodyEditing) {
|
||||
|
|
@ -2811,7 +2813,7 @@ function ConversationTab({
|
|||
className="min-w-0 max-w-full overflow-hidden break-words text-[13px] leading-relaxed [&_a]:break-all [&_code]:break-words [&_pre]:max-w-full"
|
||||
/>
|
||||
<CommentReactions reactions={comment.reactions} />
|
||||
{replyingTo === comment.id && (
|
||||
{resolvedReplyingTo === comment.id && (
|
||||
<CommentReplyForm
|
||||
className="mt-3"
|
||||
placeholder={
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { resolveCommentReplyTarget } from './comment-reply-target-state'
|
||||
|
||||
describe('comment reply target state', () => {
|
||||
it('preserves a visible reply target', () => {
|
||||
expect(resolveCommentReplyTarget(2, [{ id: 1 }, { id: 2 }])).toBe(2)
|
||||
})
|
||||
|
||||
it('clears a hidden reply target', () => {
|
||||
expect(resolveCommentReplyTarget(3, [{ id: 1 }, { id: 2 }])).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps an empty reply target empty', () => {
|
||||
expect(resolveCommentReplyTarget(null, [{ id: 1 }])).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
export type CommentReplyTargetComment = {
|
||||
id: number
|
||||
}
|
||||
|
||||
export function resolveCommentReplyTarget(
|
||||
replyingTo: number | null,
|
||||
visibleComments: readonly CommentReplyTargetComment[]
|
||||
): number | null {
|
||||
if (replyingTo === null) {
|
||||
return null
|
||||
}
|
||||
return visibleComments.some((comment) => comment.id === replyingTo) ? replyingTo : null
|
||||
}
|
||||
Loading…
Reference in New Issue