Conversation
| @@ -0,0 +1,29 @@ | |||
| import styles from "../../../../../../globalComponents/Modal/ModalChildren.module.css"; | |||
There was a problem hiding this comment.
폴더구조가 맥락별로 깔끔하게 바뀌었지만, 그 깊이가 어마어마해졌습니다 ㅋㅋㅋㅋㅋ
자주 있는 일인데요, TypeScript의 alias로 해결할 수 있습니다.
찾아보면 stackOverflow에 많이 있으니 한번 알아보시면 좋을 것 같습니다!
There was a problem hiding this comment.
사실 저도 PR올리면서 이게 맞나 싶었는데 다음 과제가 프로젝트를 ts로 변경하는거여서 아래의 내용도 다 포함해서 리팩토링하고 다음 PR엔 깔끔하게 보이도록 하겠습니다.🥲
| const [onModal, setOnModal] = useState(false); | ||
| const [modalContent, setModalContent] = useState(null); | ||
|
|
||
| const handleClickModal = (actionType) => { | ||
| const actionTypes = { | ||
| deleteLink: <LinkDeleteForm />, | ||
| addLink: <LinkAddToFolderForm />, | ||
| }; | ||
|
|
||
| setModalContent(actionTypes[actionType]); | ||
| setOnModal(true); | ||
| }; |
There was a problem hiding this comment.
Modal을 띄워주는 state와 Modal의 내용을 정해주는 state가 따로있네요.
하나만 있어도 될 것 같은데 줄여볼 수 있지 않을까요?
There was a problem hiding this comment.
DeleteModal, xxModal 등 여러 Modal이 있는데, 이 Modal 컴포넌트는 전혀 사용되지 않고 있네요.
Modal 컴포넌트 자체가 좀 더 확장성이 있어서 다른 구체화된 Modal 들의 수고를 덜어 줄 수 있지 않을까요?
확장을 위한 prop을 몇개 더 추가해줄 수도 있고, children에 구체화된 컴포넌트를 넣을 수도 있을 것 같네요.
There was a problem hiding this comment.
여러 xxForm들도 하나의 Form 컴포넌트로 추상화해서 사용하면 css나 로직의 중복을 줄일 수 있을 것 같네요.
|
|
||
| useEffect(() => { | ||
| if (window.Kakao && !window.Kakao.isInitialized()) { | ||
| window.Kakao.init("a841341da0099a5d1291638b48030745"); |
There was a problem hiding this comment.
Kakao를 다루기 위한 Key가 그대로 노출 돼있네요.
현재는 상관없지만, 상용 앱이면 정말 위험합니다. (실제로 예전 회사에서 이거 때문에 AWS자원 2천만원 털렸습니다.)
카카오 뿐만아니라, 모든 비밀 key들에도 해당하는 내용입니다.
시스템 환경변수에 숨겨 저장하고 그것을 가져와 사용할 수 있도록 하는 습관을 들이는 것이 좋습니다.
DotEnv를 한번 찾아보세요.
There was a problem hiding this comment.
카카오는 카카오 것만, 페북은 페북 것만 나눠도 좋을 것 같네요.
| const currentUrl = | ||
| window.location.origin + location.pathname + location.search; |
There was a problem hiding this comment.
currentUrl도 hook이나 함수로 숨겨서 제공해주면 코드가 더 깔끔해지겠네요.
요구사항
기본
주요 변경사항
멘토에게