리뷰용 pr입니다. #64
리뷰용 pr입니다. #64
Conversation
로그인 페이지 CSS
navbar css
�Social Login
| "@testing-library/jest-dom": "^6.4.8", | ||
| "@testing-library/react": "^13.0.0", | ||
| "@testing-library/user-event": "^13.2.1", | ||
| "@types/jest": "^27.0.1", | ||
| "@types/jest": "^29.5.12", | ||
| "@types/node": "^16.7.13", | ||
| "@types/react": "^18.0.0", | ||
| "@types/react-dom": "^18.0.0", | ||
| "@types/react-modal": "^3.16.3", | ||
| "@types/styled-components": "^5.1.34", |
There was a problem hiding this comment.
dependency와 devDependency에 대한 학습이 필요해보여요!
testing-library같은 의존성이나, @types같은 친구들이 dependencies에 들어간 이유가 있을까요!?
한번 알아보는거 추천드립니당
| "scripts": { | ||
| "start": "react-scripts start", | ||
| "build": "react-scripts build", | ||
| "test": "react-scripts test", | ||
| "eject": "react-scripts eject" | ||
| "eject": "react-scripts eject", |
There was a problem hiding this comment.
프로젝트를 CRA로 만드신것같네용
그런데 CRA는 작년에 deprecated됐습니다🥲 (facebook/create-react-app#13072)
일단 플젝 진행중이긴 하니 마이그레이션이 필수는 아니지만, 나중에 시간 여유 생기면 webpack같은 번들러로 프로젝트 세팅 직접 해보시는거 추천드릴게요!
번들러 관련 개념은 아주아주아주아주 중요한지라 나중에라도 학습해보시는거 강하게 추천드립니다.
그게 아니라 cra처럼 가볍게 프로젝트 세팅이 필요한 상황이라면, create vite를 사용하는걸 추천드려요!
| Route, | ||
| Navigate, | ||
| } from 'react-router-dom'; | ||
| import MainPage from './pages/MainPage'; |
There was a problem hiding this comment.
절대경로 사용은 어떤가요!?
세팅하다보면 자연스럽게 tsconfig관련 세팅 알아보게 되고,
번들링시에 오류날거라 tsc-alias가 왜필요한지도 알아보게될거라 나중에 시간나면 시도해보시는거 추천드립니당
|
|
||
| // 로그인 상태를 체크하는 함수 | ||
| const useAuth = () => { | ||
| const [isLoggedIn, setIsLoggedIn] = useState<boolean>(false); |
There was a problem hiding this comment.
initial값이 false라 boolean이 자동으로 타입추론 될거에요!
은 굳이 안써도될듯합니당
| const [loading, setLoading] = useState<boolean>(true); // 로딩 상태 추가 | ||
|
|
||
| useEffect(() => { | ||
| const refreshToken = localStorage.getItem('refreshToken'); |
There was a problem hiding this comment.
백엔드에도 같이 질문하고싶은 내용인데, refreshToken을 로컬스토리지에서 관리하는 이유가 있나요!?
그리고 로직이 뭔가 이상해서 로컬스토리지에 refreshToken이라는 이름으로 직접 집어넣어봤는데
이렇게 뚫려버리는 이슈가 있네요! 확인 한번 해보셔야할것같아용
| cy="0" | ||
| r="1" | ||
| gradientUnits="userSpaceOnUse" | ||
| gradientTransform="translate(278.5 543.5) rotate(90) scale(278.5)" | ||
| > | ||
| <stop stopColor="#00D1FF" /> | ||
| <stop offset="1" stopColor="#00D1FF" stopOpacity="0" /> | ||
| </radialGradient> | ||
| </defs> | ||
| </svg> | ||
| </div> | ||
|
|
||
| <div | ||
| className="loginBox" | ||
| data-aos="fade-right" | ||
| data-aos-duration="2000" | ||
| data-aos-easing="ease-out-cubic" |
There was a problem hiding this comment.
오우 이렇게 보니 svg 분리가 꼭 필요해보이긴 하네요...
svg들 별도로 묶어놓고 타입 못찾는다하면
declare module '*.svg' {
import type React from 'react';
const SVG: React.FC<React.SVGProps<SVGSVGElement>>;
export default SVG;
}
추가해주고
읽는거자체가 터지면 @svgr같은거 세팅해서 할 수 있을거에요
| list: StatusPersonalBlock['blockListResDto']; | ||
| pageInfo?: StatusPersonalBlock['pageInfoResDto']; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| component?: React.ComponentType<any>; |
There was a problem hiding this comment.
음... 이렇게 들어가야 하는 상황이면 뭔가 컴포넌트 설계가 잘못된것같은데, 나중에 차차 봐야 알것같네요 이 부분은 ㅜ
| done: { | ||
| ...prevColumns.done, | ||
| list: [], | ||
| pageInfo: { currentPage: 0, totalPages: 1, totalItems: 1 }, | ||
| }, | ||
| delete: { | ||
| ...prevColumns.delete, | ||
| list: prevColumns.delete.list, // 휴지통 리스트 유지 | ||
| pageInfo: prevColumns.delete.pageInfo, // 휴지통 페이지 정보 유지 | ||
| }, | ||
| })); | ||
|
|
||
| fetchData(0); | ||
| fetchDashboardData(); | ||
| }, [location.pathname, fetchData]); | ||
|
|
||
| // 페이지가 변경될 때 데이터를 다시 가져옴 | ||
| useEffect(() => { | ||
| if (page > 0) { | ||
| fetchData(page); | ||
| } | ||
| }, [page, fetchData]); | ||
|
|
||
| useEffect(() => { | ||
| fetchBlockData(0); // 페이지가 로드될 때 처음으로 데이터를 불러옵니다. | ||
| }, [dashboardId]); | ||
|
|
||
| // fetchTrigger 상태가 변경되면 데이터를 다시 불러옴 | ||
| useEffect(() => { | ||
| fetchBlockData(0); // 트리거가 변경되면 다시 데이터 호출 | ||
| }, [fetchTrigger]); | ||
| // 블록 순서 변경 디바운스 처리 | ||
| const debouncedData = useDebounce(columns, 10); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
메인페이지긴 하지만 컴포넌트가 아는 정보가 너무 많은것 같아요.
컴포넌트는 가장 추상화된 레이어로서, 최대한 UI로직만 담을 수 있도록 설계되어야한다고 생각해요!
이렇게 컴포넌트가 알아야하는 정보가 너무 많아진다거나, 비대해진다면 커스텀훅으로 분리해보는건 어떨까 싶네요!
|
|
||
| console.log('전달받은 카테고리', categories); | ||
|
|
||
| const toggleFunc = (event: React.MouseEvent) => { |
There was a problem hiding this comment.
React 말고 MouseEvent만 import해도 될거같아요.
event: MouseEvent<HTMLImageElement>이런식으로요!
There was a problem hiding this comment.
네이밍도 handleClickToggle과 같은 형태가 좋지 않을까 싶네용
| import { useAtom } from 'jotai'; | ||
|
|
||
| const TeamFileBoard = () => { | ||
| const [visibleValue, _] = useAtom(visibleAtom); |
|
@MyungJiwoo @sinamong0620 |
|
하나씩 다 읽어보면서 뜨끔했던 부분이 많았지만 그만큼 부족한 부분이 어딘지 알게 되었습니다. ㅎㅎ... 아직 댓글을 못 단 리뷰는 더 알아보려고 보류한 부분인데, 시간이 조금 걸리더라도 하나씩 다 공부해보고 다시 댓글 남겨보도록 하겠습니다! 코드 리뷰해주셔서 감사합니다! 🥹 |
|
@MyungJiwoo 어차피 좋은 코드라는것도 서비스를 견고하게, 더 잘 만들어나가기 위해 필요한거고, 당장 서비스가 동작하지 않는거면 아무짝에도 쓸모없는거라서요... 기존 프로젝트 팀원들이랑 정해뒀던 대로 기능 구현 일정은 맞추되, 남는 시간에 리팩토링해나가는게 좋을 것 같습니다. 그리고 읽어보시면 좋을 것 같은 아티클 하나 남겨드릴게요. 시간 남으면 한번 읽어보세요~ |
🪄 목적
💻 상세 작업 내용
📸 스크린샷
👼🏻 리뷰 요구사항 (선택)