-
Notifications
You must be signed in to change notification settings - Fork 0
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
Y_FE_Toy1_Team9 (9굴 WIKI) #3
base: main
Are you sure you want to change the base?
Conversation
사이드바 설정 기능 및 전체적인 갤러리 페이지 레이아웃
Env: firebase.ts내 auth추가
Refactor: 홈 화면 레이아웃 코드 변경
로그인 여부에 따른 페이지 라우팅 기능, 로그인 유저 ID값 전달 기능 구현
Feat: 캐러셀 추가
갤러리 페이지 이미지 상세보기 및 기능 추가 및 카테고리바 디자인 수정
dev -> main (머지X)
Feat/#119 서지수 리드미 수정
fix github logo
Docs/#122 위키페이지 리드미 작성
Docs: Update README.md
최종 완료 q(≧▽≦q)
Docs: Update readme.md
[feat] 회원가입 성공 시 새로운 유저 database 생성
design: SideBar html, css 작업
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.
우선 정말 짧은 시간동안 고생 많으셨습니다!!!
전반적으로 고쳤으면 하는 내용들은
- 변수명 개선 (축약형, 의미가 부족한 변수명)
- 매직넘버 개선
- 컴포넌트 복잡도 개선 (리렌더링 최적화)
- 불필요한 코드 삭제 (주석 포함)
등이 꼽혔던 것 같습니다.
좋았던 점은, 로그인이 안 된 유저의 진입을 막는 부분을 시도하신 부분, 캐로샐 직접 구현, ESC 를 통한 모달창 닫기 등 ux 고려, 드래그앤드롭으로 이미지 업로드 하는 ux, 키보드 화살표를 통한 제어 등 ux에 신경을 많이 쓰신 부분들이 정말 좋았습니다.
개선할 점들이 많이 꼽혔지만, 이번 프로젝트를 경험으로 많은 것을 배우셨으면 좋겠다는 마음과 함께 리뷰를 마치겠습니다.
고생하셨습니다!
스타일 관련된 리뷰는 최소화 했습니다.
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.
issue와 PR을 통해 협업하신 점 너무 멋집니다.
PR을 생성할 때 closed #이슈번호 라고 명시해두면, 해당 PR이 닫힐 때 issue도 같이 닫히는데 다음에 한 번 적용해보시면 좋을 것 같아요
그리고 issue template에 yml 파일로도 템플릿화가 가능한데, 다음에 찾아보시고 적용해보시면 좋을 것 같습니다~!
const [uid, setUid] = useState(""); | ||
const [email, setEmail] = 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.
전체에서 사용되는 값을 state로 관리하셨네요, 필요에 따라서 Router에서 전달할 수 있도록 하게하신 것 같은데 이 방식 보다는 context API 를 사용했으면 어땠을까 싶습니다. 그럼 불필요하게 router에게 전달할 필요도 없어지고 props drilling 도 피할 수 있고 최상단에 선언된 provider를 통해 모든 컴포넌트가 접근할 수 있는 state가 되어서 관리가 편합니다.
context api 관련해서 공부가 꺼려지신다면 정말정말 잘 설명된 레퍼런스가 있으니 남겨놓고 가겠습니다.
https://velog.io/@velopert/react-context-tutorial
}, []); | ||
|
||
return ( | ||
<> |
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.
이 리뷰를 마지막으로 불필요한 wrapper 가 없는지 확인해봅시다!
padding: string; | ||
normal?: string; | ||
disabled?: boolean; | ||
onClick?: () => void; |
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.
typescript에 VoidFunction 이라는 유틸 타입이 있는데 자주 애용하는 편입니다. 똑같이 동작하는데 자동완성이 되어서 편해요!
@@ -0,0 +1,31 @@ | |||
import * as style from "./buttonStyle"; |
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.
파일 하나하나만 볼 때는 상관 없지만 폴더구조로 봤을 때 조금 난해하지 않나요?
대소문자라도 통일이 되었으면 그래도 바로 다음에 style이 붙어서 정렬이 되었을텐데요!
이런 이슈를 막고자 컴포넌트별로 폴더를 따로 관리하는 것이 좋습니다.
예를 들어 지금은 test code도, storybook도 없지만 다음과 같은 경우가 생긴다면 무척이나 어지러울 겁니다.
Button.tsx
Button.styles.ts
Button.stories.tsx
Button.test.ts
Button.cy.ts
Foo.tsx
Foo.styles.ts
....
폴더별로 따로 관리한다면 이런 것을 막을 수 있게 됩니다.
@@ -0,0 +1,37 @@ | |||
import { HasChildMap, Wiki } from "@/components/wiki/types/WikiCommonType"; |
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 을 import 할 때는 import type {} from "" 으로 명시하는 게 좋습니다.
무슨 차이인지를 한 번 찾아보고 답변으로 남겨보시는 것도 좋을 것 같아 이유는 스스로의 공부로 남겨보겠습니다 !
padding={"0.38rem 0.69rem"} | ||
margin={"0 0.31rem 0 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.
padding={"0.38rem 0.69rem"} | |
margin={"0 0.31rem 0 0"} | |
padding="0.38rem 0.69rem" | |
margin="0 0.31rem 0 0" |
string 만 전달할 때는 중괄호로 감싸지 않으셔도 됩니다!
e: React.ChangeEvent<HTMLSelectElement>, | ||
) => { | ||
const newParentID = e.target.value; | ||
const isNewWiki = () => form.wikiID === ""; |
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,29 @@ | |||
import "@toast-ui/editor/dist/toastui-editor.css"; | |||
import { Editor } from "@toast-ui/react-editor"; |
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.
많은 에디터가 있었을텐데, 그중 toast-ui 를 사용한 이유도 README에 정리가 되었었으면 좋았겠네요, 라이브러리를 활용할 때는 그 라이브러리를 선택한 이유가 분명할 수록 좋습니다.
return ( | ||
<Styled.WikiTop> | ||
<Styled.WikiTitle>{title}</Styled.WikiTitle> | ||
<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.
필요한 wrapper인가요?
직원들을 위한 위키 사이트 - 9굴 WIKI
💁 프로젝트 정보
🌐 배포 주소
🚖 개발 팀 소개
💻 개발 스택
🌙 환경
🌙 개발
🌙 소통
💡 User Flow
🗂 디렉토리 구조
🤝 협업 방식
커밋 컨벤션, 코딩 컨벤션, 깃허브 규칙 등의 내용은 아래의 노션 페이지를 참고해주세요!
🔗 노션 페이지
🛠️ 주요기능
⭐ 출퇴근 기록
⭐ 메인페이지 와 WIKI, Gallery 페이지 연동
⭐ 회원가입, 로그인 기능 및 인증
⭐ WIKI 페이지 게시물 CRUD
⭐ Gallery 페이지 게시물 CRUD
Gallery 페이지에서 게시물을 CRUD 할 수 있습니다.
🔍 팀원별 세부 구현 사항
이용훈: 📷 갤러리 페이지
1. 카테고리 추가
앨범 상위 카테고리 추가
2. 앨범 추가
이미지들을 저장할 앨범 카테고리(폴더) 생성
3. 이미지 추가
앨범 폴더 내부에 이미지 추가
4. 이미지 삭제
앨범 폴더 내부에 이미지 삭제
5. 이미지 상세보기
이미지 방향 전환 및 크기 조절
이승연: 🔑 로그인 페이지
1. 접근 제한 라우팅
로그인 여부에 따른 제한 접근 라우팅
2. 회원 가입
회원 가입
회원가입 유효성 검사
3. 로그인
로그인
로그인 유효성 검사
4. 로그인 정보 전달
로그인한 유저 정보 전달
양재준: 📂 WIKI 페이지
1. 위키 페이지 로딩 & 초기화
위키 데이터 로딩 및 초기 설정
2. 하위 위키 항목 표시
하위 위키 항목 표시
3. 위키 작성
새로운 위키 작성
4. 위키 편집
선택한 위키 항목의 내용 수정
5. 위키 삭제
위키 항목 삭제
서지수: 🌐 헤더 및 메인 페이지
1. 헤더 통근 다이얼로그
출근 기능
퇴근 기능
2. 로그아웃 기능
3. 최근 작성 위키 조회 기능
김소정: 🌐 메인 페이지
1. 메인 캐러셀
홈 화면 공지사항을 보여주는 캐러셀 구현
2. 홈 화면 갤러리 미리보기 구현