Skip to content

fix: 채팅 전송 스크롤 상태 초기화#532

Merged
manNomi merged 1 commit into
mainfrom
fix/chat-force-scroll-reset
May 26, 2026
Merged

fix: 채팅 전송 스크롤 상태 초기화#532
manNomi merged 1 commit into
mainfrom
fix/chat-force-scroll-reset

Conversation

@manNomi
Copy link
Copy Markdown
Contributor

@manNomi manNomi commented May 26, 2026

관련 이슈

작업 내용

  • 텍스트 메시지 전송 시 force-scroll 상태를 즉시 고정하지 않고, pending outbound 텍스트로만 등록하도록 변경했습니다.
  • 메시지 목록 증가분에서 같은 senderId와 같은 내용의 내 메시지가 확인될 때만 하단 강제 이동을 수행하도록 수정했습니다.
  • 서버 echo가 없거나 전송이 거절되는 경우 pending 상태를 5초 후 제거해, 이후 무관한 수신 메시지로 사용자가 하단에 튀는 문제를 방지했습니다.
  • 채팅방 전환/언마운트 시 pending 타이머를 정리합니다.

검증

  • pnpm --filter @solid-connect/web lint:check
  • pnpm --filter @solid-connect/web typecheck
  • push hook: @solid-connect/web ci:check@solid-connect/web build 통과

참고

  • #531은 이미 merge되어 리뷰 반영분만 새 브랜치/새 PR로 분리했습니다.
  • 로컬 Node가 v23.10.0이라 engines.node: 22.x 경고가 출력되지만 검증은 모두 통과했습니다.

@manNomi manNomi requested review from enunsnv and wibaek as code owners May 26, 2026 10:05
@vercel
Copy link
Copy Markdown

vercel Bot commented May 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
solid-connection-web Ready Ready Preview, Comment May 26, 2026 10:07am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
solid-connect-web-admin Skipped Skipped May 26, 2026 10:07am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Review Change Stack

Walkthrough

이 변경사항은 채팅 메시지 자동 스크롤 제어 로직을 정교하게 개선합니다.

  1. Pending 추적 인프라 도입: PendingOutboundTextScroll 타입과 normalizePendingText 헬퍼 함수를 추가하여 발신 메시지를 일관되게 추적합니다.

  2. 메시지 전송 시 pending 등록: 사용자가 메시지를 보낼 때 내용을 정규화하고, 즉시 pending 목록에 추가한 후 자동 제거 타이머를 예약합니다.

  3. 메시지 도착 시 확정 매칭: 서버에서 메시지가 도착하면 pending 목록과 발신자 및 정규화된 내용으로 매칭하여 일치하는 항목을 제거하고, 이를 바탕으로 스크롤 강제 여부를 판단합니다.

  4. 라이프사이클 정리: 채팅방 전환 시와 컴포넌트 언마운트 시 모든 pending 항목과 타이머를 정리하여 상태 누수를 방지합니다.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • wibaek
  • enunsnv
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 채팅 전송 시 스크롤 강제 이동 문제를 해결하는 것이 주 목표이며, '채팅 전송 스크롤 상태 초기화'는 이를 정확하게 반영하고 있습니다.
Description check ✅ Passed PR 설명은 템플릿의 주요 섹션(관련 이슈, 작업 내용, 특이 사항)을 포함하고 있으며, 변경사항의 의도와 검증 과정을 명확히 기술하고 있습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/chat-force-scroll-reset

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/web/src/app/mentor/chat/[chatId]/_ui/ChatContent/_hooks/useChatListHandler.ts (1)

251-266: ⚡ Quick win

1. .some() 사용 시 첫 번째 매칭 후 반복이 중단되는 문제

.some() 메서드는 첫 번째 true 반환 시 즉시 반복을 중단합니다. 만약 여러 outbound 메시지가 동시에 도착하면, 첫 번째 매칭된 pending 항목만 제거되고 나머지는 타임아웃까지 남아있게 됩니다.

forEach 또는 reduce를 사용하여 모든 새 메시지를 순회하고, 매칭된 모든 pending 항목을 수집하는 것이 더 정확합니다.

♻️ 모든 매칭을 처리하도록 수정
     const newMessages = submittedMessages.slice(prevMessageCount);
     const matchedPendingOutboundTextIds = new Set<string>();
-    const hasConfirmedOutboundText = newMessages.some((message) => {
+
+    newMessages.forEach((message) => {
       const pendingOutboundText = pendingOutboundTextScrollsRef.current.find(
         (item) => item.senderId === message.senderId && item.content === normalizePendingText(message.content),
       );
 
-      if (!pendingOutboundText) return false;
+      if (!pendingOutboundText) return;
 
       matchedPendingOutboundTextIds.add(pendingOutboundText.id);
-      return true;
     });
+
+    const hasConfirmedOutboundText = matchedPendingOutboundTextIds.size > 0;

     if (matchedPendingOutboundTextIds.size > 0) {
       removePendingOutboundTextScrolls(matchedPendingOutboundTextIds);
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/web/src/app/mentor/chat/`[chatId]/_ui/ChatContent/_hooks/useChatListHandler.ts
around lines 251 - 266, The current use of submittedMessages.some(...) stops
after the first match so only one pendingOutboundText gets removed; instead
iterate all newMessages (derived from submittedMessages.slice(prevMessageCount))
with a forEach or for...of, for each message find pendingOutboundText via
pendingOutboundTextScrollsRef.current.find(...) (using normalizePendingText for
comparison), add every matched pendingOutboundText.id to
matchedPendingOutboundTextIds, and after the loop call
removePendingOutboundTextScrolls(matchedPendingOutboundTextIds); remove or
replace the hasConfirmedOutboundText boolean logic that relied on .some() if not
needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@apps/web/src/app/mentor/chat/`[chatId]/_ui/ChatContent/_hooks/useChatListHandler.ts:
- Around line 251-266: The current use of submittedMessages.some(...) stops
after the first match so only one pendingOutboundText gets removed; instead
iterate all newMessages (derived from submittedMessages.slice(prevMessageCount))
with a forEach or for...of, for each message find pendingOutboundText via
pendingOutboundTextScrollsRef.current.find(...) (using normalizePendingText for
comparison), add every matched pendingOutboundText.id to
matchedPendingOutboundTextIds, and after the loop call
removePendingOutboundTextScrolls(matchedPendingOutboundTextIds); remove or
replace the hasConfirmedOutboundText boolean logic that relied on .some() if not
needed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ad1ebc14-cff1-49ea-a60a-177a669f5b92

📥 Commits

Reviewing files that changed from the base of the PR and between cb362e6 and 4fe68fc.

📒 Files selected for processing (1)
  • apps/web/src/app/mentor/chat/[chatId]/_ui/ChatContent/_hooks/useChatListHandler.ts

@manNomi manNomi merged commit 4351ca9 into main May 26, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant