-
Notifications
You must be signed in to change notification settings - Fork 0
[QA] 날짜 피커, 날짜 형식 수정 #66
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
- NearDatePicker의 선택된 날짜, 오늘 날짜, 확인/닫기 버튼 등에 NearTheme의 `BLUE01_5AA2E9` 색상을 적용하여 디자인을 통일했습니다.
- 마지막으로 연락한 날짜 형식을 'yyyy-MM-dd'에서 'yyyy.MM.dd'로 변경하여 표시합니다.
- 사용자가 시스템 폰트 크기를 변경하더라도 앱의 UI가 깨지지 않도록 `fontScale`을 1f로 고정합니다.
- 연락 주기를 표시할 때, '매주/2주마다'는 요일을, '매달/6개월마다'는 날짜를 함께 보여주도록 수정했습니다. - `DateExtension`에 오늘 날짜(`getTodayDayOfMonth`)를 가져오는 함수를 추가했습니다. - `buildContactFrequencyText` 컴포저블 함수를 추가하여 연락 주기 텍스트 생성 로직을 분리했습니다.
Summary of ChangesHello @stopstone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 앱의 사용자 인터페이스 일관성과 사용자 경험을 개선하기 위한 여러 UI 개선 사항을 포함합니다. 날짜 피커의 색상을 앱의 테마에 맞게 조정하고, 날짜 표시 형식을 통일하며, 폰트 스케일 문제를 해결하여 다양한 기기에서 UI 레이아웃이 깨지지 않도록 합니다. 또한, 연락 주기 표시 로직을 개선하여 가독성을 높였습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
이번 PR은 QA 피드백을 반영하여 UI를 개선하고 날짜 형식을 수정하는 변경사항을 포함하고 있습니다. 전반적으로 코드 가독성을 높이고 UI 일관성을 맞추려는 노력이 돋보입니다. 특히, 복잡한 문자열 생성을 별도의 함수로 분리한 점과 시스템 폰트 크기 설정에 앱 UI가 영향을 받지 않도록 수정한 점은 좋은 개선입니다.
다만, 몇 가지 개선할 점이 보입니다. 연락 주기 표시 로직에서 설정된 값이 아닌 현재 날짜를 사용하고 있어 사용자에게 혼동을 줄 수 있는 버그가 있습니다. 또한, 리컴포지션이 자주 발생하는 UI 컴포넌트에서 문자열을 직접 조작하는 부분은 성능에 영향을 줄 수 있으므로 데이터 처리 로직을 뷰모델로 옮기는 것을 권장합니다. 자세한 내용은 각 파일에 남긴 코멘트를 참고해주세요.
| val suffix = | ||
| when (contactFrequency.reminderInterval) { | ||
| ReminderInterval.EVERY_WEEK, | ||
| ReminderInterval.EVERY_TWO_WEEK -> DateExtension.getTodayDayOfWeekInKorean() | ||
| ReminderInterval.EVERY_MONTH, | ||
| ReminderInterval.EVERY_SIX_MONTH -> | ||
| stringResource( | ||
| R.string.friend_profile_editor_contact_period_day_of_month, | ||
| DateExtension.getTodayDayOfMonth(), | ||
| ) | ||
| else -> null | ||
| } |
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.
주간/격주 반복 설정 시, 설정된 요일(contactFrequency.dayOfWeek) 대신 현재 요일을 표시하고 있어 의도와 다르게 동작할 수 있습니다. 예를 들어, '매주 수요일'로 설정했더라도 목요일에 보면 '매주 목요일'로 표시됩니다. contactFrequency.dayOfWeek를 사용하여 설정된 요일을 표시하도록 수정해야 합니다.
또한, 월간/반년 주기 설정 시 현재 날짜의 '일'을 가져와 사용하고 있는데, 이 또한 주간 주기 문제와 마찬가지로 조회하는 날짜에 따라 표시가 달라지는 문제가 있습니다. 이 설정값은 ContactFrequency 모델에 저장되고 사용되어야 할 것으로 보입니다.
val suffix =
when (contactFrequency.reminderInterval) {
ReminderInterval.EVERY_WEEK,
ReminderInterval.EVERY_TWO_WEEK -> when (contactFrequency.dayOfWeek) {
DayOfWeek.MONDAY -> stringResource(R.string.day_of_week_monday)
DayOfWeek.TUESDAY -> stringResource(R.string.day_of_week_tuesday)
DayOfWeek.WEDNESDAY -> stringResource(R.string.day_of_week_wednesday)
DayOfWeek.THURSDAY -> stringResource(R.string.day_of_week_thursday)
DayOfWeek.FRIDAY -> stringResource(R.string.day_of_week_friday)
DayOfWeek.SATURDAY -> stringResource(R.string.day_of_week_saturday)
DayOfWeek.SUNDAY -> stringResource(R.string.day_of_week_sunday)
}
ReminderInterval.EVERY_MONTH,
ReminderInterval.EVERY_SIX_MONTH ->
stringResource(
R.string.friend_profile_editor_contact_period_day_of_month,
DateExtension.getTodayDayOfMonth(), // TODO: 이 값은 상태로부터 읽어와야 합니다.
)
else -> null
}| Row(verticalAlignment = Alignment.CenterVertically) { | ||
| Text( | ||
| friendSummary.lastContactedAt ?: "", | ||
| friendSummary.lastContactedAt?.replace("-", ".") ?: "", |
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.
- `ContactItem` 컴포저블에 있던 날짜 형식(`yyyy-MM-dd` -> `yyyy.MM.dd`) 변환 로직을 `FriendSummaryMapper`로 이동시켜 데이터 계층에서 처리하도록 변경했습니다.
rhkrwngud445
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.
고생하셨습니다!
문서화 및 사소한 문법 관련해서 코멘트 남겼습니다!
확인 부탁드려요😀
| val currentDensity = LocalDensity.current | ||
| val themeDensity = | ||
| Density( | ||
| density = currentDensity.density, | ||
| fontScale = 1f, | ||
| ) |
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.
이 코드는 시스템 폰트 사이즈를 고정시키는 것이 맞을까요?
만약 그렇다면 접근성과 트레이드 오프긴 해서 고민이 되네요🤔
현재로서는 좋아보입니다! TODO로 접근성 관련 수정을 기록해주시면 향후 대응에 좋아보여요!
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.
기념일 수정 부분(FriendProfileEditorScreen)에서
연락 주기 -[패딩]- 텍스트필드 의 유동적인 패딩을 통일하는 방법이 떠오르지 않아 이렇게 구성하였습니다.
접근성 부분에서 아쉬운 부분이 많이 있을 것 같네요 😭
다른 방법으로는 텍스트필드를 고정값으로 해야 될 것 같아 이 부분은 다른 분들과 논의해보면 좋을 것 같습니다!
말씀주신대로 TODO 작성 해두겠습니다!
| return intervalText + | ||
| " " + | ||
| stringResource( | ||
| R.string.friend_profile_editor_contact_period_format, | ||
| suffix, | ||
| ) |
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.
이것도 사소한 거지만 아래와 같은 문법은 어떠실까요?
return "$intervalText ${stringResource(
R.string.friend_profile_editor_contact_period_format,
suffix,
)}"
``
반영은 안하셔도 됩니다!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.kt
캘린더의 텍스트버튼, DatePicker 색상을 키 컬러에 맞게 수정하였습니다.NearDatePicker.kt
yyyy-mm-dd 형태의 날짜 형식을 yyyy.mm.dd 로 변경하였습니다. ContactItem.kt
참고 사항
프로필 수정 화면에서 텍스트와 텍스트필드 사이 간격이 맞지 않는 현상이 있어
Theme.kt에서 폰트 스케일을 1f로 고정하였습니다.