-
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_Team2 Company Space #4
base: main
Are you sure you want to change the base?
Conversation
Tw 33 feature/sidebar/connect
디자인 초안이 나옴에 따른 컴포넌트 관계 설정 및 컴포넌트 레이아웃 재조정 (이슈 #1)
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.
우선 정말 짧은 시간동안 고생 많으셨습니다!!!
전반적으로 고쳤으면 하는 내용들은
- 변수명 개선
- 컴포넌트 복잡도 개선
- 반복되어 활용되는 코드들 모듈화
- 불필요한 코드 삭제 (주석 포함)
등이 꼽혔던 것 같습니다.
좋았던 점은 캐로샐 애니메이션 너무 멋져버리고,, 바로가기에서 Todo로 바로 추가하는 건 누구 아이디어인지 기획이 진짜 미쳤습니다. Footer도 깔끔하게 정리하신 것도 좋았고 코드 퀄리티는 개선할 점이 많았지만 앱 자체의 완성도는 높았다고 생각합니다.
고생 많으셨습니다!!
스타일관련된 코드 리뷰는 최소화했습니다.
|
||
import { firestore } from '../../shared/api/firebase'; | ||
import { Link, useParams } from 'react-router-dom'; | ||
import { theme } from '../../styles/Theme'; |
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.
cra로 작업하셨으면, craco 라는 패키지를 활용해서 pathAlias 를 적용해보시는 걸 해보시면 좋을 것 같습니다.
복잡한 상대경로를 간단하게 import 할 수 있게 도와줍니다.
@@ -0,0 +1,148 @@ | |||
import React, { useState, useEffect } 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.
React 를 import 하지마시구, 필요한 모듈만 import 하는 걸 적용해보세요
예를 들어, React.ChangeEvent -> ChangeEvent 이렇게 사용할 수 있도록요,
어떤 차이가 있는지 공부해보시구 답변 남겨보시는 것도 좋을 것 같습니다!
min-width: 256px; | ||
height: 100vh; | ||
background-color: ${theme.blueBg3}; | ||
color: ${theme.gray300}; | ||
padding: 24px 20px 0px; | ||
position: fixed; |
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.
저는 스타일 코드를 적을 때, 비슷한 놈들끼리 묶어서 적고 개행을 넣어주는 걸 좋아합니다.
min-width: 256px; | |
height: 100vh; | |
background-color: ${theme.blueBg3}; | |
color: ${theme.gray300}; | |
padding: 24px 20px 0px; | |
position: fixed; | |
position: fixed; | |
min-width: 256px; | |
height: 100vh; | |
padding: 24px 20px 0px; | |
background-color: ${theme.blueBg3}; | |
color: ${theme.gray300}; |
무슨 차이가 있냐면, 일단 스타일 코드만 읽어봐도 머릿속에 ui가 대충 그려지는 효과가 있습니다.
그리고 비슷한 걸 담당하는 애들끼리 모여있으니까 위에서부터 편하게 읽을 수 있다는 점도 좋습니다.
저는 개인적으로 적을 때
.foo {
position: fixed;
top: 0;
right: 0;
bottom: 0;
left: 0;
display: flex;
flex-direction: column;
justify-contents: center;
align-items: center;
width: 100px;
height: 100px;
padding: 100px;
margin: 100px;
background: red;
border: 1px solid red;
border-radius: 100px;
font-family: 'Noto';
font-size: 100px;
line-height: 100px;
font-weight: 400;
color: red;
// etc
}
이런 순서대로 적습니다.
한 번 적용해보세요! 개행을 넣으면 코드가 길어지는 단점이 있는데, 그런 걸 방지하려고 저는 styles.ts 파일에 styled-component를 모아두고 import 해서 씁니다.
padding: 8px; | ||
display: flex; | ||
align-items: center; | ||
cursor: pointer; |
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.
클릭 가능한 요소는 a, button, input 과 같이 focusable 한 요소들을 쓰는 게 좋은데요, 왜 그럴까요?? 🤔
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.
메뉴의 클릭 범위를 display: block으로 생각해서, div 태그에 cursor: pointer을 작성했었습니다.
button 태그는 예, 아니요, 확인, 저장 등 서버 데이터 교환? 과 같은 형식에 사용을 해야할 것 같다고 생각했습니다.
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.
focus가 불가능한 요소는 tabindex가 자동으로 할당되지 않아서인데요, 키보드를 통해서 클릭이 되지 않습니다.
ux 측면에서 더 좋은 이유였습니다!
`; | ||
|
||
const ItemContainer = styled.div<ItemContainerProps>` | ||
background-color: ${(props) => (props.$isUrl ? theme.blueBg1 : '')}; |
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.
background-color: ${(props) => (props.$isUrl ? theme.blueBg1 : '')}; | |
${({$isUrl, theme}) => $isUrl && `background-color: ${theme.blueBg1}` } |
이렇게 해야, 불필요한 css가 적용이 되지 않겠죠?!
지금은
background-color: ''
이렇게 되어서 적용이 안 되는 css가 불필요하게 CSS Tree에 읽히게 됩니다.
const [docs] = useState<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.
이 리뷰를 마지막으로 불필요하게 useState를 사용한 곳은 없는지 확인해보세요
|
||
// 글 수정 버튼 함수 | ||
function handleClickEditBtn(): void { | ||
if (authState.state === 'loaded' && authState.isAuthentication) { |
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.
반복되어서 사용되는 변수는 한 번만 선언해서 활용하는 것도 좋은 방법입니다.
{postState ? ( | ||
<> | ||
{toggleAddBtn ? ( |
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.
이중 삼항연산자는 정말정말 가독성을 낮추게 하는 지름길입니다.
차라리 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.
알겠습니다.
|
||
const FindPwPage: React.FC = () => { | ||
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.
불필요한 Fragment는 삭제합시다!
font-size: ${theme.textStyles.subtitle4.fontSize}; | ||
line-height: ${theme.textStyles.subtitle4.lineHeight}; |
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.
StyledComponent에는 themeProvider가 있는데요, 이를 활용하면 매번 theme을 가지고 다니지 않아도 typed 하게 theme을 활용할 수 있습니다.
@minsoo-web 정성 가득한 코드리뷰 감사드립니다..! 세세한 코드 리뷰 + 전반적인 완성도 평가까지 감동입니다 ㅠㅠ |
🚀 Company Space 🚀
🪐 프로젝트 기간
2023.09.08 ~ 2023.09.22
🪐 관련 링크
[배포 링크]
🚀 Company Space 🚀
[레포지토리 링크]
🚀 Company Space Repo 🚀
[피그마 링크]
🚀 Company Space Figma 🚀
[대박징조 노션]
🚀 Notion 🚀
🪐 기술 스택 / 라이브러리
✔️ 언어
✔️ 백엔드(DB)
✔️ 라이브러리
✔️ 협업
✔️ 디자인
✔️ 코드 포맷 및 오류 도구
🪐 팀원 정보
🏅2조 - 대박징조
팀장 : 신현진
팀원 : 박준규
팀원 : 박용희
팀원 : 정범환
팀원 : 장문용
🪐 구현 사항 체크
🌟 필수 구현 사항
README.md
파일 작성🌟 선택 구현 사항
🪐 페이지 및 역할, 기능 소개
💫 화면 구성
[메인 페이지]
[위키 페이지]
[갤러리 페이지]
[로그인 페이지]
[회원가입 페이지]
[비밀번호 찾기 페이지]
[출퇴근 모달]
[404 페이지]
💫 각 조원 별 역할, 기능 소개
🌟 메인 페이지, 헤더, 푸터 - 정범환
💡 메인 페이지에서 기간 별로 할일 목록을 관리하고, 위키나 갤러리를 요약하여 보거나 바로 이동할 수 있습니다.
로그인할 시 사용자의 이름을 표출하고 기간 별로 할일 목록과 완료된 일 목록을 볼 수 있습니다.
할일 목록과 완료된 일 목록을 생성하고, 관련 문서를 지정하고 추가, 수정 및 삭제할 수 있습니다.
문서 목록에서 특정 문서에 관련된 할 일을 생성할 수 있습니다.
푸터에 참여자들의 contact 수단들을 정리하였습니다.
🌟 위키 페이지, 사이드바 - 박용희
💡문서 데이터 CRUD기능을 파이어베이스와 연동하여 구현했습니다.
사이드 메뉴
Firebase에 저장된 데이터를 불러오고, 사용자가 클릭에 따라 UI가 바뀌어집니다.
글 수정
로그인 상태
비로그인 상태
URL로 이동 시
건의사항
Firebase에 저장된 데이터를 불러오고, 데이터를 보여줍니다.
로그인의 여부에 따라서 글 추가 삭제 가능을 사용할 수 있습니다.
글 추가 및 삭제를 할 경우, Fireabse에 저장된 데이터를 Create, Delete 합니다.
모달창
로그인 상태 - 추가
로그인 상태 - 삭제
비로그인 상태 - 삭제
404 페이지 이동
wiki/:id 속성을 이용하여 라우터를 사용했기 때문에 id값을 비교하여 해당 id가 없을 경우, 404페이지로 이동합니다.
🌟 갤러리 페이지, 404페이지 - 신현진
💡 사진 데이터 추가/삭제 기능(갤러리 페이지)과 404페이지를 구현하였습니다. 추가적으로 eslint/prettier와 firebase의 설정을 했고 전체적으로 manage하는 역할을 맡았습니다.
데이터 불러오기 전 로딩 애니메이션 구현
로그인 되지 않은 상태일 때
로그인 된 상태일 때
각 카드에 hover를 하면 x 아이콘이 생겨 해당하는 데이터를 삭제할 수 있습니다.
추가하기 버튼이 생겨 이를 누르면 modal 창이 나와 데이터를 추가할 수 있습니다.
이름 입력 후 엔터키를 누르거나 추가하기 버튼을 누르면 추가됩니다.
모달 바깥 부분을 클릭한 경우 모달이 닫힙니다.
사진과 이름 둘 중 하나라도 입력하지 않으면 alert창을 통해 알려주어 둘 다 필수로 입력하도록 해두었습니다.
이미지가 존재하지 않을 때
🌟 출퇴근 모달 - 장문용
💡Local storage를 이용해서 Modal로 근무 시간을 측정하고 출퇴근 시간을 보여주는 기능을 구현하였습니다.
로그인을 하면 heaer에 버튼이 생성됩니다.
모달에는 현재 날짜와 시간 표시 / 출근 시간 / 예상 퇴근 시간이 표시됩니다.
Timer가 가는 중에는 빨간색으로 표시됩니다.
Timer 중단 기능 (일시 정지)
모달 창을 닫아도 header에 Timer 상태가 표시됩니다.
로그인한 사용자 이름이 적용되고 modal의 멘트가 변경됩니다.
alert
출퇴근, 업무 재개 한번 더 확인
local storage에 저장
Modal 열림 상태(isModalOpen)
출근 상태 (isCommuteButtonClicked)
TimerRunning 상태(isTimerRunning)
Timer 시간(seconds)
출근 시간(wordStateTime)
🌟 로그인 관련 기능 - 박준규
💡 Firebase의 Authentication 빌드를 활용하여 로그인 / 회원가입 / 비밀번호 찾기 기능을 구현하였으며, 페이지 전반의 기능들에 대해 로그인 유무를 판별 가능하도록 유저 로그인 정보를 전달했습니다.
회원가입
페이지유효성 검사 기능 (이메일 중복 및 유효성 여부, 비밀번호 확인 일치 여부)
가입 후 인증메일 전송
로그인
페이지유효성 검사 기능
이메일 인증 여부
를 통한로그인 접근을 제한
하는 기능을 추가비밀번호 찾기
페이지새 비밀번호 수정 메일 전송 기능
🪐 아쉬운 부분
리팩토링 기간이 있다면 추가로 수행 할 내용들 입니다❗
반응형 구현 X
dependencies, devDependencies 구분 X - 프로젝트 진행 중 npm 패키지 관련한 오류가 매우x500 많이 발생했어서 절망의 구렁텅이에 여러 번 빠진 팀원들이 더 이상 npm을 건드는 것을 원치 않았습니다. 시간이 난다면 수정해보겠습니다!
vite, emotion 사용하지 못함
렌더링 최적화X
로그인 관련