[Feature] 캘린더 컴포넌트를 구현합니다. #19
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 캘린더 헤더(연 달 표시, 이전 달 다음 달 버튼) - 캘린더 요일 표시
librarywon
reviewed
Jul 25, 2024
|
|
||
| class Calendar( | ||
| initDate: LocalDate = LocalDate.now(), | ||
| private val dateTimeFormatter: DateTimeFormatter = | ||
| DateTimeFormatter.ofPattern(DEFAULT_DATE_TIME_FORMAT_PATTERN), | ||
| DateTimeFormatter.ofPattern(DEFAULT_DATE_TIME_FORMAT_PATTERN).withLocale(Locale.ENGLISH), |
There was a problem hiding this comment.
Locale.ENGLISH 를 적용하신 이유가 무엇인가요?
There was a problem hiding this comment.
명시해주지 않으면 의도와는 다르게 디폴트로 들어가는 인자가 1월 2024 이런식으로 디바이스의 언어설정 기준으로 들어가게 됩니다.
디자인상으로 의도된건 Jan 2024와 같은 영어약어의 표현이기 때문에 해당 Locale 설정을 추가하였습니다.
There was a problem hiding this comment.
오 디바이스 언어를 인식하는줄은 몰랐습니다! 새로운 사실을 배워가네요
Comment on lines
58
to
78
| Row(modifier = Modifier.padding(bottom = 8.dp)) { | ||
| DayOfWeek.entries.forEach { dayOfWeek -> | ||
| DayOfWeek( | ||
| name = dayOfWeek.abbreviation, | ||
| modifier = Modifier.weight(1f), | ||
| ) | ||
| } | ||
| } | ||
| LazyVerticalGrid( | ||
| columns = GridCells.Fixed(7), | ||
| verticalArrangement = Arrangement.spacedBy(DAY_DEFAULT_MARGIN.dp), | ||
| horizontalArrangement = Arrangement.spacedBy(DAY_DEFAULT_MARGIN.dp), | ||
| ) { | ||
| items((days.first().date.dayOfWeek.ordinal + 1) % 7) { | ||
| Box(modifier = Modifier.weight(1f)) | ||
| } | ||
|
|
||
| items(days) { | ||
| Day(studyDay = it, modifier = Modifier.weight(1f)) | ||
| } | ||
| } |
There was a problem hiding this comment.
월을 표시하는 로직과 일을 표시하는 로직을 분리하면 가독성이더 좋아지지 않을까 제안드려봅니다!
There was a problem hiding this comment.
반영완료 하였습니다. 가독성을 위한 분리를 해보고 추후 장단점을 피부로 같이 느껴보는 시간을 가졌음 좋겠습니다!
Comment on lines
+40
to
+44
| Then("윤년이기 때문에 1부터 29일까지 반환된다.") { | ||
| actual = calendar.getMonth() | ||
| actual.first().date shouldBe LocalDate.of(2024, 2, 1) | ||
| actual.last().date shouldBe LocalDate.of(2024, 2, 29) | ||
| } |
There was a problem hiding this comment.
윤년처리까지 치밀하게 테스트 해보셨군요..!
There was a problem hiding this comment.
워낙에 애초에 잘 짜여진 Java의 라이브러리다 보니 엣지케이스 위주로 테스트를 진행하였습니다!
|
추가적으로 월을 표기하는 곳에 디자인 변경사항이 발생했으니 그것도 반영해주시면 좋을 것 같습니다. |
librarywon
approved these changes
Jul 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
resolved #15
AS-IS
TO-BE
KEY-POINT
SCREENSHOT (Optional)