Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Feature/#9] : 약속 생성 UI 구현 #12
[Feature/#9] : 약속 생성 UI 구현 #12
Changes from 1 commit
d35ca17
184cadd
8961814
06c6252
99cff84
6f5ffe6
505b32a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
P3 : 코드가 길어지니 가독성 측면에서 효율성이 떨어지는 것 같아요. 저라면 제목, 요일, 날짜, 선택했을 때 나타나는 하이라이터 각각을 컴포넌트로 한번 더 빼내서 사용했을 것 같아요. 리팩 측면에서 한번 생각해보시고 분리하면 좋을 것 같네요
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.
P1 : 해당 파일 코리 중 제안한 코드들이 있을겁니다. 세부적으로 spacer, padding 등을 변경하며 코드를 실행하였을 때 달라진 프리뷰를 첨부합니다.(원형 하이라이터는 코리를 위해 활성화한 것임)
왼쪽이 원본 코드, 오른쪽은 제가 단 제안 코드를 적용했을 때의 화면입니다. 조금씩 영역과 여백의 차이를 확인할 수 있을 겁니다
아래의 피그마 GUI 예시에서 확인할 수 있듯이 디자인쌤들이 지정한 여백, 터치 영역 등을 마우스로 잘 체킹하여 파악하고 그와 유사하게 구현하는 연습을 하면 더 좋을 것 같네요!
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.
P1 : 해당 파일의 캘린더 프로세스 코드는 UI 코드가 조금 더 정리된 후에 확인하는 것이 나을 것 같아요. 코리 반영 및 리팩 후 다시 체킹할 수 있게 언급 부탁드려요
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.
P3: 1월에서 그 전으로 가면 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.
P3 : 넣으신 이유가 있나요?
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.
P3 : padStart()는 무슨 역할을 하나요? UI 상에서는 해당 부분이 안쓰인 것 같은데 꼭 자리수를 채워야 코드가 동작하는 것인가요?
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.
연속된 날짜 선택할때 더 늦은 날짜를 먼저 누르고 더 빠른 날짜(8일 이상 차이나는)를 먼저 누를경우, 늦은 날짜를 끝마칠 날짜로 정하고 그 날짜를 기준으로 7일 차이나는 날짜를 시작날짜로 정하도록 계산할때 사용하고있는데 혹시 다른 방법 있을까요 ㅠㅠㅠㅠㅠ...... ... ...
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.
P2 : 코드상 재사용되는 컴포넌트로 확인되는데 따로 빼서 호출해서 사용하면 더 좋을 것 같은데요
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.
P2 : 또한 동일 코드에서 재사용되므로 빼서 호출해서 사용하면 좋을 것 같습니다. 인자로 padding 값 추가하구요
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.
P1: extract string plz