-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix] 날짜를 바뀌어도 이전에 선택했던 시간대로 보여지도록 수정 (iOS와 통일) #299
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
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.
Pull Request Overview
This PR ensures that when the user switches dates, the previously selected meal time tab (morning/lunch/dinner) is remembered and shown, aligning Android behavior with iOS.
- Persist selected time tab position in
SharedPreferences - Load saved position (or fallback to current time) in the view‐pager adapter
- Register page change listener to save new selections
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/src/main/java/com/eatssu/android/presentation/cafeteria/CafeteriaViewPagerAdapter.kt | Load saved tab index from MySharedPreferences and fall back to current time; removed API annotation. |
| app/src/main/java/com/eatssu/android/presentation/cafeteria/CafeteriaFragment.kt | Initialize view pager with saved position and register listener to save changes. |
| app/src/main/java/com/eatssu/android/data/MySharedPreferences.kt | Added savePreTimePosition and getPreTimePosition for persisting tab index. |
| val time = LocalTime.now() | ||
| when (time.hour) { |
Copilot
AI
May 28, 2025
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.
This method uses LocalTime.now() without any API level guard. Re-adding @RequiresApi(Build.VERSION_CODES.O) or checking the SDK version before calling Java Time APIs will prevent crashes on older devices.
| viewPager.adapter = viewpagerFragmentAdapter | ||
| viewPager.setCurrentItem(viewpagerFragmentAdapter.getDefaultFragmentPosition(), false) | ||
| viewPager.setCurrentItem( | ||
| viewpagerFragmentAdapter.getDefaultFragmentPosition(App.appContext), |
Copilot
AI
May 28, 2025
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.
[nitpick] Using a global App.appContext can hinder testability and may mask context-related issues. Consider passing requireContext() directly to getDefaultFragmentPosition instead of using a static application context.
| viewpagerFragmentAdapter.getDefaultFragmentPosition(App.appContext), | |
| viewpagerFragmentAdapter.getDefaultFragmentPosition(requireContext()), |
| viewPager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() { | ||
| override fun onPageSelected(position: Int) { | ||
| MySharedPreferences.savePreTimePosition(requireContext(), position) | ||
| } | ||
| }) |
Copilot
AI
May 28, 2025
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.
The registered OnPageChangeCallback is never unregistered, which may cause a memory leak when the fragment's view is destroyed. Unregister the callback in onDestroyView().
| viewPager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() { | |
| override fun onPageSelected(position: Int) { | |
| MySharedPreferences.savePreTimePosition(requireContext(), position) | |
| } | |
| }) | |
| val pageChangeCallback = object : ViewPager2.OnPageChangeCallback() { | |
| override fun onPageSelected(position: Int) { | |
| MySharedPreferences.savePreTimePosition(requireContext(), position) | |
| } | |
| } | |
| viewPager.registerOnPageChangeCallback(pageChangeCallback) | |
| this.pageChangeCallback = pageChangeCallback |
kangyuri1114
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.
굿굿
| return prefs.getBoolean("ALARM_ON", false) | ||
| } | ||
|
|
||
| fun savePreTimePosition(context: Context, position: Int) { |
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.
다른 함수들처럼 save보다는 set 어떨까요??
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,1,2로 저장하기 보다는 ENUM이나 String으로 저장하는게 좋지 않을까요??
|
@kangyuri1114 유리님!!!! 이거 반영안해도 바텀시트 #293 머지되면서 해당 코드 없어도 의도대로 동작하는것 같아요!!!! 현재 배포된 버전으로 실물 기기 테스트 한번 해주세요! 제가 테스트한게 맞다면 이거 머지 말고 close해도 될듯 |
Summary
Describe your changes
이전에 선택했던 식사 시간대를 저장해서, 날짜를 바뀌어도 그 시간대로 보여지도록 수정 (iOS와 통일)
예시) 현재 시간 점심 대 -> 아침으로 변경 -> 날짜 변동해도 계속 아침으로 나옴
Screen_recording_20250528_185320.webm
Issue
To reviewers
이게 최선일지....