-
Notifications
You must be signed in to change notification settings - Fork 4
[민혁, 세은] 모달 구현 과제 제출 #6
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
base: main
Are you sure you want to change the base?
Conversation
이슈 공통 템플릿 생성
Feat/seeun
Feature/minhyuk
Feat: Modal Type 로직 구현
Feat/seeun
Feat/minhyuk
Feat: Survey Modal 기능 구현 완료
nickname 유무에 따라 배경색 변경
Fix: form data type 수정(convert any to form) , Survey modal props 수정
Feat: Card 컴포넌트 생성
Fix: svg 선언식 에러 해결
kwakseongjae
left a comment
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="mr-[1.6rem] h-[23rem] w-[35.6rem] rounded-[0.8rem] border-[0.05rem] border-[#D4D4D4] bg-white"> | ||
| <div className="flex h-[5rem] w-[35.6rem] items-center justify-between border-b-[0.05rem] border-[#D4D4D4] px-[2rem]"> | ||
| <p className=" text-[1.6rem] font-bold text-[#525252]">{title}</p> | ||
| <span className="text-[1.2rem] font-normal text-[#A3A3A3]">작성일: {formattedDate}</span> | ||
| </div> | ||
| <div className="flex h-[16.4rem] w-[35.6rem] flex-col py-[1.6rem]"> | ||
| <p className="px-[2.4rem] text-[1.4rem] text-[#737373]">{content}</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.
tailwind의 장점을 한번 살려보는게 어떨까요 ?
tailwind의 mobile first css 에 대해 한번 공부해보세요 !
https://tailwindcss.com/docs/responsive-design
|
|
||
| const colorList = ['bg-[#F04D1D]', 'bg-[#FF7364]', 'bg-[#A75EFF]', 'bg-[#1EBDFE]', 'bg-[#0DCF85]']; | ||
|
|
||
| const Card = ({ title, content, tagList }: { title: string; content: string; tagList: string[] }) => { |
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.
type은 따로 선언해주십시당 :)
저는 3개 이상이면 interface나 type 선언 해줍니당 !
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.
동감..!
| className={ | ||
| 'h-[4rem] w-[4rem] rounded-full py-[1rem] text-center text-[1.4rem] font-bold text-white' + | ||
| (nickName.length > 0 ? ' bg-[#F0AF32]' : ' bg-white') | ||
| } |
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.
좋은 시도입니당 👍👍
피드백을 드리자면
LoginModal과 SurveyModal을 컴포넌트로 분리한건 좋았으나 파일로 관리해서 호출하지 못한게 아쉽습니당 !
리팩토링 단계에선 별도의 파일로 분리해서 관리해주세요 ! 그 때 모달 파일과, 로그인 모달, 설문 모달의 세 파일을 어떤식의 파일 구조로 가져갈지, 계층은 어떻게 설정할지에 대해 고민하고 파일 구조에 변화를 주면 더 좋을거같습니다.
Layout나 UI라는 폴더를 가져가는 경우도 있으니 종합적으로 고려해보세요 :)
| className="mx-auto mt-[3.2rem] h-[3.2rem] w-[41.6rem] rounded-md bg-[#F5F5F5]" | ||
| placeholder="닉네임을 입력하세요" | ||
| onChange={(e) => setNickName(e.target.value)} | ||
| ></input> |
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.
self closing tags 기억해둡시다 !!
https://jake-seo-dev.tistory.com/461
| setFormData(data); | ||
| console.log('Received form data:', data); | ||
| }; | ||
| // |
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 | ||
| onClick={() => showModal(modalType)} | ||
| className="mt-[9.7rem] h-[6.4rem] w-[32rem] rounded-[3.2rem] bg-[#6ED1F9] text-[2.4rem] font-bold text-white" | ||
| > | ||
| 로그인 | ||
| </button> | ||
| </div> | ||
| {modalOpen && ( | ||
| <Modal | ||
| modalType={modalType} | ||
| setModalOpen={setModalOpen} | ||
| formData={formData} | ||
| receiveFormData={receiveFormData} | ||
| /> | ||
| )} |
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.
리팩토링할때 이 부분을 한번 작업해봐주세요 !
계속 읽다보면 뭔가 이상해보일거예요 :)
| import ArrowSVG from '@assets/arrow.svg?react'; | ||
|
|
||
| type form = { title: string; content: string; tagList: string[] }; | ||
| const formDataList: form[] = []; |
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.
흠 context api라도 사용해주세요,,
| {formDataList.map((data) => ( | ||
| <Card title={data.title} content={data.content} tagList={data.tagList} /> | ||
| ))} |
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.
id 안쓰시나용 ?
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.
path 관리가 일반적이진 않아 보여요. 한번 정도 더 고민해보시겠어요 ?
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.
나아중에 최종 배포할 때는 빼셔야 하는거 아시죵??
| @@ -0,0 +1,45 @@ | |||
| import React from 'react'; | |||
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 colorList = ['bg-[#F04D1D]', 'bg-[#FF7364]', 'bg-[#A75EFF]', 'bg-[#1EBDFE]', 'bg-[#0DCF85]']; | ||
|
|
||
| const Card = ({ title, content, tagList }: { title: string; content: string; tagList: string[] }) => { |
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.
동감..!
| "noFallthroughCasesInSwitch": true, | ||
|
|
||
| "baseUrl": ".", | ||
| "paths": { |
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.
GOOOOD
| import Header from '@components/Header'; | ||
|
|
||
| const HomePage = () => { | ||
| const [modalOpen, setModalOpen] = useState<boolean>(false); |
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.
이번 작업에선 UI를 전문디자이너가 작업해놓은게 아니라서 스타일이 통일되는게 없어서 그랬을수도 있지만,,! 다음엔 config를 한번 사용해보세요,,,!
✨배포 링크
언어 및 라이브러리
🛠️ Motivation
민혁 👨🔧
tailwind를 사용해 Modal UI 구현세은 👩🎨
Vite를 사용한 개발 환경 설정react-router-dom v6를 사용해 페이지 라우팅tailwind를 사용해 Home, Myspace UI 구현🔑 Key Changes
민혁 👨🔧
컴포넌트 추상화
세은 👩🎨
Vite를 사용한 개발 환경 설정 및 절대경로 설정react-router-dom v6를 사용해 페이지 라우팅🐛발견된 버그
로그인 하지 않고 /myspace 이동 시
UI 버그들
🚀 Future Implementation Plans
🙏🏼 To Reviewers
세은 : 하드코딩된 부분은 없는지, 재사용성을 고려해 코드가 작성되었는지 봐주세요.