-
Notifications
You must be signed in to change notification settings - Fork 1
Fix/139 3차 QA 수정 #141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/139 3차 QA 수정 #141
Changes from all commits
7109c07
8206ed5
93c52d8
d30b03c
fae304f
da18261
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { motion, AnimatePresence } from 'framer-motion'; | |||||
| import cn from '@lib/cn'; | ||||||
| import useOutsideClick from '@hooks/useOutsideClick'; | ||||||
| import ChevronIcon from '@assets/svg/chevron'; | ||||||
| import { DropdownProps } from '@/types/dropdownTypes'; | ||||||
| import { DropdownProps, DropdownOption } from '@/types/dropdownTypes'; | ||||||
| // import CheckIcon from '@assets/svg/check'; | ||||||
|
|
||||||
| /** | ||||||
|
|
@@ -28,6 +28,7 @@ import { DropdownProps } from '@/types/dropdownTypes'; | |||||
| */ | ||||||
| export default function Dropdown<T extends string>({ | ||||||
| options, | ||||||
| optionData, | ||||||
| value, | ||||||
| onChange, | ||||||
| placeholder, | ||||||
|
|
@@ -47,22 +48,34 @@ export default function Dropdown<T extends string>({ | |||||
| const dropdownRef = useRef<HTMLDivElement>(null); | ||||||
| const buttonRef = useRef<HTMLButtonElement>(null); | ||||||
|
|
||||||
| // optionData가 제공되면 우선 사용, 없으면 options 사용 | ||||||
| const useOptionData = !!optionData; | ||||||
| const finalOptions = useOptionData | ||||||
| ? optionData! | ||||||
| : options.map(opt => ({ value: opt, label: opt })); | ||||||
|
|
||||||
| // 내/외부 상태 관리 판별 | ||||||
| const isControlled = value !== undefined; | ||||||
| const selectedValue = isControlled ? value : internalValue; | ||||||
|
|
||||||
| // 선택된 값에 해당하는 표시 텍스트 찾기 | ||||||
| const displayValue = useOptionData | ||||||
| ? finalOptions.find(opt => opt.value === selectedValue)?.label || '' | ||||||
| : selectedValue; | ||||||
|
|
||||||
| // 외부 클릭 감지 | ||||||
| useOutsideClick(dropdownRef, () => { | ||||||
| setIsOpen(false); | ||||||
| setFocusedIndex(-1); | ||||||
| }); | ||||||
|
|
||||||
| // 값 선택 핸들러 | ||||||
| const handleSelect = (option: T) => { | ||||||
| const handleSelect = (optionData: DropdownOption) => { | ||||||
| const selectedValue = optionData.value as T; | ||||||
| if (!isControlled) { | ||||||
| setInternalValue(option); | ||||||
| setInternalValue(selectedValue); | ||||||
| } | ||||||
| onChange?.(option); | ||||||
| onChange?.(selectedValue); | ||||||
| setIsOpen(false); | ||||||
| setFocusedIndex(-1); | ||||||
| buttonRef.current?.focus(); | ||||||
|
|
@@ -77,7 +90,7 @@ export default function Dropdown<T extends string>({ | |||||
| case ' ': | ||||||
| e.preventDefault(); | ||||||
| if (isOpen && focusedIndex >= 0) { | ||||||
| handleSelect(options[focusedIndex]); | ||||||
| handleSelect(finalOptions[focusedIndex]); | ||||||
| } else { | ||||||
| setIsOpen(!isOpen); | ||||||
| } | ||||||
|
|
@@ -88,7 +101,7 @@ export default function Dropdown<T extends string>({ | |||||
| if (!isOpen) { | ||||||
| setIsOpen(true); | ||||||
| } else { | ||||||
| setFocusedIndex((prev) => (prev < options.length - 1 ? prev + 1 : 0)); | ||||||
| setFocusedIndex((prev) => (prev < finalOptions.length - 1 ? prev + 1 : 0)); | ||||||
| } | ||||||
| break; | ||||||
|
|
||||||
|
|
@@ -97,7 +110,7 @@ export default function Dropdown<T extends string>({ | |||||
| if (!isOpen) { | ||||||
| setIsOpen(true); | ||||||
| } else { | ||||||
| setFocusedIndex((prev) => (prev > 0 ? prev - 1 : options.length - 1)); | ||||||
| setFocusedIndex((prev) => (prev > 0 ? prev - 1 : finalOptions.length - 1)); | ||||||
| } | ||||||
| break; | ||||||
|
|
||||||
|
|
@@ -149,7 +162,7 @@ export default function Dropdown<T extends string>({ | |||||
| truncateText && 'flex-1 truncate text-left', | ||||||
| )} | ||||||
| > | ||||||
| {selectedValue || placeholder} | ||||||
| {displayValue || placeholder} | ||||||
| </span> | ||||||
| <ChevronIcon | ||||||
| size={24} | ||||||
|
|
@@ -179,13 +192,13 @@ export default function Dropdown<T extends string>({ | |||||
| listboxClassName, | ||||||
| )} | ||||||
| > | ||||||
| {options.map((option, index) => { | ||||||
| const isSelected = option === selectedValue; | ||||||
| {finalOptions.map((optionItem, index) => { | ||||||
| const isSelected = optionItem.value === selectedValue; | ||||||
| const isFocused = index === focusedIndex; | ||||||
|
|
||||||
| return ( | ||||||
| <li | ||||||
| key={option} | ||||||
| key={`${optionItem.value}-${index}`} | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 키 값으로 배열 인덱스 사용을 개선해주세요. 배열 인덱스를 키로 사용하면 성능 문제가 발생할 수 있습니다. 다음과 같이 수정하는 것을 권장합니다: - key={`${optionItem.value}-${index}`}
+ key={optionItem.value}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| id={`dropdown-option-${index}`} | ||||||
| role='option' | ||||||
| aria-selected={isSelected} | ||||||
|
|
@@ -198,7 +211,7 @@ export default function Dropdown<T extends string>({ | |||||
| : 'hover:bg-gray-100', | ||||||
| optionClassName, | ||||||
| )} | ||||||
| onClick={() => handleSelect(option)} | ||||||
| onClick={() => handleSelect(optionItem)} | ||||||
| onMouseEnter={() => setFocusedIndex(index)} | ||||||
| > | ||||||
| {/* 아이콘 영역 */} | ||||||
|
|
@@ -211,7 +224,7 @@ export default function Dropdown<T extends string>({ | |||||
| /> | ||||||
| )} | ||||||
| </div> */} | ||||||
| <span>{option}</span> | ||||||
| <span>{optionItem.label}</span> | ||||||
| </li> | ||||||
| ); | ||||||
| })} | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
리뷰 생성 후 캐시 무효화 로직이 잘 구현되었습니다.
후기 작성 성공 시 관련 액티비티 쿼리들을 무효화하여 실시간 업데이트를 보장합니다. 예약 정보를 찾아 액티비티 ID를 전달하는 로직도 적절합니다.
다만
reservation을 찾지 못할 경우에 대한 방어 로직을 고려해보세요.선택적 개선사항으로 다음과 같은 방어 로직을 추가할 수 있습니다:
onSuccess: () => { // 성공 후 추가 쿼리 무효화 - if (reservation?.activity.id) { + if (reservation?.activity?.id) { invalidateActivityQueries(reservation.activity.id); + } else { + console.warn('예약 정보를 찾을 수 없어 캐시 무효화를 건너뜁니다.'); } },📝 Committable suggestion
🤖 Prompt for AI Agents