Conversation
hotfix: 배너 이미지 + 텍스트 불일치 문제 해결
hotfix: 조언 페이지 메시지 전송 관련 UX 개선사항 반영 (재전송 방지 + 스크롤 컨트롤 + 코드 리팩토링)
There was a problem hiding this comment.
Pull request overview
QA 피드백을 반영해 조언(Advice) 채팅 페이지의 UI/상태 관리를 리팩터링하고, 관련 MSW mock을 보강한 PR입니다.
Changes:
- 조언 제출 UI를
react-hook-form기반 폼/바텀시트 컴포넌트로 재구성하고 Context로 상태를 공유하도록 변경 - 채팅 히스토리 자동 스크롤 및 Header UI 단순화(목표 선택 제거)
- MSW에
/advice/chat도메인 핸들러 추가 및 홈 배너 메시지 순서 조정
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/feature/advice/hooks/useSubscribeAdviceArrival.tsx | 팝업 컴포넌트 이름을 Wrapper로 변경 |
| src/feature/advice/hooks/useAdviceFormContext.ts | Advice 폼/스타일 시트 컨텍스트 접근 훅 추가 |
| src/feature/advice/components/AdviceSubmitFormElements.tsx | 제출 폼을 RHF + BottomSheet 기반으로 구성하는 엘리먼트 모음 추가 |
| src/feature/advice/components/AdviceStyleSelectSheet.tsx | 기존 스타일 선택 시트 컴포넌트 제거(새 폼 엘리먼트로 이동) |
| src/feature/advice/components/AdviceSendButton.tsx | 기존 전송 버튼 컴포넌트 제거(새 폼 엘리먼트로 이동) |
| src/feature/advice/components/AdviceHeader.tsx | 헤더를 히스토리 이동 버튼 중심으로 단순화 |
| src/feature/advice/components/AdviceFormContext.tsx | 폼 제출/시트 상태 공유용 Context 추가 |
| src/feature/advice/components/AdviceChatHistory.tsx | 컨텍스트 기반 pending 상태 사용 + 자동 스크롤 추가 |
| src/feature/advice/AdviceSubmitFormSchema.ts | zod 기반 폼 스키마 추가 |
| src/composite/home/homeBanner/constants.ts | 배너 메시지 노출 순서 조정 |
| src/composite/advice/index.tsx | 조언 페이지를 Context + 새 폼 엘리먼트 기반으로 리팩터링 |
| mocks/handlers.ts | advice 핸들러들을 전역 handlers에 추가 |
| mocks/domain/advice.ts | /advice/chat MSW 핸들러 구현 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const AdviceSubmitButton = ({ ...props }: ButtonHTMLAttributes<HTMLButtonElement>) => { | ||
| const { formState, watch } = useFormContext<AdviceChatRequest>(); | ||
| const isDisabled = formState.isSubmitting || !watch('userMessage'); |
There was a problem hiding this comment.
The submit button disablement no longer accounts for the backend/request state or daily limit. Previously sending was blocked when remainingCount === 0 and while a request was pending; now it only checks isSubmitting and empty message. Consider also disabling based on isSendingRequest/remainingCount from useAdviceFormContext to prevent duplicate submits and requests after the limit is reached.
| const isDisabled = formState.isSubmitting || !watch('userMessage'); | |
| const { isSendingRequest, remainingCount } = useAdviceFormContext(); | |
| const isDisabled = | |
| formState.isSubmitting || | |
| !watch('userMessage') || | |
| isSendingRequest || | |
| remainingCount === 0; |
| export const useAdviceFormContext = () => { | ||
| const context = useContext(AdviceFormContext); | ||
| if (!context) { | ||
| throw new Error('useAdviceFormContext must be used within an AdviceForm'); |
There was a problem hiding this comment.
This error message references AdviceForm, but the actual provider is AdviceFormContext.Provider (and the wrapper component is AdviceSubmitForm.Root). Updating the message to match the real provider/component name will make runtime errors much easier to diagnose.
| throw new Error('useAdviceFormContext must be used within an AdviceForm'); | |
| throw new Error('useAdviceFormContext must be used within AdviceSubmitForm.Root (AdviceFormContext.Provider)'); |
| export const useAdviceStyleSelectContext = () => { | ||
| const context = useContext(AdviceStyleSelectContext); | ||
| if (!context) { | ||
| throw new Error('useAdviceStyleSelectContext must be used within an AdviceStyleSelectContextProvider'); |
There was a problem hiding this comment.
This error message mentions AdviceStyleSelectContextProvider, but the provider in use is AdviceStyleSelectContext.Provider. Consider updating the wording to match the actual provider/component name to reduce confusion when the hook is misused.
| throw new Error('useAdviceStyleSelectContext must be used within an AdviceStyleSelectContextProvider'); | |
| throw new Error('useAdviceStyleSelectContext must be used within an AdviceStyleSelectContext.Provider'); |
| @@ -12,7 +12,7 @@ interface DailyAdviceCheckedStorage { | |||
| const IS_DAILY_ADVICE_CHECKED = 'isDailyAdviceChecked'; | |||
|
|
|||
| /** 아침 조언 도착 팝업 제공자, 커스텀 이벤트 구독 및 바텀시트 팝업 담당 */ | |||
There was a problem hiding this comment.
The JSDoc still calls this a “provider”, but the exported component was renamed to AdviceArrivalPopupWrapper. Update the comment to match the new name/role so it doesn’t mislead future readers.
| /** 아침 조언 도착 팝업 제공자, 커스텀 이벤트 구독 및 바텀시트 팝업 담당 */ | |
| /** 아침 조언 도착 팝업 래퍼, 커스텀 이벤트 구독 및 바텀시트 팝업 표시를 담당 */ |
|
|
||
| const response: AdviceChatResponse = { | ||
| data: { | ||
| remainingCount: 5, // 남은 조언 횟수 |
There was a problem hiding this comment.
The mock returns remainingCount: 5, but the UI copy and badge default imply a 1-day max of 3 (e.g., “1일 3개 입력만 가능”, maxAdviceCount = 3). Aligning the mock remainingCount with the UI limit will avoid confusing Storybook/MSW runs.
| remainingCount: 5, // 남은 조언 횟수 | |
| remainingCount: 3, // 남은 조언 횟수 (UI max와 일치) |
| const { isSendingRequest } = useAdviceFormContext(); | ||
| const { displayMessages, backgroundType, shouldShowOnboarding } = useAdviceChatMessages(adviceChat, isSendingRequest); | ||
|
|
||
| const scrollRef = useRef<HTMLElement>(null); |
There was a problem hiding this comment.
useRef<HTMLElement>(null) is a type error under strict: true because null isn’t assignable to HTMLElement. Use a nullable ref type (e.g., useRef<HTMLElement | null>(null)) or a non-null assertion if you can guarantee initialization.
| const scrollRef = useRef<HTMLElement>(null); | |
| const scrollRef = useRef<HTMLElement | null>(null); |
| return ( | ||
| <> | ||
| <section className="px-5 pb-[138px] absolute inset-0 overflow-y-auto z-10 space-y-4"> | ||
| <section ref={scrollRef} className="px-5 pb-34.5 absolute inset-0 overflow-y-auto z-10 space-y-4"> |
There was a problem hiding this comment.
pb-34.5 is not a default Tailwind spacing step (and there’s no custom spacing config in the repo), so this class will likely be ignored and the chat list may overlap the sticky form. Consider using an arbitrary value (e.g., pb-[138px]) or an existing spacing token.
| <section ref={scrollRef} className="px-5 pb-34.5 absolute inset-0 overflow-y-auto z-10 space-y-4"> | |
| <section ref={scrollRef} className="px-5 pb-[138px] absolute inset-0 overflow-y-auto z-10 space-y-4"> |
| }; | ||
| requestAdvice(adviceChatRequest); | ||
| // 사용자 메시지만 초기화 | ||
| formMethods.setValue('userMessage', ''); | ||
| }; |
There was a problem hiding this comment.
requestAdvice (from mutateAsync) returns a Promise but isn’t awaited here. As a result, formState.isSubmitting won’t reliably represent the in-flight request, errors can become unhandled rejections, and userMessage gets cleared even when the request fails. Make the submit handler async, await requestAdvice, and only clear the input on success (or after the Promise resolves).
No description provided.