-
Notifications
You must be signed in to change notification settings - Fork 1
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
[4주차 기본/심화/공유 과제] 회원가입 & 로그인 #5
base: main
Are you sure you want to change the base?
Conversation
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.
코드도 깔끔하게 짜시고,
로직분리도 잘하신 것 같아요 ~!
고생하셨습니다 👍👍👍
week4-assignment/src/main.tsx
Outdated
@@ -0,0 +1,10 @@ | |||
import { StrictMode } from 'react' | |||
import { createRoot } from 'react-dom/client' | |||
import './index.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.
App.css, index.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.
넵 바로 삭제 했습니다..!
margin: 3rem; | ||
color: ${({ theme }) => theme.colors.black}; | ||
`; | ||
|
||
export const InputWrapper = styled.div` | ||
margin-bottom: 15px; | ||
width: 100%; | ||
`; | ||
|
||
export const Input = styled.input` | ||
padding: 10px; |
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.
margin에는 rem, padding에는 px 등으로 단위를 섞어서 사용하시고 있는 것 같은데,
혹시 단위를 사용하신 기준이 있으실까요?
그게 아니라면 단위를 일관성이 있게 사용하시는것이 유지보수나 가독성 측면에서 더 좋을 것 같습니다!
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.
이유 없습니다.... 바봅니다. 수정하겠습니다.
week4-assignment/src/App.tsx
Outdated
import Login from './pages/LoginPage'; | ||
import Signup from './pages/SignupPage'; | ||
import MyPage from './pages/MypagePage'; |
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해오는 것은 어떨까요?
특히 이 프로젝트같이, LoginPage와 Login 컴포넌트가 둘다 있는 경우에는, 헷갈릴 우려가 있을 것 같습니다!
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> | ||
<Login onLogin={handleLogin} /> | ||
</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.
<div> | |
<Login onLogin={handleLogin} /> | |
</div> | |
<Login onLogin={handleLogin} /> |
요기도 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.
넵 수정했습니다
const [username, setUsername] = useState<string>(''); | ||
const [password, setPassword] = useState<string>(''); | ||
const [hobby, setHobby] = useState<string>(''); | ||
const [step, setStep] = useState<number>(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.
const [username, setUsername] = useState<string>(''); | |
const [password, setPassword] = useState<string>(''); | |
const [hobby, setHobby] = useState<string>(''); | |
const [step, setStep] = useState<number>(1); | |
const [username, setUsername] = useState(''); | |
const [password, setPassword] = useState(''); | |
const [hobby, setHobby] = useState(''); | |
const [step, setStep] = useState(1); |
이렇게 초기값을 지정해준 경우, type을 명시적으로 알려주지 않고, typescript가 자동으로 추론하게 하는 것이 좀 더 권장되는 방식이라고 합니다!
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.
오호.... 처음 알았습니다 !! 앞으로 이 점 생각해서 코드 작성하겠습니다! 감사합니다.
<> | ||
<Label htmlFor="hobby">취미</Label> | ||
<InputWrapper> | ||
<Input |
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.
몬가 정렬이 이상한 것 같아요..!
prettier 설정을 확인해보면 좋을 것 같습니다! 모르겠으면 절 호출해주세요~
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.
prettier 설정 했습니다. 감사합니다!
export const Title = styled.p` | ||
font-size: 2.5rem; | ||
text-align: center; | ||
flex: 1; | ||
margin-right: 2rem; | ||
`; | ||
|
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.
Title이면 p태그 보다는 h1 ~ h6등 헤딩 태그가 더 적적할것 같아요!
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 response: AxiosResponse<MyHobbyResponse> = await axios.get( | ||
'http://211.188.53.75:8080/user/my-hobby', |
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.
api 요청마다 baseUrl을 작성하는 등 과정을 줄이기 위해 axios instance를 만들어서 해보시는 것도 좋을 것 같아요!
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 MypageInfo from '../components/Mypage/MypageInfo'; | ||
|
||
const MypagePage = () => { | ||
const [selectedComponent, setSelectedComponent] = 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.
const [selectedComponent, setSelectedComponent] = useState<string>('취미'); | |
const [selectedComponent, setSelectedComponent] = useState<'취미' | '내 정보'>('취미'); |
이와 같은 선택 메뉴 처럼 특정 값만 오는 state의 경우에는
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.
오.. 👍👍 올 수 있는 값만 명시하면 더 안전한 코드가 되는군요..! 수정했습니다!
✨ 구현 기능 명세
💡 기본 과제
취미
,내 정보
메뉴 탭로그아웃
버튼취미
,내 정보
취미 페이지, 내 정보 페이지 출력 (1개의 페이지로 구현해도 되고, url 달라도 됨)🔥 심화 과제
공유과제
제목: 타입스크립트의 타입
링크 첨부 : https://wave-web.tistory.com/122
❗️ 내가 새로 알게 된 점
❓ 구현 과정에서의 어려웠던/고민했던 부분
🥲 소요 시간
20h
🖼️ 구현 결과물
회원가입(이름)
-.Chrome.2024-11-12.15-18-48.mp4
회원가입(비밀번호, 취미)
-.Chrome.2024-11-12.15-19-53.mp4
로그인, 로그아웃
-.Chrome.2024-11-12.15-21-27.mp4
마이페이지(취미)
-.Chrome.2024-11-12.15-22-14.mp4
마이페이지(내 정보)
-.Chrome.2024-11-12.15-22-24.mp4