-
Notifications
You must be signed in to change notification settings - Fork 2
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
[FE] feat: 사용자가 작성하던 리뷰를 로컬 스토리지와 연동하고, 복원하는 기능 추가 #987
base: develop
Are you sure you want to change the base?
Changes from all commits
d911669
2da4262
c548e89
4235e7e
3aeb37c
995527a
e9022d0
4993856
28c7af5
2624752
6a37f53
badc5c3
71e1b34
ca5ce80
25bb3c9
61647cc
daf1183
b590349
1195b04
3a5ab61
2a44ab9
c7c4342
567b119
d5947ae
339c387
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 |
---|---|---|
@@ -1,3 +1,10 @@ | ||
export const STORED_DATA_NAME = { | ||
selectedCategories :'selectedCategories', | ||
answerValidations: 'answerValidations', | ||
answers: 'answers', | ||
visitedCardIdList: 'visitedCardIdList' | ||
} as const; | ||
|
||
Comment on lines
+1
to
+7
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.
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. 속성값을 불변으로 유지하고, 겸사겸사 타입 추론도 각 문자열 리터럴로 해 줘서 상수라는 의미에 적합한 것 같았습니다. |
||
export const LOCAL_STORAGE_KEY = { | ||
isHighlightEditable: 'isHighlightEditable', | ||
isHighlightError: 'isHighlightError', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,8 @@ | ||
import { ReviewWritingCardQuestion } from '@/types'; | ||
import { useEffect } from 'react'; | ||
import { useRecoilValue } from 'recoil'; | ||
|
||
import { answerMapAtom } from '@/recoil'; | ||
import { ReviewWritingAnswer, ReviewWritingCardQuestion } from '@/types'; | ||
|
||
import useAboveSelectionLimit from './useAboveSelectionLimit'; | ||
import useCheckTailQuestionAnswer from './useCheckTailQuestionAnswer'; | ||
|
@@ -16,7 +20,9 @@ interface UseMultipleChoiceProps { | |
const useMultipleChoice = ({ question, handleModalOpen }: UseMultipleChoiceProps) => { | ||
const { isAnsweredTailQuestion } = useCheckTailQuestionAnswer({ question }); | ||
|
||
const { selectedOptionList, isSelectedCheckbox, updateSelectedOptionList } = useOptionSelection(); | ||
const { selectedOptionList, isSelectedCheckbox, updateSelectedOptionList, initSelectedOptionList } = | ||
useOptionSelection(); | ||
const answerMap = useRecoilValue(answerMapAtom); | ||
|
||
const { updateAnswerState } = useUpdateMultipleChoiceAnswer({ question }); | ||
|
||
|
@@ -34,6 +40,30 @@ const useMultipleChoice = ({ question, handleModalOpen }: UseMultipleChoiceProps | |
}, | ||
); | ||
|
||
interface FindSelectedOptionIdsParams { | ||
answerMap: Map<number, ReviewWritingAnswer> | null; | ||
questionId: number; | ||
} | ||
|
||
// 로컬 스토리지에 저장했던 답변으로부터, questionId를 통해 해당 질문의 selectedOptionIds를 찾는 함수 | ||
const findSelectedOptionIds = ({ answerMap, questionId }: FindSelectedOptionIdsParams) => { | ||
if (!answerMap) return null; | ||
|
||
const selectedItem = answerMap.get(questionId); | ||
return selectedItem ? selectedItem.selectedOptionIds : null; | ||
}; | ||
|
||
// 저장된 객관식 답변이 있다면 복원 | ||
useEffect(() => { | ||
if (!answerMap || answerMap.size === 0) return; | ||
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. 선택된 객관식 문항 상태를 관리하는 훅인 useOptionSelection이 아니라, useMultipleChoice에서 로컬 상태에 저장된 값에서 해당 질문에서 선택된 객관식 문항들을 useOptionSelection으로 넘겨주는 플로우를 선택한 이유가 있나요? 리뷰 작성 페이지를 리팩토링할 때, 구조가 복잡해져서 최대한 훅,컴포넌트의 역할을 작게 나누려고 했어요. useOptionSelection은 선택된 객관식 문항 상태를 관리하고, useMultipleChoice는 객관식의 답변 선택 액션과 useOptionSelection에서 관리하는 상태를 change이벤트에서 업데이트할 수 있게 하는 역할로 구현했어요. 제가 봤을때는, 책임분리에 따르면 useOptionSelection에서 로컬 스토리지에 저장된 데이터를 바탕으로 selectedOptionList의 상태를 업데이트하는 게 좋을 것 같아요. 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. 선택된 객관식 문항 상태와 관련된 로직이라 저도 useOptionSelection에서 복원하는 것이 나을 것 같다 생각합니다 |
||
if (selectedOptionList.length > 0) return; | ||
|
||
const questionId = question.questionId; | ||
const selectedOptionIds = findSelectedOptionIds({ answerMap, questionId }); | ||
|
||
if (selectedOptionIds) initSelectedOptionList([...selectedOptionIds]); | ||
}, [answerMap, question]); | ||
|
||
const handleCheckboxChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
const { id, checked } = event.currentTarget; | ||
const optionId = Number(id); | ||
|
@@ -88,4 +118,5 @@ const useMultipleChoice = ({ question, handleModalOpen }: UseMultipleChoiceProps | |
unCheckCategoryOptionId, | ||
}; | ||
}; | ||
|
||
export default useMultipleChoice; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,10 @@ const useOptionSelection = () => { | |
checked: boolean; | ||
} | ||
|
||
const initSelectedOptionList = (newSelectedOptionList: number[]) => { | ||
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. init이라는 단어가, 초기화 의미라서 선택 문항들이 아무것도 선택되지 않은 초기값 형태가 생각나네요. 구현의도를 보면 로컬 스토리지에 저장된 값으로 상태를 업데이트하는 거라서, 함수명이 구현의도를 더 잘 담았으면 좋겠어요. |
||
setSelectedOptionList(newSelectedOptionList); | ||
}; | ||
|
||
/** | ||
* checkbox의 change 이벤트에 따라 새로운 selectedOptionList를 반환하는 함수 | ||
*/ | ||
|
@@ -37,6 +41,7 @@ const useOptionSelection = () => { | |
selectedOptionList, | ||
isSelectedCheckbox, | ||
updateSelectedOptionList, | ||
initSelectedOptionList, | ||
}; | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { STORED_DATA_NAME } from '@/constants'; | ||
import { useSearchParamAndQuery } from '@/hooks'; | ||
|
||
const useDeleteReviewInLocalStorage = () => { | ||
const { param: reviewRequestCode } = useSearchParamAndQuery({ | ||
paramKey: 'reviewRequestCode', | ||
}); | ||
|
||
const deleteReviewDataInLocalStorage = (key: keyof typeof STORED_DATA_NAME) => { | ||
localStorage.removeItem(`${STORED_DATA_NAME[key]}_${reviewRequestCode}`); | ||
}; | ||
|
||
const deleteAllReviewDataInLocalStorage = () => { | ||
Object.values(STORED_DATA_NAME).forEach((key) => { | ||
localStorage.removeItem(`${STORED_DATA_NAME[key]}_${reviewRequestCode}`); | ||
}); | ||
}; | ||
Comment on lines
+13
to
+17
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.
localStorage.removeItem(`${key}_${reviewRequestCode}`); 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. 쑤쑤가 남겨준 코멘트에 각각 코멘트를 달고 싶은데 바로 작성할 수 있는 곳이 여기밖에 없네요 😂 로컬스토리지오 확실히 리뷰미 플랫폼에서 로컬 스토리지를 사용할 때의 가장 큰 단점이 상수 객체 관련앞으로 로컬에 저장할 값이 많아지지는 않을 것 같아 그냥 if문 조건일단 두 번째 조건은, 상태를 로컬 스토리지에서 가져와서 업데이트해놔도 최종적으로 기존의 초기화 로직이 실행돼서 최종 렌더링을 빈 배열로 하던 (=UI에 복원 안 되고 로컬 데이터도 날아감)문제를 해결하기 위해 둔 조건인데 지금은 또 안 먹히네요ㅋㅋㅋㅋㅠ 아마 앞 조건도 비슷한 이유로 놔뒀던 것 같아요. 만든 지 조금 지나니까 기억이 가물가물한데 버그 고치면서 수정해볼게요~ |
||
|
||
return { | ||
deleteReviewDataInLocalStorage, | ||
deleteAllReviewDataInLocalStorage, | ||
}; | ||
}; | ||
|
||
export default useDeleteReviewInLocalStorage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import { useCallback } from 'react'; | ||
import { useSetRecoilState } from 'recoil'; | ||
|
||
import { STORED_DATA_NAME } from '@/constants'; | ||
import { useSearchParamAndQuery } from '@/hooks'; | ||
import { CARD_FORM_MODAL_KEY } from '@/pages/ReviewWritingPage/constants'; | ||
import { selectedCategoryAtom, answerMapAtom, answerValidationMapAtom } from '@/recoil'; | ||
|
||
const useRestoreFromLocalStorage = () => { | ||
const setSelectedCategory = useSetRecoilState(selectedCategoryAtom); | ||
const setAnswerMap = useSetRecoilState(answerMapAtom); | ||
const setAnswerValidation = useSetRecoilState(answerValidationMapAtom); | ||
|
||
const { param: reviewRequestCode } = useSearchParamAndQuery({ | ||
paramKey: 'reviewRequestCode', | ||
}); | ||
|
||
const initialModalsState = { | ||
[CARD_FORM_MODAL_KEY.restoreConfirm]: | ||
!!localStorage.getItem(`${STORED_DATA_NAME.selectedCategories}_${reviewRequestCode}`) || | ||
!!localStorage.getItem(`${STORED_DATA_NAME.answers}_${reviewRequestCode}`) || | ||
!!localStorage.getItem(`${STORED_DATA_NAME.answerValidations}_${reviewRequestCode}`), | ||
Comment on lines
+20
to
+22
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. 코드기 길어서, 가독성을 위해서 따로 변수로 만들어주면 좋을 것 같아요 |
||
}; | ||
|
||
const restoreData = useCallback(() => { | ||
const storedSelectedCategories = localStorage.getItem( | ||
`${STORED_DATA_NAME.selectedCategories}_${reviewRequestCode}`, | ||
); | ||
const storedAnswerValidations = localStorage.getItem(`${STORED_DATA_NAME.answerValidations}_${reviewRequestCode}`); | ||
const storedAnswers = localStorage.getItem(`${STORED_DATA_NAME.answers}_${reviewRequestCode}`); | ||
|
||
const selectedCategories = storedSelectedCategories ? JSON.parse(storedSelectedCategories) : null; | ||
const answerValidations = storedAnswerValidations ? new Map(JSON.parse(storedAnswerValidations)) : new Map(); | ||
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. 답변 유효성과 강점 선택을 로컬 스토리지에 담지 않는 것이 어떨까요? 사용자가 로컬 스토리지의 데이터를 변경해서, 답변과 답변 유효성, 강점 선택 사이의 데이터 불일치가 일어날 가능성이 높아요. 그래서 로컬 스토리지에는 답변만 담은 후 로컬 스토리지에 저장된 답변을 바탕으로 답변 유효성과 강점 선택 여부를 검사한 후 전역 상태에 업데이트하는 게 맞다고 봐요. 아마 이렇게 변경한다면, 답변 유효성을 검사하는 로직을 객관식과 주관식 답변을 관리하는 훅에서 따로 빼서 재사용가능한 유틸함수로 바꾸어야할거에요. 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.
실시간 업데이트 도중 데이터 동기화 문제가 생길 수 있다는 의미인지 궁금합니다. 동기화 리스크는 실시간 저장이라면 있을 수밖에 없지만 저장하는 데이터가 많아지면 더 리스크가 커지겠네요. 하지만 이것과 별개로 로컬 스토리지에 저장하는 값을 줄이면 좋겠다는 점은 동감합니다 😂 구현 초창기에 일단 전역 상태들은 어차피 전역이니까~ 싶어서 다 로컬에 저장하고 시작했더니 점점 값이 많아졌네요 ㅠ_ㅠ 일단 바다의 제안대로 모든 값의 근간인 answerMap과 지역 상태들만 저장/복원한 다음 차차 연관된 상태들을 업데이트해주는 방향으로 리팩토링해보겠습니다! |
||
const answers = storedAnswers ? JSON.parse(storedAnswers) : null; | ||
|
||
setSelectedCategory(selectedCategories); | ||
setAnswerValidation(answerValidations); | ||
setAnswerMap(new Map(answers)); | ||
}, [reviewRequestCode, setSelectedCategory, setAnswerMap, setAnswerValidation]); | ||
|
||
return { restoreData, initialModalsState }; | ||
}; | ||
|
||
export default useRestoreFromLocalStorage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
import { useCallback, useEffect } from 'react'; | ||
import { useRecoilValue } from 'recoil'; | ||
|
||
import { STORED_DATA_NAME } from '@/constants'; | ||
import { useSearchParamAndQuery } from '@/hooks'; | ||
import { answerMapAtom, answerValidationMapAtom, selectedCategoryAtom } from '@/recoil'; | ||
|
||
/** | ||
* 리뷰와 관련된 데이터들을 실시간으로 로컬 스토리지에 저장하는 훅 | ||
*/ | ||
const useSaveReviewToLocalStorage = () => { | ||
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. atom의 effects를 사용하지 않고, useEffect로 로컬 스토리지에 저장하는 훅을 만든 이유가 있나요? |
||
const selectedCategory = useRecoilValue(selectedCategoryAtom); | ||
const answerMap = useRecoilValue(answerMapAtom); | ||
const answerValidation = useRecoilValue(answerValidationMapAtom); | ||
|
||
const { param: reviewRequestCode } = useSearchParamAndQuery({ | ||
paramKey: 'reviewRequestCode', | ||
}); | ||
|
||
const getCurrentSelectedCategory = useCallback(() => { | ||
if (!selectedCategory || selectedCategory.length === 0) return null; | ||
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. null을 반환하는 이유가 있나요? |
||
|
||
return selectedCategory; | ||
}, [selectedCategory]); | ||
|
||
const getCurrentAnswerValidation = useCallback(() => { | ||
if (!answerValidation || answerValidation.size === 0) return null; | ||
|
||
const plainObjectAnswers = Array.from(answerValidation.entries()); | ||
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.
|
||
return plainObjectAnswers.length > 0 ? plainObjectAnswers : null; | ||
}, [answerValidation]); | ||
|
||
const getCurrentAnswers = useCallback(() => { | ||
if (!answerMap || answerMap.size === 0) return null; | ||
|
||
const plainObjectAnswers = Array.from(answerMap.entries()); | ||
return plainObjectAnswers.length > 0 ? plainObjectAnswers : null; | ||
}, [answerMap]); | ||
|
||
const saveToLocalStorage = useCallback(() => { | ||
const selectedCategories = getCurrentSelectedCategory(); | ||
const answers = getCurrentAnswers(); | ||
const answerValidations = getCurrentAnswerValidation(); | ||
|
||
if (selectedCategories) { | ||
localStorage.setItem( | ||
`${STORED_DATA_NAME.selectedCategories}_${reviewRequestCode}`, | ||
JSON.stringify(selectedCategories), | ||
); | ||
} | ||
|
||
if (answerValidations) { | ||
localStorage.setItem( | ||
`${STORED_DATA_NAME.answerValidations}_${reviewRequestCode}`, | ||
JSON.stringify(answerValidations), | ||
); | ||
} | ||
|
||
if (answers) { | ||
localStorage.setItem(`${STORED_DATA_NAME.answers}_${reviewRequestCode}`, JSON.stringify(answers)); | ||
} | ||
}, [getCurrentSelectedCategory, getCurrentAnswers, getCurrentAnswerValidation, reviewRequestCode]); | ||
|
||
// 로컬 스토리지 동기화 | ||
useEffect(() => { | ||
saveToLocalStorage(); | ||
}, [saveToLocalStorage]); | ||
}; | ||
|
||
export default useSaveReviewToLocalStorage; |
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.
기존에 LOCAL_STORAGE_KEY라는 변수가 있는데 따로 만든 이유가 있나요?
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.
파일명이 storageKey라서, 일단 이 상수 객체가 어떤 용도인지 간접적으로 알 수 있다고 생각했어요. 그렇다면 모든 key값들을 하나의 객체에 몰아두기보다 용도별로 다른 객체를 사용하는 게 좋을 것 같아 분리했습니다.
다만 이 파일대로라면 '그럼
STORED_DATA_NAME
은 로컬 스토리지 키가 아닌가?' 하는 혼동이 올 수 있겠네요 😅스토리지에 저장할 값이 크게 늘어날 일은 별로 없을 것 같아
LOCAL_STORAGE_KEY
하나로 관리하는 것도 괜찮아 보이기는 하는데, 바다의 자세한 의견이 궁금합니다~!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.
개인적으로
LOCAL_STORAGE_KEY
객체 하나로 관리하는 것이 좋을 것 같아요.처음에 해당 폴더로 들어오게 되면
STORED_DATA_NAME, LOCAL_STORAGE_KEY, SESSION_STORAGE_KEY
가 있는데STORED_DATA_NAME
이 로컬 스토리지 키인지? 다른 키인지? 헷갈릴 것 같아요. 저 네이밍만 보고 로컬 스토리지 키라는 걸 추측해야 하니까 하나로 관리하는 것이 좋을 것 같아요!만약 용도별로 나누고 싶다면 아래처럼 해도 괜찮을 것 같아요😊