Skip to content
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

[fix] 3차 스프린트 QA 수정사항 반영 #245

Merged
merged 27 commits into from
Mar 12, 2024

Conversation

jihyunniiii
Copy link
Collaborator

@jihyunniiii jihyunniiii commented Mar 10, 2024

Related issue 🛠

Work Description ✏️

  • 3차 스프린트 QA 수정사항을 반영하였습니다.
    • 서치바 -> 입력 전 검색 버튼 비활성화
    • 지도/리스트뷰 -> 검색창에서 뒤로가기 클릭 시 검색 취소 + 검색 페이지 이동
    • 랭킹 -> 숫자 크기 12Bold -> 16Bold
    • 랭킹 -> 데이터 텍소노미 스크롤 시작 시점으로 변경
    • 단체 변경 후 홈 화면 진입 시 검색결과 유지되어 있는 문제 해결
    • 검색 + 지도 뷰에서 칩 필터링을 진행하는 경우 검색 결과가 없어도 지도뷰 유지
    • 리이슈 문제 완전 해결 ㅋ

Screenshot 📸

서치바 -> 입력 전 검색 버튼 비활성화

Screen_recording_20240310_103628.mp4

지도/리스트뷰 -> 검색창에서 뒤로가기 클릭 시 검색 취소 + 검색 페이지 이동

Screen_recording_20240310_160134.mp4

단체 변경 후 홈 화면 진입 시 검색결과 유지되어 있는 문제 해결

Screen_recording_20240311_144431.mp4

검색 + 지도 뷰에서 칩 필터링을 진행하는 경우 검색 결과가 없어도 지도뷰 유지

Screen_recording_20240311_215422.mp4

Uncompleted Tasks 😅

  • 단체 변경 후 홈 화면 진입 시 검색결과 유지되어 있는 문제 해결
  • 검색 + 지도 뷰에서 칩 필터링을 진행하는 경우 검색 결과가 없어도 지도뷰 유지

To Reviewers 📢

  • 토큰 재발급.. 따로 이슈 파서 진행하겠습니다 ㅋㅋ 하암,,,
  • 서치바 입력 전 검색 버튼 비활성화 -> 서치바가 있는 모든 뷰에 적용해달라고 하셔서 커스텀 뷰 만든 부분을 수정하였습니다. 요거 있는 뷰 담당한 분은 제외하고 해주시면 될 것 같아요 ~
  • 검색 뷰 키보드 관련 디자인 측과 얘기해서 제외하기로 결정했습니다.
  • 지도/리스트뷰 뒤로가기 관련 제 실기기에서 잘 되는 거 확인했는데,, 다른 분들도 한 번 확인해주시면 좋을 것 같습니다!
  • 단체 변경 후 홈 화면 진입 시 검색결과 유지되어 있는 문제 해결 -> 이거 해결은 했는데,, 로직이 맘에 안 들어요,, 근데 SharedPreference를 사용하고 있는 한, 이 방법이 최선일 것 같기도 합니다,, (OnSharedPreferenceChangeListener를 사용해서 해결했어요)
  • 검색 + 지도 뷰에서 칩 필터링을 진행하는 경우 검색 결과가 없어도 지도뷰 유지 -> ㅋㅋ 이전 검색 값을 저장하는 방식으로 구현하였는데,,, 로직이 맘에 안 들어요,, 좋은 방법 있다면 의견 플리즈
  • 리이슈 문제 완전 해결 (인 듯함 ㅋㅋ)

Copy link
Member

@DoReMinWoo DoReMinWoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고고고고고고고고고고생핑~

