-
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
[ Style ] 설정 뷰 내의 경고 모달 뷰 구현 #227
Conversation
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.
고생 많으셨습니다~~
경고 모달이라는 의미에서 비슷한 맥락이기 때문에 variant를 주어서 구현해주셨군요.
흠 제 생각에는 이 3개의 모달은 스타일이 모두 달라서 variant
보다는 다른 컴포넌트다 라는 것을 나타내주면 좋을것 같은데요.
합성 컴포넌트 패턴으로 사용할수 있게 하면 어떨까 싶은데 다른 분들은 어떻게 생각하시나요? @KIMGEONHWI @Ivoryeee
const ModalContentsAlert = {
Logout: LogoutModal,
DeleteAccount: DeleteAccountModal,
Complete: CompleteModal,
}
export default ModalContentsAlert
<ModalWrapper>
<ModalContentsAlert.Logout/>
<ModalWrapper>
<ModalWrapper>
<ModalContentsAlert.DeleteAccount/>
<ModalWrapper>
<ModalWrapper>
<ModalContentsAlert.Complete/>
<ModalWrapper>
<p className="subhead-bold-22 flex justify-center text-white">[email protected] 계정이</p> | ||
<p className="subhead-bold-22 mb-[3rem] flex justify-center text-white"> | ||
영구적으로 삭제됩니다. 삭제하시겠습니까? | ||
</p> |
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.
p1: api 연결 시 해당 부분에는 실제 이메일을 넣어야되기 때문에 prop으로 받을 수 있게 변경 부탁드려요!
<p className="subhead-bold-22 flex justify-center text-white">{`${userEmail} 계정이`}</p>
<p className="subhead-bold-22 mb-[3rem] flex justify-center text-white">
영구적으로 삭제됩니다. 삭제하시겠습니까?
</p>
{error && ( | ||
<div className="mt-[1rem]"> | ||
<p className="body-reg-16 mb-[3rem] text-error-01">계속하려면 “[email protected]”을(를) 입력하세요.</p> | ||
</div> | ||
)} |
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.
p1: 이 부분의 이메일도 prop으로 받을수 있게 변경 부탁드립니다~~
|
||
const renderDeleteAccountCompleteModal = () => ( | ||
<div className="w-[47.2rem]"> | ||
<p className="subhead-bold-22 flex justify-center text-white">[email protected] 계정이</p> |
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.
p1: 이 부분도 이메일 prop으로 받을수있게 부탁드려요~
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 handleSubmit = (e: FormEvent) => { | ||
e.preventDefault(); | ||
|
||
if (String(inputRef.current?.value) !== '[email protected]') { |
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.
p1: 이 부분도 prop
으로 변경해 주세요!
type="text" | ||
placeholder="[email protected]" | ||
onChange={handleInputChange} | ||
className={`${error ? errorStyle : ''} subhead-med-18 placeholder-text-gray-03 w-full rounded-[5px] bg-gray-bg-02 p-[1rem] text-white outline-none`} |
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.
p5: 에러 부분 스타일을 분리해 두신 이유가 있을까요? 테일윈드를 사용하는 만큼 스타일로 인해 가독성이 크게 저하되지 않는다면 클래스 내에 직관적으로 나타내도 좋을 것 같아요!
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.
p5: 에러 부분 스타일을 분리해 두신 이유가 있을까요? 테일윈드를 사용하는 만큼 스타일로 인해 가독성이 크게 저하되지 않는다면 클래스 내에 직관적으로 나타내도 좋을 것 같아요!
저도 이 의견에 동의합니다!
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.
반영했습니다~
저는 좋은 의견인 것 같습니다!! |
말씀 주신대로 합성 컴포넌트를 사용한다면 역할 분리와 재사용성 측면에서 더 좋은 구현이 될 것 같아요! |
<div className="mb-[3rem] flex justify-center"> | ||
<IconWarning width="5.4rem" /> | ||
</div> | ||
<p className="subhead-bold-22 flex justify-center text-white">[email protected] 계정이</p> |
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.
p1: 이 부분의 이메일도 prop으로 받을수 있게 변경 부탁드립니다!
<input | ||
ref={inputRef} | ||
type="text" | ||
placeholder="[email protected]" |
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.
p1: 이 부분의 이메일도 prop으로 받을수 있게 변경 부탁드립니다!
type="text" | ||
placeholder="[email protected]" | ||
onChange={handleInputChange} | ||
className={`${error ? errorStyle : ''} subhead-med-18 placeholder-text-gray-03 w-full rounded-[5px] bg-gray-bg-02 p-[1rem] text-white outline-none`} |
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.
p5: 에러 부분 스타일을 분리해 두신 이유가 있을까요? 테일윈드를 사용하는 만큼 스타일로 인해 가독성이 크게 저하되지 않는다면 클래스 내에 직관적으로 나타내도 좋을 것 같아요!
저도 이 의견에 동의합니다!
|
||
const renderLogoutModal = () => ( | ||
<> | ||
<p className="subhead-bold-22 flex justify-center text-white">[email protected] 계정이</p> |
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.
p1: 이 부분의 이메일도 prop으로 받을수 있게 변경 부탁드립니다!
저도 합성 컴포넌트 패턴을 적용하는 것이 좋아보여요. 한컴포넌트내에서 상반되는 디자인들을 모두 처리하려고 하다보니, 코드 가독성도 많이 떨어지는 거 같습니다. |
제가 말씀을 잘못드렸는데 위 방식은 합성 컴포넌트 방식이라기보다는 단순히 컴포넌트를 매핑해서 export 하는 방식이라고 할 수 있겠네요! 이점 유의 부탁드립니다 |
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.
logout, DeleteAccount, complete 컴포넌트를 모두 파일로 분리했주셨군요!
개인적인 의견으로 3개의 컴포넌트를 모두 ModalContentAlert에 작성하는게 관련성 있는것을 더 가까히 배치해서 가독성을 높일 수 있지 않을까 싶어요.
코드의 길이가 매우 길어진다면 분리해도 무방하겠지만 지금 같은 상황에서는 하나의 파일에서 관리하는게 어떨까 싶은데 어떻게 생각하시나요?!
@KIMGEONHWI @Ivoryeee 다른 분들 의견도 궁금합니다.
좋은 의견 감사합니다! 컴포넌트가 길어져 가독성을 해칠 수 있다고 생각해 분리했지만, ModalContentsAlert 폴더 내에서 관리하기 때문에 관련성 있는 코드가 모여 있다는 점에서는 가독성 측면에서 크게 차이가 없다고 생각했습니다. |
저도 한서님의 의견에 동의합니다! 폴더구조 변경 작업 들어가게 되면, 동일 폴더 내에서 관리될 것이라 가독성 측면에서는 문제 없을 것이라고 판단됩니다. 또한, 추후에 경고모달이 추가되게 되면 코드가 길어져서 오히려 관리하기 힘들수도 있을것 같아요. |
저도 동의합니다! 파일을 번갈아 보며 코드를 확인하는 것도 비용이기에 현재 코드 상으로는 같은 파일 내에 위치시키는 게 가독성을 높일 수 있을 것 같습니다. 추후 코드가 길어지게 돼 코드 가독성 저하가 우려된다면 그때 특정 파일로 분리해도 좋을 것 같아요! |
* [style] 사이드바 모립세트 뷰 추가 (#220) * [style] 모립세트 경로 추가 (#220) * [style] tab 스타일 변경사항 반영 (#220) * [style] 모립세트 페이지 퍼블리싱 (#220) * [ Style ] 모립세트 컴포넌트 구현 (#225) * refactor: SideBoxTemporary 컴포넌트 삭제 * feat: MoribSetCOntainer 컴포넌트 구현 * style: 허용할 사이트 영역 홀,짝수번째 배경화면 다르게 수정 * feat: 취소 버튼 온클릭 핸들러 추가 * refactor: MoribSetContainer가 열린 상태에서 다른 요소 클릭 가능하게 수정 * feat: 배열 별도의 상수 파일로 분리 및 기능 구현 * refactor: 아이콘 url도 ALLOW_SITE_LIST에서 관리 * codereview: 대원 코드리뷰 반영 * refactor: 모립세트 워딩 허용서비스 세트로 수정 * codereview: 대원, 상아 코드리뷰 반영 * feat: 툴팁 생성 * refactor: 타이머페이지 툴팁 삭제 * refacotr: 툴팁 isAllowedServiceVisible이 true일 때만 화면에 등장하도록 수정 * codereview: BoxAllowedService에서 PopoverAllowedService로 네이밍 변경 * [ Style ] 설정 뷰 내의 경고 모달 뷰 구현 (#227) * feat: alert모달 내 버튼 공통 컴포넌트 생성 (#222) * feat: ModalContentsAlert 생성 (#222) * feat: input창 에러일 때 추가 * fix: 필요없는 코드 삭제 (#222) * refactor: 약간의 로직분리 .. (#222) * code review: user email prop 추가, 모달 컴포넌트 하나의 객체로 export 하도록 수정 * style: 스타일 누락된 것 추가 * refactor: userEmail props 추가 누락된 부분 수정 * [ Style ] 온보딩 뷰 구현 (#228) * refactor: home/ not found 페이지에 쓰이는 사이드바 컴포넌트 리팩토링 (#224) - 세팅모달, 기존 SidebarHome에서 쓰이는 컴포넌트 구조 배럴파일 방식으로 변경 - HomePage, NotFoundePage 두 곳에서 쓰이므로 shared로 위치 이동 - Sidebar에서 쓰이는 svg는 Sidebar 폴더 아래로 이동 * refactor: 홈페이지 구조 absolute를 사용해서 변경 (#224) * feat: 온보딩 페이지 라우팅 (#224) * refactor: 설정 모달 위치 변경 (#224) * refactor: 사이드바 위치 변경 (#224) * refactor: 레이아웃 생성 및 적용 (#224) * refactor: 폰트 적용 방식 자동완성 될 수 있도록 변경 (#224) * feat: 사용하는 아이콘 다운로드 및 export 설정 (#224) * feat: StartStep, FieldStep 구현 (#224) * feat: ServiceStep 구현 및 export (#224) * chore: 상수 분리 및 컴포넌트 디자인 일부 수정 (#224) * feat: useFunnel 훅 생성 및 온보딩 페이지에 적용 (#224) * feat: Step 컴포넌트 컨벤션에 맞게 네이밍 변경 (#224) * feat: 온보딩 페이지 퍼블리싱 (#224) * feat: input 상태 ServiceStep 컴포넌트에 연결 (#224) * style: StepField 컴포넌트 화면 너비에 따라 패딩값 일부 수정 (#224) * feat: ServiceStep 버튼에 disabled 속성 추가 및 엔터 눌렀을 때 입력되게 끔 로직 추가 (#224) * feat: svg 아이콘 폴더 shared로 변경, assets 아래 페이지별 폴더를 만들어 관리 (#224) * feat: 바뀐 svg들의 위치에 맞게 import 경로 변경 및 BoxAllowedService 버튼에 홈으로 이동하는 로직 추가 (#224) * style: 반응형 고려하여 홈 페이지 컴포넌트 일부 스타일 수정 (#224) - 미완성입니다요 * refactor: truncate 활용해서 스타일 재적용 (#224) * refactor: StepField.tsx p태그에서 h2태그로 변경 (#224) * chore: 브라우저 리스트 업데이트하면서 일부 패키지 버전업 (#224) * refactor: StepService.tsx 인풋 상태 input -> inputURL로 수정 (#224) * [Refactor] home 페이지 폴더구조 변경 (#231) * feat: home 페이지 폴더구조 변경 (#230) * feat: 공통적으로 쓰이는 BoxTodo, ButtonTogoToggle -> shared로 이동 (#230) * feat: shared로 이동한 컴포넌트 경로 변경 (#230) * refactor: ModalContents 폴더 구조 변경 (#230) * feat: import 문 구분을 위해 폴더 구조 배럴파일 구조로 변경 (#230) * feat: commit 누락된 부분 저장 (#230) * feat: 홈페이지 import 문 수정 (#230) * refactor: atoms 컴포넌트 용도에 맞게 폴더 이동 및 네이밍 컨벤션에 맞게 변경 (#230) * refactor: 바뀐 컴포넌트 경로에 맞게 import문 수정 (#230) * refactor: 홈에서만 쓰이는 hook 폴더 이동 및 훅을 사용하는 컴포넌트의 import문 변경 (#230) * refactor: ButtonSVG 사용 불필요하여 삭제 (#230) * refactor: 상아 피드백 반영해서 UI 분류 페이지 제거 모든 컴포넌트 depth 1로 고정 (#230) * refactor: hook import 경로 수정 (#230) * refactor: 로그인 페이지 구조 변경 및 불필요한 templates 레거시 코드 삭제 (#230) (#232) * [Refactor] 타이머 페이지 폴더 구조 변경 (#233) * refactor: timer 컴포넌트/훅 컨벤션에 맞게 정리 및 불필요 컴포넌트/훅 삭제 (#230) * refactor: 불필요한 templates 컴포넌트 삭제 (#230) * Merge branch 'develop' of github.com:morib-in/Morib-Client into refactor/#230/timer-folder-structure * [Refactor] 온보딩 페이지 폴더구조 변경 (#234) * refactor: 온보딩 페이지 컴포넌트 폴더 삭제 및 컴포넌트명으로 폴더 생성하여 적용 (#230) * refactor: 상아 피드백 반영해서 컴포넌트 depth 1로 고정 (#230) * [style] 컬러팔레트 네이밍 추가 및 색상 공통 컴포넌트 생성 (#220) * [refactor] 색상 네이밍에 맞춰 컬러 팔레트 수정 (#220) * [feat] url 중복 유효성 검사 로직 추가 (#220) * [refactor] 드롭다운 클릭 시 이벤트 전파 방지 (#220) * [style] 카테고리 내 허용서비스 컴포넌트 분리 (#220) * [code review] 코드리뷰 반영 (#220) * [style] moribSet -> allowedService 네이밍 변경 (#220) * [chore] 리베이스 충돌 해결 (#220) * [style] sideBar 삭제 후 Layout 적용 (#220) * [code review] 코드리뷰 반영 (#220) * [style] 사이드바 홈, 허용서비스뷰 추가 (#220) * [style] 드롭다운 이벤트 전파 방지 적용 (#220) --------- Co-authored-by: 김건휘 <[email protected]> Co-authored-by: Hanseo Kim <[email protected]> Co-authored-by: suwonthugger <[email protected]>
🔥 Related Issues
✅ 작업 리스트
🔧 작업 내용
설정 뷰 내에서 사용되는 경고 모달 뷰를 구현했습니다!
경고 모달 내에서 사용되는 버튼은 경고 모달 안에서 공통으로 사용할 수 있게 primary, danger로 variant 설정한 ButtonAlert 컴포넌트 생성했습니다. 현재 경고 모달이 설정 뷰 안에만 있으므로 Setting 폴더 안의 components에 위치해뒀습니다.
ModalContentsAlert 컴포넌트를 생성하고, variant를'logout' | 'delete-account' | 'delete-account-complete'
로 설정하여 세 가지 경고 뷰를 하나의 컴포넌트 안에 담았습니다.코드리뷰 반영하여 Setting 폴더 안에 ModalContentsAlert 폴더 생성하고 Logout, DeleteAccount, Complete 파일 생성한 후에 ModalContentsAlert.tsx에서 하나의 객체로 export하도록 했습니다.
사용 예시
🤔 궁금한 점
경고 모달이 세 가지 뷰로 나뉘어서, 이걸 svg와 텍스트, input창을 children props으로 받아서 사용하도록 할까 고민하다가, 어차피 현재는 설정뷰에서만 경고모달이 존재하고, 재사용이 필요하면 나중에 경고모달이 늘어날때 분리해도 괜찮을 것 같아 일단은 variant를 설정하여 한 컴포넌트에다 두었습니다.아니면 이걸 각각의 컴포넌트로 나눠서 사용하는게 나을까요? input창에 에러 상태랑 스타일 추가하니까 코드가 좀 긴것 같기도.. 의견이 궁금합니다
참고로 뷰 퍼블리싱만 하고 모달 연결은 해두지 않았습니다..!
📸 스크린샷 / GIF / Link
logout
delete-account
2024-12-05.8.24.24.mov
delete-account-complete