Conversation
…o refactor/photo-manange-qa-2-pm031(#202)
jeonbinggu
added
design
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @jeonbinggu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 현상관리 페이지의 2차 QA 반영을 목표로, 사용자 경험을 개선하기 위한 여러 기능 및 UI/UX 개선 사항을 포함합니다. 특히 주소 입력 및 인화 옵션 선택 과정의 상태 관리 로직을 Zustand 스토어를 활용하여 더욱 견고하게 만들었으며, 실시간 가격 견적 시스템을 도입하여 사용자에게 즉각적인 피드백을 제공합니다. 이를 통해 사용자가 더 직관적이고 오류 없이 주문 과정을 진행할 수 있도록 지원합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
현상 관리 플로우의 상태 관리를 Zustand 스토어를 사용하도록 리팩토링하여 페이지 간 상태를 유지하는 훌륭한 개선이 이루어졌습니다. 이를 통해 뒤로가기나 새로고침 시에도 사용자 경험이 향상되었습니다. 주소 선택 및 가격 견적 로직 또한 더 견고해졌습니다. 리뷰 중 몇 군데에서 window.setTimeout(..., 0) 패턴이 반복적으로 사용되는 것을 확인했는데, 코드의 명확성과 예측 가능성을 높이기 위해 이를 단순화할 수 있는 부분을 제안했습니다. 또한 타입 정의를 개선하여 더 타입-세이프하고 명확한 코드를 작성할 수 있는 부분에 대한 제안도 포함했습니다. 전반적으로 이 기능의 아키텍처를 개선한 훌륭한 작업입니다.
| useEffect(() => { | ||
| const id = window.setTimeout(() => { | ||
| setAddressDetail(deliveryAddress?.addressDetail ?? ""); | ||
| }, 0); | ||
|
|
||
| return () => window.clearTimeout(id); | ||
| }, [deliveryAddress?.addressDetail]); |
There was a problem hiding this comment.
여기서 window.setTimeout(..., 0)을 사용하는 것은 불필요해 보입니다. useEffect는 렌더링 후에 실행되며, React가 상태 업데이트를 배치(batch) 처리합니다. setTimeout을 사용하면 컴포넌트의 동작을 예측하기 어렵게 만들고 디버깅을 복잡하게 할 수 있습니다. useEffect 내에서 직접 setAddressDetail을 호출하는 것이 더 명확합니다.
useEffect(() => {
setAddressDetail(deliveryAddress?.addressDetail ?? "");
}, [deliveryAddress?.addressDetail]);
| setDeliveryAddress({ | ||
| recipientName: "", // store 타입 유지용(지금 플로우에선 안 씀) | ||
| phone: "", // store 타입 유지용(지금 플로우에선 안 씀) | ||
| zipcode: data.zipcode, | ||
| address: data.address, | ||
| addressDetail, // 현재 입력중 상세주소 유지 | ||
| }); |
There was a problem hiding this comment.
recipientName과 phone에 빈 문자열을 할당하여 타입 호환성을 맞추는 것보다, DeliveryAddressRequest 타입 자체에서 해당 속성들을 선택적(optional)으로 만드는 것이 더 나은 접근 방식일 수 있습니다. 이렇게 하면 아직 값이 없는 상태를 더 명확하게 표현할 수 있고, 불필요한 플레이스홀더 값 사용을 피할 수 있습니다. 예를 들어, 타입을 다음과 같이 수정하는 것을 고려해볼 수 있습니다.
// In src/types/photomanage/printOrder.ts
export interface DeliveryAddressRequest {
recipientName?: string;
phone?: string;
zipcode: string;
address: string;
addressDetail?: string;
}| useEffect(() => { | ||
| const id = window.setTimeout(() => { | ||
| setRecipientName(deliveryAddress?.recipientName ?? ""); | ||
| setPhone(deliveryAddress?.phone ?? ""); | ||
| }, 0); | ||
|
|
||
| return () => window.clearTimeout(id); | ||
| }, [deliveryAddress?.recipientName, deliveryAddress?.phone]); |
There was a problem hiding this comment.
window.setTimeout(..., 0)을 사용하여 상태 업데이트를 스케줄링할 필요가 없어 보입니다. useEffect 내에서 직접 상태 설정 함수(setRecipientName, setPhone)를 호출하는 것이 코드를 더 간단하고 예측 가능하게 만듭니다.
useEffect(() => {
setRecipientName(deliveryAddress?.recipientName ?? "");
setPhone(deliveryAddress?.phone ?? "");
}, [deliveryAddress?.recipientName, deliveryAddress?.phone]);
| useEffect(() => { | ||
| if (!categories.length) return; | ||
|
|
||
| const id = window.setTimeout(() => { | ||
| setSelection({ | ||
| FILM: findOption(categories, "FILM", selectedOptions.filmType), | ||
| PRINT_METHOD: findOption( | ||
| categories, | ||
| "PRINT_METHOD", | ||
| selectedOptions.printMethod, | ||
| ), | ||
| PAPER: findOption(categories, "PAPER", selectedOptions.paperType), | ||
| SIZE: findOption(categories, "SIZE", selectedOptions.size), | ||
| PRINT_TYPE: findOption( | ||
| categories, | ||
| "PRINT_TYPE", | ||
| selectedOptions.frameType, | ||
| ), | ||
| }); | ||
| }, 0); | ||
|
|
||
| return () => window.clearTimeout(id); | ||
| }, [categories, selectedOptions]); |
There was a problem hiding this comment.
이 useEffect에서도 window.setTimeout(..., 0)을 사용하고 있는데, 특별한 이유가 없다면 불필요합니다. setTimeout 없이 직접 setSelection을 호출해도 동일하게 동작하며 코드가 더 간결해집니다.
//뒤로가기/새로고침 복원: store.selectedOptions(코드) -> DropDownOption
useEffect(() => {
if (!categories.length) return;
setSelection({
FILM: findOption(categories, "FILM", selectedOptions.filmType),
PRINT_METHOD: findOption(
categories,
"PRINT_METHOD",
selectedOptions.printMethod,
),
PAPER: findOption(categories, "PAPER", selectedOptions.paperType),
SIZE: findOption(categories, "SIZE", selectedOptions.size),
PRINT_TYPE: findOption(
categories,
"PRINT_TYPE",
selectedOptions.frameType,
),
});
}, [categories, selectedOptions]);
| const t = window.setTimeout(() => { | ||
| setIsQuoting(true); | ||
| }, 0); | ||
|
|
||
| quotePrintPrice(request) | ||
| .then((res) => { | ||
| if (quoteRequestId.current === id) setQuote(res.data); | ||
| if (quoteRequestId.current !== id) return; | ||
|
|
||
| //store에 바로 반영 | ||
| setTotalPrice(res.data.totalAmount ?? 0); | ||
| }) | ||
| .catch((err) => { | ||
| console.error("견적 조회 실패:", err); | ||
| if (quoteRequestId.current !== id) return; | ||
| setTotalPrice(0); | ||
| }) | ||
| .finally(() => { | ||
| if (quoteRequestId.current !== id) return; | ||
| window.setTimeout(() => setIsQuoting(false), 0); | ||
| }); | ||
| }, [buildQuoteRequest]); | ||
|
|
||
| // 사이즈·인화유형 미선택 시 견적 표시하지 않음 | ||
| const displayedQuote = selection.SIZE && selection.PRINT_TYPE ? quote : null; | ||
| return () => window.clearTimeout(t); |
There was a problem hiding this comment.
isQuoting 상태를 설정하기 위해 window.setTimeout을 사용하는 것은 불필요합니다. useEffect의 시작 부분에서 setIsQuoting(true)를, finally 블록에서 setIsQuoting(false)를 직접 호출하는 것이 더 간단하고 명확한 방법입니다.
if (!request) return;
const id = ++quoteRequestId.current;
setIsQuoting(true);
quotePrintPrice(request)
.then((res) => {
if (quoteRequestId.current !== id) return;
//store에 바로 반영
setTotalPrice(res.data.totalAmount ?? 0);
})
.catch((err) => {
console.error("견적 조회 실패:", err);
if (quoteRequestId.current !== id) return;
setTotalPrice(0);
})
.finally(() => {
if (quoteRequestId.current !== id) return;
setIsQuoting(false);
});
There was a problem hiding this comment.
SHOULD: 해당 부분 race condition이 생길 수 있을거 같아 추후 확인 부탁드려요!
3 participants
🔀 Pull Request Title
현상관리 페이지 2차 QA 반영
🎞️ 주요 코드 설명
주제1
주제2
📌 PR 설명
이번 PR에서 어떤 작업을 했는지 요약해주세요.
📷 스크린샷
UI 변경이 있을 경우 스크린샷을 첨부해주세요.