-
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
12조 과제 제출 (정태욱, 박철민, 방미선, 백지욱) #8
base: main
Are you sure you want to change the base?
Conversation
const isToday = today === dayjs(date).format(DATE_FORMAT) | ||
let textColor = isToday ? 'text-point' : 'text-main' | ||
if (disable) textColor += ' opacity-20' | ||
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.
일정이 겹치는 문제가 있어서 아래 코드 처럼 일정의 id값을 가지는 이차원 배열로 한 주의 일정을 관리하도록 변경했습니다.
const visited = [
[1,2], // 일
[1,2], // 월
[null,2], // 화
[null, null], // 수
[3,null], // 목
[3,null], // 금
[null,null] // 토
]
리뷰를 다시는 분이 있을까봐 링크로 변경된 사항을 남깁니다. 죄송합니당 ㅠㅠ
https://github.com/MINI-FASTCAMPUS5/scheduler-front/blob/main/src/components/calendar/Daily.tsx#L162
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.
안녕하세요! 9조 김성은입니다.
프로젝트 전제척으로 완성도가 높고 디자인 구현도 개성있게 잘 하신것 같습니다. 멋지세요..
미니 프로젝트 진행하시느라 고생 많으셨습니다! 12조의 코드를 보고 많이 배워갑니다.
아는만큼 보인다는데 12조 코드를 보니 눈앞이 캄캄하네요 흑흑
관리자 페이지 담당이라 해당 부분 코드리뷰 진행했습니다. 워낙 잘짜셔서 첨언드릴 말이 없지만 공부하는 마음으로 보고 리뷰 남깁니당! ㅎㅎ
철민님, 테일윈드 처음 사용하신다고 하셨는데 깔끔하게 잘사용하신것 같아요! 멋지십니다! 👍
코드가 구조도 간결하고 함수들을 읽기 쉽게 잘작성하신것 같습니다.
중복되어 사용된 코드들 외에는 특별히 리팩토링 요소가 필요한 부분은 보이지 않는것 같습니다!
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.
approveSchedule함수와 cancelSchedule 함수는 이름이나 구조에서 그 기능이 명확하게 보여서 좋은것 같습니다!
두 함수의 try catch 구문이 거의 동일한데 중복을 최소화하지 않고 사용하신 이유가 그러한 이유 때문인것 같은데,
공통함수로 분리하고 두 함수에서 재사용하는 방식으로 리팩토링 하는 방법도 있을것 같습니다!
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.
전체적으로 간결하게 구성을 잘짜신것 같습니다! 특별히 리팩토링 할 부분이 보이지 않지만 굳이 찾아보자면
'isSuccess && 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.
잘 정돈된 프로젝트 인거 같습니다. 다른 분들도 꼼꼼히 보시고 코멘트를 남겨 주셔서 다른 말씀을 드릴 건 없습니다.
저는 개인적인 전체적인 프로젝트 구조가 잘 되었구나 하는 생각을 합니다. 디렉토리를 구성하고 그 안에 컴포넌트나 페이지 등등 컨벤션을 잘 지켜서 구성되어 있는거 같습니다. 전체적으로 잘 세분화해서 리액트에 특성에 맞게 잘 나누어서 구성하신거 같아 제가 해두엇던 프로젝트를 한번 돌아보게 되었습니다. 잘 보았습니다. 감사합니다.
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.
안녕하세요~ 9조의 이용수입니다.
발표회에서 보았을 때 너무 잘 만드셔서 넋을 잃은 프로젝트였습니다 ㅠㅠ
코드 리뷰 하면서 많은 걸 배우고 느낄 수 있었고
컴포넌트 확실하게 분리하신 부분, 많이 배워갑니다 !
고생 많으셨어요 !
|
||
return ( | ||
<section className='overflow-hidden h-full p-4'> | ||
<div className='h-full'> |
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.
tailwind를 쓰다보니 그 부분을 고려하지 못한 것 같습니다 감사합니다!
return ( | ||
<div className='h-[100vh] w-[100vw]'> | ||
<div className=' absolute flex text-[50px] z-40 top-5 left-8'> | ||
<img src='/YeonganIdolLogoWhite.svg' className='w-[600px]' /> |
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.
alt 속성도 있으면 좋을 것 같아요 !
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 handleEdit = (message: string) => { | ||
toast(message, { | ||
position: 'top-center' |
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.
너무 좋은 방법인 것 같습니다.
toast를 발표 12시간 전에 부랴부랴 추가해서 생각하지 못한 것 같습니당 ㅋㅋㅋ..
개선해 두겠습니다!
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.
안녕하세요! 9조 안태욱입니다.
프로젝트 전제척으로 완성도가 높아서 발표하실때 넋 놓고 보았습니다...
미니 프로젝트 진행하시느라 고생 많으셨고, 코드 보면서 많이 배워갑니다.
로그인, 회원가입 페이지 담당이라 해당 부분 코드리뷰 진행했습니다.
지욱님께서 워낙 잘짜셔서 따로 말씀드릴게 없지만 같이 공부하는 마음으로 리뷰 남깁니다..!
<div className='flex w-full flex-col'> | ||
<div className='justify-center pl-[35px] pr-[35px] mb-[70px]'> | ||
<svg viewBox='0 0 300 81'> | ||
<image href='/YeonganIdolLogoOrigin.svg' width='300' height='81' /> |
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.
image 태그 대신 img 태그로 로딩하는 것이 더 일반적으로 알고 있는데, 따로 image로 작성하신 이유가 있을까요? 또 alt태그 사용으로 사용자 접근성에 좀더 용이하게 관리 할 수 있을것 같습니다..!!
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.
안녕하세요, 12조 담당 김민수 멘토입니다.
리뷰가 많이 늦었네요 ㅜ 죄송합니다..
우선, 너무 고생 많으셨습니다!
react-query를 활용해서 서버상태 관리를 유연하게 하려고 하신 점이 특히 인상 깊었습니다.
전반적으로 하나의 파일에 선언부와 실행부가 섞여있어서 가독성이 떨어지는 점과,
반복되어 사용되는 것을 hook으로 개선해보기
any 타입 없애기
등이 개선 사항으로 기억에 남는 것 같습니다.
리뷰가 많은데, 확인 한 번 부탁드립니다~!
"editor.codeActionsOnSave": { | ||
"source.fixAll.eslint": true // 저장할 때마다 eslint가 자동으로 코드를 고쳐줍니다. | ||
}, |
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.
"editor.codeActionsOnSave": { | |
"source.fixAll.eslint": true // 저장할 때마다 eslint가 자동으로 코드를 고쳐줍니다. | |
}, | |
"editor.codeActionsOnSave": { | |
"source.fixAll.eslint": true, | |
"source.organizeImports": true | |
}, |
이렇게 세팅해두면, 저장할 때 import도 같이 정렬해줍니다.
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.
와 감사합니다. 이런 정보는 찾기가 너무 힘든 것 같아요.
회사에서 연차, 당직을 사용하기 위해 남아있는 횟수를 확인하고, 신청하고 사용합니다. | ||
<br> 우리는 덕질을 위해, 남아있는 티켓의 횟수를 확인하고, 신청하고 사용합니다. | ||
<br> 연간아이돌은 덕질을 위해 만들어진 공연 신청 서비스입니다. | ||
<br> 일상에 지친 당신에게 매달 한번!! 원하는 공연을 신청해서 티켓을 소진하세요!! |
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.
회사에서 연차, 당직을 사용하기 위해 남아있는 횟수를 확인하고, 신청하고 사용합니다. | |
<br> 우리는 덕질을 위해, 남아있는 티켓의 횟수를 확인하고, 신청하고 사용합니다. | |
<br> 연간아이돌은 덕질을 위해 만들어진 공연 신청 서비스입니다. | |
<br> 일상에 지친 당신에게 매달 한번!! 원하는 공연을 신청해서 티켓을 소진하세요!! | |
회사에서 연차, 당직을 사용하기 위해 남아있는 횟수를 확인하고, 신청하고 사용합니다. | |
우리는 덕질을 위해, 남아있는 티켓의 횟수를 확인하고, 신청하고 사용합니다. | |
연간아이돌은 덕질을 위해 만들어진 공연 신청 서비스입니다. | |
일상에 지친 당신에게 매달 한번!! 원하는 공연을 신청해서 티켓을 소진하세요!! |
맨 뒤에 스페이스 두 개 넣어주면 줄바꿈됩니다!
import dayjs from 'dayjs' | ||
import { DATE_ROUTE_FORMAT } from '@/constants' | ||
import { UserContext } from '@/context/UserProvider' | ||
// import useUser from '@/hooks/user' |
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.
svg에서 색상만 다른 경우는 currentColor 라는 걸 활용하는 것도 방법인데, 같이 공부해보는 걸 추천드립니다!
{type === 'add' && user == 'admin' && ( | ||
<AddForm onCancle={onCancle} onSubmit={handleSubmit} date={date} /> | ||
)} | ||
{type === 'edit' && user == 'admin' && ( | ||
<EditForm onCancle={onCancle} onEdit={handleEdit} schedule={schedule!} /> | ||
)} |
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.
user === "admin" 을 맨 위로 올려서 한 번만 검증하고,
type 별로 SwitchCase를 적용하면 가독성이 더 좋아질 것 같습니다.
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='bg-white w-4/5 f-full max-w-[640px] max-h-[740px] min-h-[640px] rounded-2xl'> | ||
<div style={{ position: 'relative' }}> | ||
{children} | ||
<button className='absolute top-3 right-3 p-2 font-bold' onClick={() => onClose()}> |
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 className='absolute top-3 right-3 p-2 font-bold' onClick={() => onClose()}> | |
<button className='absolute top-3 right-3 p-2 font-bold' onClick={onClose}> |
const { isFetching } = useSchedule() | ||
const { year, month } = useSchedule() |
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.
없습니다. 제대로 확인을 안 했습니다 ㅠ
import useSchedule from '@/hooks/schedule' | ||
import useResize from '@/hooks/resize' | ||
import useUser from '@/hooks/user' | ||
import useHover from '@/hooks/hover' |
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.
named export 를 사용하면 트리쉐이킹도 막아주고, 배럴 파일을 활용해서 한 번의 import 문으로 여러 함수들을 import 할 수 있어서 저는 named 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.
진짜 좋은 방법인 것 같습니다.
트리쉐이킹을 고려할 생각을 못했는데 조언 감사합니다.
베럴 파일 키워드를 찾아봤는데 프로젝트 사이즈가 커지면 정말 유용할 것 같아요. 꼭 찍먹 해보겠슴다.
if (dayjs(date).format('ddd') === '일' || dayjs(date).format('ddd') === '토') { | ||
bgStyle = 'bg-blue-100' | ||
} | ||
bgStyle |
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.
앗.. 지우겠습니다..
연간아이돌(YeonganIdol)
연차? 당직? 우리는 콘서트 보러 간다!!!
회사에서 연차, 당직을 사용하기 위해 남아있는 횟수를 확인하고, 신청하고 사용합니다.
우리는 덕질을 위해, 남아있는 티켓의 횟수를 확인하고, 신청하고 사용합니다.
연간아이돌은 덕질을 위해 만들어진 공연 신청 서비스입니다.
일상에 지친 당신에게 매달 한번!! 원하는 공연을 신청해서 티켓을 소진하세요!!
기획사는 연간아이돌을 통해 신청된 티켓의 승인, 거절을 결정할 수 있습니다. 홍보는 덤입니다.
기획사 권한 신청은 관리자에게 문의하세요.
미니 프로젝트 기간: 2023.07.24 ~ 2023.08.10
✔️ 배포 링크 바로가기
✔️ 프론트엔드 깃허브 레포지토리 바로가기
✔️ 백엔드 깃허브 레포지토리 바로가기
🤗 개발팀
정태욱(FE-리더)
박철민
방미선
백지욱
캘린더 부가기능 전체,
프로젝트 셋팅,
로딩 애니메이션,
깃허브 관리,
사용자 마이페이지,
매니저 행사 등록/수정
매니저 대시보드,
디자인-1,
css 전역 스타일,
와이어프레임,
로그인 배경화면,
캐릭터 디자인
회원정보 수정,
디자인-2,
와이어프레임,
유저 플로우
로그인 페이지
🛠️ 사용 기술 및 개발 환경
Development
Config
Environment
Cowork Tools
Deployment
💻 전체 화면 구성
사용자 페이지
매니저(기획사) 페이지
관리자(권한 관리) 페이지
🙏 감사합니다