-
Notifications
You must be signed in to change notification settings - Fork 3
주식리스트 테이블 레이아웃 구현 #98
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
Conversation
KimKyuHoi
left a comment
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.
👍 👍 👍
KimKyuHoi
left a comment
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.
그리고 Git Action 내에서 ui>src>components 내에서 type관련이랑 render내에서 쓰면안되는 약간 그런 규칙관련해서 warning이 올라오긴 하는데 한번 파악 해주시면 감사하겠습니다.
| @@ -0,0 +1,8 @@ | |||
| export type Stock = { | |||
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.
아 파일명을 stock.type.d.ts만 바꿔주시면 감사하겠습니당
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.
@KimKyuHoi 제가 찾아봤을때에는 전역 타입이 아니고 model내부에서의 타입은 .types.ts.를 많이 사용한다해서 파일명으로 사용하긴 했습니다!
혹시 이 부분에 대해 type.d.ts가 더 좋은 네이밍 일까요?
render에서 state를 사용한부분이 문제가 있는것같은데 확인해보고 다른 이슈에서 해결하겠습니다! |
|
Approve 해놓아서 머지해놓으시면 될것같습니다 |
Pull request
Related issue
Resolve #90 @handje
Motivation and context
Solution
전달사항
파일구조
fsd 패턴에 맞도록 ui / model 을 분리하였습니다. @KimKyuHoi 검색해보며 최대한 폴더를 분리하였지만, 다른 방법이 있으시다면 말씀해주세요!
How has this been tested
Types of changes
Checklist