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

[mod] 필터뷰 슈정 #221

Merged
merged 18 commits into from
Feb 14, 2024
Merged

[mod] 필터뷰 슈정 #221

merged 18 commits into from
Feb 14, 2024

Conversation

jihyunniiii
Copy link
Collaborator

@jihyunniiii jihyunniiii commented Feb 5, 2024

Related issue 🛠

Work Description ✏️

  • 리젝 이후 추가된 필터뷰의 수정사항을 반영하였습니다.
    UX Writing 수정
    건너뛰기 위치 변경
    성분공개 필터 중복선택 -> 단일선택 변경
    회원 필터 정보 변경하기 API 변경사항 반영 (request, response 모두 List 형태로 변경)
    유저 필터칩 조회 API 변경사항 반영 (response List 형태로 변경)
    필터 관련 택소노미 변경

Screenshot 📸

  • 필터 변경
Screen_recording_20240205_182958.mp4
  • 택소노미
스크린샷 2024-02-05 오후 6 29 25
  • 서버통신
스크린샷 2024-02-05 오후 6 31 51 스크린샷 2024-02-05 오후 6 31 37

Uncompleted Tasks 😅

  • 없는 것,,, 같습니다요,,,? 있다면 알려주세요,,?

To Reviewers 📢

끝났습니다요 ~ 신난당 ㅋ

Copy link
Member

@Dan2dani Dan2dani left a comment

Choose a reason for hiding this comment

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

나무랄게 없네영 !! 굳굳입니다 ~

@StringRes val titleRes: Int,
@StringRes val desRes: Int,
) {
GLUTENFREE(
R.string.bread_type_gluten_free_title,
R.string.bread_type_gluten_free_des
id = 1,
Copy link
Member

Choose a reason for hiding this comment

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

굳 제가 딱 원하던 방식이네여 ~

layout="@layout/view_bread_type_button"
selected="@{viewModel.breadFilterType.get(BreadFilterType.GLUTENFREE)}"
selected="@{viewModel.breadFilterTypeList.contains(BreadFilterType.GLUTENFREE.id)}"
Copy link
Member

Choose a reason for hiding this comment

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

건빵 코드를 다시 보니 ui 랜더링 시점에 ui를 어떻게 보여줄지를 결정하는 로직들이 많군용 지금 같이 contain 여부를 ui에서 확인하는 ! 이거 나중에 다 뷰모델 등으로 이전하고 ui에서는 정말 보여주기만 하는 방식으로에 대한 고민이 필요할 거 같아욤 나중에 이부분은 같이 고민해보면 좋을 거 같습니당 ~

Copy link
Member

Choose a reason for hiding this comment

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

저도 이번에 이 부분하면서 고민을 해보겠습니담

)
val breadFilterTypeList: MutableStateFlow<List<Int>> = MutableStateFlow(emptyList())
private val _nutrientFilterType = MutableStateFlow<NutrientFilterType?>(null)
val nutrientFilterType get() = _nutrientFilterType.asStateFlow()
val isFilterBtnEnabled: StateFlow<Boolean> = combine(
Copy link
Member

Choose a reason for hiding this comment

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

combine을 사용하지 않고 이를 해결할 수 있는 방법은 없는지 알아봐도 좋을 거 같아영

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

여러 개의 flow를 한 번에 관찰해야 하는 경우에 combine, zip, merge 등등 여러 방법을 사용할 수 있지만, 지금처럼 여러 개의 flow를 관찰하고 결과를 내야 하는 경우에는 combine을 사용하는 방법이 제일 적합할 것 같다는 생각이 듭니다요,,! 다른 좋은 방법이 있을까염?

@@ -33,33 +33,21 @@ class FilterSettingViewModel @Inject constructor(
val previousActivity get() = _previousActivity.asStateFlow()
Copy link
Member

Choose a reason for hiding this comment

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

지금 보니 flow가 정말 많이 사용됐다는 느낌이 드는거 같아욤 flow를 유사한 거 까리 뭉텅이로 같이 사용하는 방식으로 해도 좋을 거 같습니당

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

고민해보겠습니다요,,,

}

fun setUserFilter() {
viewModelScope.launch {
_mainPurposeType.value?.let {
RequestSettingFilter(
it.name,
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
Member

@Dan2dani Dan2dani left a comment

Choose a reason for hiding this comment

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

어프룹 답니다

# Conflicts:
#	app/src/main/java/com/sopt/geonppang/presentation/filterSetting/FilterSettingViewModel.kt
@jihyunniiii jihyunniiii merged commit 295649c into develop Feb 14, 2024
1 check passed
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.

[mod] 필터뷰 수정
2 participants