-
Notifications
You must be signed in to change notification settings - Fork 46
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
[조혜진] Week12 #374
The head ref may contain hidden characters: "part2-\uC870\uD61C\uC9C4-week12"
[조혜진] Week12 #374
Conversation
import styles from "./Cards.module.css"; | ||
import Modal from "../Modal/Modal"; | ||
|
||
function Cards({ items }) { |
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.
아래처럼 props에 대한 타입들도 지정해주면 좋을 것 같아요.
function Cards({ items }) { | |
interface CardsProps { | |
items : Card[] | |
} | |
function Cards({ items } : CardsProps) { |
}; | ||
} | ||
|
||
type ModalType = "addFolder" | "share" | "edit" | "deleteFolder" | null; |
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.
이런 modal 타입은 보통 상수로 작성하는 것이 일반적이니 참고해주세요 (e.g. ADD_FOLDER)
type ModalType = "addFolder" | "share" | "edit" | "deleteFolder" | null; | ||
|
||
function FolderMain() { | ||
const [activeButton, setActiveButton] = useState("전체"); |
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.
"전체" 라는 문자열도 공통으로 사용되고 있으니 상수로 관리해서 타입을 지정해주면 더 안전할 것 같습니다.
|
||
const initKakaoSDK = () => { | ||
if (!window.Kakao.isInitialized()) { | ||
window.Kakao.init("bab4fbe2388df19be8aee73ca45a5cef"); |
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.
이런 api 키는 노출이 되면 안되기 때문에 이런 키 값은 환경변수로 관리하는 것이 일반적이랍니다!
</div> | ||
)} | ||
</div> | ||
<img |
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.
요런 부분은 button으로 작업하는게 적절할 것 같습니다!
setPopoverIndex(index); | ||
}; | ||
|
||
const handleClosePopover = (e) => { |
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.
이벤트 타입들도 지정해주면 좋을 것 같습니다.
height={24} | ||
alt='close' | ||
onClick={handleClearSearch} | ||
tabIndex={0} |
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.
tabIndex를 추가한 것 잘하셨습니다.👍
하지만 클릭 가능하다면 button태그를 사용하는 것이 가장 좋겠죠?!
onSubmit?: (value: string) => void; | ||
} | ||
|
||
interface Link { |
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.
FolderMain.tsx에 같은 타입이 있는 것 같은데요. 여러 컴포넌트에서 공유할 수 있는 type폴더를 만들어 사용하면 좋을 것 같습니다.
혜진님, 파트2 하시느라 고생 많으셨어요. 워낙 잘해주셔서 제가 코멘트 드리는 내용들이 많지 않았던 것 같았는데요..! |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게