-
Notifications
You must be signed in to change notification settings - Fork 44
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
[조혜진] Week11 #356
The head ref may contain hidden characters: "part2-\uC870\uD61C\uC9C4-week11"
[조혜진] Week11 #356
Conversation
import { toast, ToastContainer } from "react-toastify"; | ||
import "react-toastify/dist/ReactToastify.css"; | ||
|
||
function Modal({ |
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.
공통으로 사용할 수 있는 모달 컴포넌트를 정말 잘 만들어주셨네요!! 👍👍👍 현업에서도 비슷하게 사용하고 있답니다.
if (inputValue.trim() !== "") { | ||
setIsError(false); | ||
} else setIsError(true); |
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.
inputValue.trim() 값이 빈 문자열인 경우에는 false 아닌 경우에는 true를 리턴하고 싶다면 아래와 같이 작성할수도 있답니다!
if (inputValue.trim() !== "") { | |
setIsError(false); | |
} else setIsError(true); | |
setIsError(!!inputValue.trim()); |
<div className={styles.modal_background} onClick={onClose}> | ||
<div className={styles.modal_container} onClick={(e) => e.stopPropagation()}> | ||
<div className={styles.title_container}> | ||
<h1 className={styles.title}>{title}</h1> |
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.
모달의 타이틀을 h1로 가져가기에는 페이지에서 h1을 사용할 수 있어 무리가 있어 보입니다!
{btnText} | ||
</button> | ||
)} | ||
{share && folderList && ( |
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 컴포넌트에 넣기보다는 메인 콘텐츠는 children 컴포넌트로 받아서 렌더링하는 구조로 작업한 후에 ShareModal 컴포넌트를 만들어서 children으로 넘겨주는 방식이 좋을 것 같습니다~! 요건 멘토링 시간에 이야기 나눠 볼게요!
</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.
닫기버튼 모달 마지막에 위치시킨 것 좋네요! 접근성 측면에서 닫기 버튼은 마지막에 위치하는 것이 좋답니다.
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.
이미지 태그대신 버튼 태그를 사용하면 굳이 keydown 이벤트를 사용하지 않아도 될 것 같습니다!
align-items: center; | ||
justify-content: center; | ||
z-index: 10; | ||
left: 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.
보통 모달은 position: fixed 에 top, left, right, bottom 을 모두 0으로 주는 방식으로 작업을 많이 하는데요. 이렇게 작성할 경우 inset: 0;
으로 간단히 작성할 수 있습니다. top, left를 0으로 주고 width, height를 100%로 주는 것도 좋지만 더 간단히 작성할 수 있는 방법이 있으니 참고해주세요!
)} | ||
{btnColor === "submit" && ( | ||
<button | ||
className={`${styles.submit_button} ${isError ? styles.disabled : ""}`} |
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.
disabled 처리는 클래스로 추가하는 것 대신 button 태그의 disabled 속성에 true를 주는 것 어떨까요?
<ul className={styles.list}> | ||
{folderList && | ||
folderList.data.map((folder) => ( | ||
<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.
여러가지를 선택할 수 있는 스펙 같은 경우는 li태그에 작업하지 않고 input의 checkbox로 작업하는 것이 더 적절할 것 같습니다~!
요구사항
기본
주요 변경사항
스크린샷
멘토에게