-
Notifications
You must be signed in to change notification settings - Fork 35
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
[도비] VM 1주차 수 PR #22
[도비] VM 1주차 수 PR #22
Conversation
- GlobalStyled으로 전역 스타일 적용 - styled-reset으로 기본 스타일 초기화 - ThemeProvider로 공통 스타일 관리
- "/"로 접근시 VendingMachine 컴포넌트를 렌더링한다. - "wallet"으로 접근시 Wallet 컴포넌트를 렌더링한다. Related to: #3
- NotFound 컴포넌트를 렌더링한다. - useNavigate의 navigate를 이용해서 해당하는 라우터로 이동하도록 구현했다. Related to: #3
- useLocation을 사용해서 현재 페이지와 id가 일치하는 태그에 배경색을 주도록 구현 Related to: #3
- 기존의 경우 뒤로/앞으로가기 버튼과 NotFound의 자판기로 가기 버튼을 클릭했을때 네비게이션이 업데이트 되지 않았음 - useEffect를 사용해서 수정함 Related to: #3
라우터 이동 구현 및 관련 컴포넌트 생성
- 폴더 이름 통일을 위해 첫글자를 소문자로 바꿔주었다.
Related to: #4
- 정적인 UI만 구현함 - 재고가 없을 경우 가격 대신 '품절'을 보여준다. Related to: #4
- 정적인 UI만 구현함 - Order 컴포넌트의 세부적인 구현 X Related to: #4
- 품절된 상품은 빨간불이 들어오도록 변경함 - 커서 관련 스타일 추가 Related to: #4
- styled-components의 네이밍 변경 - overflow 적용 Related to: #4
Related to: #4
VendingMachine 컴포넌트 UI 구현
- margin 및 padding 수정 - border-radius 수정
- 자판기 모양에 가깝게 스타일을 추가 및 수정함 Related to: #4
- input에 금액 입력시 자동으로 , 를 찍도록 구현 - 만 단위 숫자까지 입력하도록 설정 - 투입 버튼의 navigate 기능 삭제 Related to: #4
- 가격표시된 태그를 span에서 button 태그로 변경 - props를 한번만 넘겨받기 위해 PriceWrapper 내부의 styled-component를 일반 태그로 변경 Related to: #4
- 정적인 UI만 구현함 - mock data 사용 Related to: #5
기존 UI 수정 및 지갑 UI 구현
- Outlet과 중첩 라우터를 사용해서 공통 레이아웃 컴포넌트를 생성함 Close : #3
- id 추가, key 네임 변경
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.
안녕하세요 2주 동안 리뷰를 맞게된 카일입니다 :)
UI 코드임에도 깔끔하게 구현하신게 보이네요.
코드간의 줄바꿈 같은 것도 일정하게 해주시고 하셔서 코드파악하는데 너무 편했습니다.
앞으로의 2주가 기대가 되네요 👍
리뷰하면서 제가 질문할 때도 있을텐데 생각을 여쭤보는거니 편하게 답변주시면 감사하겠습니다!
{ | ||
"compilerOptions": { | ||
"baseUrl": "src" | ||
}, | ||
"includes": ["src"] | ||
} |
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 index element={<VendingMachine />} /> | ||
<Route path="wallet" element={<Wallet />} /> | ||
</Route> | ||
<Route path="*" element={<NotFound />} /> |
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-router-dom v6는 사용해보지 않았는데 이렇게 간단하게 처리할 수 있나보네요👍
import { Outlet } from 'react-router-dom'; | ||
import Navigation from './Navigation'; | ||
|
||
export default function Layout() { |
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.
Layout.tsx는 폴더에 안 넣으신 이유가 따로 있으실까요?? 다른 컴포넌트들은 폴더에 넣으셨는데 Layout만 나와있어서 궁금하네요!
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.
Layout이 폴더안에 있는 컴포넌트들을 감싸고 있어서 components 폴더의 index 처럼 폴더없이 따로 빼놓았는데 지금 생각해보니 너무 주관적인것 같아서 Layout.jsx도 다른 컴포넌트들처럼 폴더 안에 넣도록 하겠습니다!
${props => | ||
props.focus === props.id && |
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.
theme을 distructuring해서 사용하듯 focus와 id도 적용해볼 수 있을 것 같네요!
import Product from './Product'; | ||
|
||
export default function ProductContainer() { | ||
const list = productsList.map(product => <Product key={product.id} info={product}></Product>); |
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.
list 보다 구체적인 명칭을 사용하는건 어떨까요? productsList처럼 구체적으로 작성하면 좋을 것 같아요
|
||
const theme = { colors, fontStyles }; | ||
|
||
export default 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.
벌써 도비만의 테마를 갖고 계신 것 같네요! 이렇게 하나씩 도비만의 디자인시스템을 만들어 나가면 너무 좋을 것 같아요!
@@ -0,0 +1,2 @@ | |||
const setLocalString = price => Number(price).toLocaleString(); |
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.
중복되는 코드를 유틸함수로 분리해주신 것도 잘하셨네요!
- Install nodemon - Setting json server - Create patch api for updating multiple data
* PR 리뷰 반영 및 Product 스타일 개선 (#23) * Refactor: StrictMode 활성화 및 reducer 수정 - 전위연산자를 -1로 수정 - 일부는 스프레드 연산자로 state를 복사하도록 수정 Related to: #22 * Refactor: OrderLog 컴포넌트 리턴문 개선 - if문을 리턴문 안으로 이동함 Related to: #22 * Style: Children이 없는 컴포넌트 수정 * Design: Product 컴포넌트 스타일 개선 - isAvailable이 false이면 하얀 동그라미가 되도록 수정 - prop에 대한 조건문 수정 * 반환 기능 추가 및 리렌더링 개선 (#24) * Fix: 반환 버튼과 관련된 컴포넌트를 파일로 분리 * Feat: 반환 버튼 클릭시 투입한 돈 반환하기 구현 Related to: #13 * Style: 중복된 case문 병합 * Refactor: input에 돈 투입하는 함수를 컴포넌트와 분리 * Feat: 로그 메세지가 길어지면 스크롤이 자동으로 내려가는 기능 추가 - logMessage 함수 분리 * Feat: 전역 타이머값을 위한 TimerContext 생성 - 타이머의 남은 시간을 나타내는 UI 추가 Related to: #13 * Refactor: MoneyContext 개선 및 일부 컴포넌트에 memo 적용 - dispatch와 관련된 함수를 hook으로 분리 - dispatch 함수 내부로 logContext의 dispatch 함수를 호출하도록 수정 * Refactor: LogContext 개선 - MoneyProvider 내부의 dispatch를 사용한 함수를 hook으로 분리 * Style: OrderContainer 관련 컴포넌트에 고정된 width값 추가 * Refactor: ProductContext 개선 및 memo 적용 * Fix: 스크롤 개선 - scrollIntoView를 scrollTop으로 개선
안녕하세요 kyle 도비입니다! 2주동안 잘 부탁드립니다!
아직 요구사항과 관련된 기능은 아직 시도하지 않았고 UI와 router 관련 기능만 구현했습니다.
기능은 react를 학습하면서 천천히 구현해보려고 합니다...🐢
진행 상황
2022-05-11.5.39.23.mov
관련 마일스톤
계획