-
Notifications
You must be signed in to change notification settings - Fork 26
[양재영] sprint8 #136
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
[양재영] sprint8 #136
The head ref may contain hidden characters: "React-\uC591\uC7AC\uC601-sprint8"
Conversation
[양재영] sprint8 진행을 위해 merge 위한 pr
|
스프리트 미션 하시느라 수고 많으셨어요. |
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.
오잉..? dist이 같이 올라왔네요 ?
혹시 .gitignore 파일이 누락된 것 같군요 !
일반적으로 dist/는 빌드했을 때 생성되는 실행 파일을 담고있는 디렉토리예요.
이는, 배포하는 환경마다 다를 수 있으며 원본 코드에서 생성되는 부가적인 폴더이기에 일반적으로 형상관리(git)에 포함하지 않습니다 😉
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.
음. 아무래도 .gitignore가 누락된게 맞는 것 같군요 !
node_modules는 의존된 패키지들을 담고있는 파일이며, 용량 자체도 크기에 형상관리에서 제외하는게 일반적입니다 !
요새는 .gitignore 생성기도 있더라구요 !
한 번 참고해보셔서 작성해보시는게 좋을 것 같아요 😉
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.
아.. 지금 보니까 /vite-project에서 설치해야 하는데 실수하신 것 같군요 !
.gitignore가 누락된 것이 아닌, vite-project 내에서 세팅해야 하는데 프로덕트 루트에서 초기 세팅을 하신 것으로 보여지는군요.
만약 이게 맞다면 vite-project 외 필요 없는 파일 및 디렉토리는 지우시는게 좋을 것 같네요 😊
Updated
다시 보니까, 랜딩 페이지는 리액트가 아닌 ts로 작성하신 것이며 의도하신 것으로 보이는군요 ! 😊
랜딩 페이지에 .ts파일도 함께 리뷰하였습니다 ! 😉
| <Route | ||
| path="/" | ||
| element={ | ||
| <MainLayout> | ||
| <MainPage /> | ||
| </MainLayout> | ||
| } | ||
| /> | ||
| <Route | ||
| path="/items" | ||
| element={ | ||
| <DefaultLayout> | ||
| <ItemsPage /> | ||
| </DefaultLayout> | ||
| } | ||
| /> | ||
| <Route | ||
| path="/additem" | ||
| element={ | ||
| <DefaultLayout> | ||
| <AddItemPage /> | ||
| </DefaultLayout> | ||
| } | ||
| /> | ||
| <Route | ||
| path="/items/:productId" | ||
| element={ | ||
| <DefaultLayout> | ||
| <ItemDetailPage /> | ||
| </DefaultLayout> | ||
| } | ||
| /> |
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.
굿굿 ! 레이아웃을 정의하고 사용하셨군요 !
추가로 Route를 활용해서 중첩된 레이아웃들을 묶을 수 있어요 😉
| <Route | |
| path="/" | |
| element={ | |
| <MainLayout> | |
| <MainPage /> | |
| </MainLayout> | |
| } | |
| /> | |
| <Route | |
| path="/items" | |
| element={ | |
| <DefaultLayout> | |
| <ItemsPage /> | |
| </DefaultLayout> | |
| } | |
| /> | |
| <Route | |
| path="/additem" | |
| element={ | |
| <DefaultLayout> | |
| <AddItemPage /> | |
| </DefaultLayout> | |
| } | |
| /> | |
| <Route | |
| path="/items/:productId" | |
| element={ | |
| <DefaultLayout> | |
| <ItemDetailPage /> | |
| </DefaultLayout> | |
| } | |
| /> | |
| <Route | |
| path="/" | |
| element={ | |
| <MainLayout> | |
| <MainPage /> | |
| </MainLayout> | |
| } | |
| /> | |
| <Route element={<DefaultLayout />}> | |
| <Route path="/items" element={<ItemsPage />} /> | |
| <Route path="/additem" element={<AddItemPage />} /> | |
| <Route path="/items/:productId" element={<ItemDetailPage />} /> | |
| </Route> |
이와 관련되어 스택오버 플로우 자료를 참고하였습니다 !
|
|
||
| // ----------------------------------------------------------------------- | ||
|
|
||
| function showError(inputElement: HTMLInputElement, message: 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.
굿굿 ! 기본적으로 정의된 타입(HTMLInputElement)을 찾아서 잘 사용하셨네요 👍
| import kakaoIcon from '../../assets/ic-login-kakao.png'; | ||
|
|
||
| export default function LoginPage () { | ||
| const [email, setEmail] = useState(''); | ||
| const [password, setPassword] = useState(''); | ||
| type ErrorState = { | ||
| email?: string; | ||
| password?: 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.
타입 코드는 바깥에 선언하시는게 가독성이 더욱 좋을 것으로 사료됩니다 !
| import kakaoIcon from '../../assets/ic-login-kakao.png'; | |
| export default function LoginPage () { | |
| const [email, setEmail] = useState(''); | |
| const [password, setPassword] = useState(''); | |
| type ErrorState = { | |
| email?: string; | |
| password?: string; | |
| } | |
| import kakaoIcon from '../../assets/ic-login-kakao.png'; | |
| type ErrorState = { | |
| email?: string; | |
| password?: string; | |
| } | |
| export default function LoginPage () { | |
| const [email, setEmail] = useState(''); | |
| const [password, setPassword] = useState(''); |
타입은 런타임 때 제외되므로 성능이나 로직이 달라질 일은 없기에 큰 문제는 없을 것으로 사료되나, 일반적으로 타입은 주 로직과 구분하여 사용하기도 합니다 !
| const [email, setEmail] = useState(''); | ||
| const [password, setPassword] = 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.
다음과 같이 폼 데이터로 사용될 상태는 객체로 묶어도 되겠군요 ! 😉
| const [email, setEmail] = useState(''); | |
| const [password, setPassword] = useState(''); | |
| const [formData, setFormData] = useState({ email, password }: { email: string, password: string }); |
| const validateEmail = (value: string) => { | ||
| if(!value) return '이메일을 입력해주세요.'; | ||
| const emailRegex = /^[A-Za-z0-9]([-_.]?[A-Za-z0-9])*@[A-Za-z0-9]([-_.]?[A-Za-z0-9])*\.[A-Za-z]{2,3}$/; | ||
| if(!emailRegex.test(value)) return '잘못된 이메일입니다.'; | ||
| return ''; | ||
| } | ||
|
|
||
| const validatePassword = (value: string) => { | ||
| if(!value) return '비밀번호를 입력해주세요.'; | ||
| if(value.length < 8) return '비밀번호를 8자 이상 입력해주세요.'; | ||
| 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.
크으...... 유효성 검사 함수를 순수함수로 구성하셨군요 ! 🥺
훌륭합니다 ! 잘 만들어 두신 덕에 해당 함수는 컴포넌트 내부에 선언 될 필요가 없겠어요.
컴포넌트 내부에 선언되면 리렌더링 시 불필요한 재선언이 될 수 있고, 컴포넌트를 구성하는 자원(props, state)을 사용하지 않으므로 컴포넌트 외부에 선언해두고 사용하셔도 될 것으로 보여요 👍
순수함수?: 입력값이 같으면 언제나 같은 결과를 반환하고 외부 상태에 영향을 주거나 의존하지도 않아요.
| </form> | ||
| <div className="login-simple"> | ||
| <h1>간편 로그인하기</h1> | ||
| <div> | ||
| <a href="https://www.google.com" | ||
| ><img src={googleIcon} alt="구글" | ||
| /></a> | ||
| <a href="https://www.kakaocorp.com/page" | ||
| ><img src={kakaoIcon} alt="카카오" | ||
| /></a> | ||
| </div> | ||
| </div> | ||
| </main> | ||
| <footer className="login-footer"> | ||
| <span>판다마켓이 처음이신가요?</span> | ||
| <a href="/signup">회원가입</a> | ||
| </footer> |
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.
크으.. 기본을 놓치지 않는 정신..
리액트로, 타입스크립트로, 그리고 NextJS로 넘어가도 HTML의 기본 근육을 놓치지 않고 있군요 !
적절한 곳에 의미 있는 태그들로 구성하셨어요. 훌륭합니다 ! 👍👍👍
중급 프로젝트를 진행하시면서도 지금과 같이 시맨틱은 항상 유의하시는게 좋습니다 !
| const validateEmail = (value: string) => { | ||
| if(!value) return '이메일을 입력해주세요.'; | ||
| const emailRegex = /^[A-Za-z0-9]([-_.]?[A-Za-z0-9])*@[A-Za-z0-9]([-_.]?[A-Za-z0-9])*\.[A-Za-z]{2,3}$/; | ||
| if(!emailRegex.test(value)) return '잘못된 이메일입니다.'; | ||
| return ''; | ||
| } | ||
|
|
||
| const validateNickname = (value: string) => { | ||
| if(!value) return '닉네임을 입력해주세요.'; | ||
| return ''; | ||
| } | ||
|
|
||
| const validatePassword = (value: string) => { | ||
| if(!value) return '비밀번호를 입력해주세요.'; | ||
| if(value.length < 8) return '비밀번호를 8자 이상 입력해주세요.'; | ||
| return ''; | ||
| } | ||
|
|
||
| const validatePasswordCheck = (value: string) => { | ||
| if(!value) return '비밀번호를 입력해주세요.'; | ||
| if(value !== password) return '비밀번호가 일치하지 않습니다.'; | ||
| 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.
해당 코드도 전과 같은 피드백이지만 !
더 나아가서 유지보수성을 위해 따로 유효성 검사 파일을 만들어서 재사용하시는 것도 고려해볼 수 있겠군요 !
|
미션 진행하시느라 수고하셨습니다 재영님 ! 👍👍👍 모두 러프하게 읽어보면서 중복된 리뷰를 하지 않으려 재영님의 과거 피드백도 보면서 리뷰를 진행하였습니다 ! 이번 미션 정말 수고 많으셨습니다 재영님 ! |
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게