Comment on lines 147 to 151
return if (groupName.length > GROUP_NAME_MAX_LENGTH) {
"${groupName.substring(SUBSTRING_START_INDEX, SUBSTRING_END_INDEX) + ELLIPSIS}"
} else {
groupName
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 {} 없이 한줄로 깔끔하게 고고!?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DoReMinWoo 제가 커밋한 부분이네욥 ㅋ.ㅋ 지현언니가 수정해준대요...

Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

빠르다빨라

override fun handleOnBackPressed() {
homeViewModel.clearSearchWord()
navigateToSearch()
if (homeViewModel.searchWord.value.isNullOrEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 진짜 확장함수 쓰기 은근 어렵지 않나여?,, ㅜ.ㅜ 확장함수 그거 하나로 다 해결하고 싶은데 도저히 어떻게 해야할지 머르겠네요.. 온보딩 추가설명 뷰에서도 결국엔 실패했는데

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아니 그니까용,,, 뒤로가기 버튼 관리 쉽지 않타,,, 이거 이번 스프린트 끝나고 어떻게 하면 잘 관리할 수 있을지 고민해 봐야겠서염 ㅠ

Comment on lines 147 to 151
return if (groupName.length > GROUP_NAME_MAX_LENGTH) {
"${groupName.substring(SUBSTRING_START_INDEX, SUBSTRING_END_INDEX) + ELLIPSIS}"
} else {
groupName
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DoReMinWoo 제가 커밋한 부분이네욥 ㅋ.ㅋ 지현언니가 수정해준대요...

@jihyunniiii
Copy link
Collaborator Author

코리 달아준 이후로 수정한 부분이 많아서 코드 한 번만 다시 봐주시면 감사하겠슴둥

@DoReMinWoo @HAJIEUN02

Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지도뷰는 진짜ㅜ.ㅜ 너무 어려워보이네요 늘 고생하는 젼언니 짱..

}

init {
localStorage.sharedPreference.registerOnSharedPreferenceChangeListener(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 sharedPreference 내용 바뀌는 거 관찰하려고 localDataSource에 쉐프 그 자체를 추가한 거군용😶‍🌫️

private val _category = MutableStateFlow<CategoryType?>(null)
val category get() = _category.asStateFlow()
private val sharedPreferenceChangeListener =
SharedPreferences.OnSharedPreferenceChangeListener { _, key ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

진짜 OnSharedPreferenceChangeListener 이런 게 있는지는 상상도 못했네요,, 좋은 정보 감사합니당

private val _mainListPingleListState = MutableSharedFlow<UiState<List<MainListPingleModel>>>()
val mainListPingleListState get() = _mainListPingleListState.asSharedFlow()

fun setCategory(category: CategoryType?) {
_category.value = category
private fun clearPingleFiler() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기 Filter인데 Filer로 오타난 것 같아욤

val category get() = _category.asStateFlow()
private val sharedPreferenceChangeListener =
SharedPreferences.OnSharedPreferenceChangeListener { _, key ->
if (key == GROUP_ID) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변화한 값(key)가 GROUP_ID인 경우에 클리어해주는 로직이군여!_! 근데 groupName도 같이 바뀌니까 GROUP_ID && GROUP_NAME 이런 식으로 둘다 처리해주면 어떨까요!? 그러면 좀더 안정적이지 않을까요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그럼 아마 if문 안으로 안 들어갈 거에요 ㅠ 이게 바뀔 때마다 수행되는 거라서 ㅜ

homeViewModel.pingleFilter.flowWithLifecycle(viewLifecycleOwner.lifecycle)
.distinctUntilChanged()
.onEach { pingleFilter ->
Log.e("ㅋㅋ", pingleFilter.toString())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로그 지워주셔도 될 것 같아염ㅎ.ㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와 싓 감삼다

@@ -142,15 +143,24 @@ class MyGroupActivity : BindingActivity<ActivityMyGroupBinding>(R.layout.activit
}
}

private fun makeEllipsisGroupName(groupName: String): String = when {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요 부분 when문으로 변경해주신거 반영해서 확장함수로 빼놨어요 제 피알에서!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes ~~

HAJIEUN02 and others added 5 commits March 12, 2024 14:40
# Conflicts:
#	app/src/main/java/org/sopt/pingle/presentation/ui/mygroup/MyGroupActivity.kt
# Conflicts:
#	app/src/main/java/org/sopt/pingle/presentation/ui/mygroup/MyGroupActivity.kt
@jihyunniiii jihyunniiii merged commit f6fb272 into develop Mar 12, 2024
1 check passed
@jihyunniiii jihyunniiii deleted the fix-3rd-sprint-qa-jihyun branch March 12, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[fix] 3차 스프린트 QA 수정사항 반영
3 participants