-
Notifications
You must be signed in to change notification settings - Fork 2
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
상점 세부 페이지 기능 업데이트 #256
상점 세부 페이지 기능 업데이트 #256
Conversation
…igin/fix/store-detail # Conflicts: # koin/src/main/java/in/koreatech/koin/ui/store/fragment/StoreDetailEventFragment.kt
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.
고생했습니다~ 👻
import `in`.koreatech.koin.util.ext.observeLiveData | ||
|
||
class StoreDetailEventFragment : Fragment() { | ||
private var _binding: FragmentStoreDetailEventBinding? = 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.
Fragment Destroy 될 때, binding 할당 객체 해제해주어야 할 것 같아요~
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.
수정하겠습니당
import `in`.koreatech.koin.util.ext.observeLiveData | ||
|
||
class StoreDetailMenuFragment : Fragment() { | ||
private var _binding: FragmentStoreDetailMenuBinding? = 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.
위와 동일해요!
if(binding.storeDetailMainMenuTextView.text == category.name){ | ||
binding.storeDetailMainMenuButton.setCardBackgroundColor(ContextCompat.getColor(requireContext(), R.color.colorPrimary)) | ||
binding.storeDetailMainMenuTextView.setTextColor(Color.WHITE) | ||
} | ||
else if(binding.storeDetailSetMenuTextView.text == category.name){ | ||
binding.storeDetailSetMenuButton.setCardBackgroundColor(ContextCompat.getColor(requireContext(), R.color.colorPrimary)) | ||
binding.storeDetailSetMenuTextView.setTextColor(Color.WHITE) | ||
} | ||
else if (binding.storeDetailSideMenuTextView.text == category.name){ | ||
binding.storeDetailSideMenuButton.setCardBackgroundColor(ContextCompat.getColor(requireContext(), R.color.colorPrimary)) | ||
binding.storeDetailSideMenuTextView.setTextColor(Color.WHITE) | ||
} | ||
else{ | ||
binding.storeDetailRecommendMenuButton.setCardBackgroundColor(ContextCompat.getColor(requireContext(), R.color.colorPrimary)) | ||
binding.storeDetailRecommendMenuTextView.setTextColor(Color.WHITE) | ||
} |
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.
when 절로 바꿔보는게 어떨까요??
<androidx.constraintlayout.widget.ConstraintLayout | ||
android:layout_width="match_parent" | ||
android:layout_height="wrap_content" | ||
android:orientation="vertical"> |
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.
빼도 될듯합니다~
# Conflicts: # data/src/main/java/in/koreatech/koin/data/api/StoreApi.kt # data/src/main/java/in/koreatech/koin/data/mapper/StoreMapper.kt # data/src/main/java/in/koreatech/koin/data/repository/StoreRepositoryImpl.kt # data/src/main/java/in/koreatech/koin/data/source/remote/StoreRemoteDataSource.kt # koin/build.gradle
…-detail # Conflicts: # koin/src/main/java/in/koreatech/koin/ui/store/fragment/StoreDetailEventFragment.kt
storeDetailIsCardTextview.isVisible = false | ||
} | ||
|
||
setEtcInfo(storeDetailIsCardTextview, it.isCardOk) |
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.
setEtcInfo(storeDetailIsCardTextview, it.isCardOk)가 두 번 쓰이고 있는데 수정이 필요할 거 같습니당
그리고 isAvailable을 매개변수로 넘기는 것 보다 isAvailable의 조건이 참일때만 setEtcInfo 함수를 호출하도록 하는 게 구조적으로 더 좋을거 같다는 의견입니당
storeDetailEvent.visibility = View.VISIBLE | ||
storeDetailEventExpand.visibility = View.GONE | ||
} | ||
storeDetailEventTitleTextview.text = shopEvent.title |
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.
함수로 구조화하는 게 유지보수에 더 용이할 거 같습니다..!
viewModel.storeMenu.value?.let { | ||
category.menus?.forEachIndexed { menuIndex, shopMenus -> | ||
if (shopMenus.isSingle && shopMenus.singlePrice != null) { | ||
shopMenus.singlePrice |
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.
요건 잘못 쓰인걸까요?
) : BaseViewModel() { | ||
val store: LiveData<StoreWithMenu> get() = _store | ||
private val _store = MutableLiveData<StoreWithMenu>() | ||
val categories: LiveData<StoreMenu> get() = _categories | ||
private var _categories = MutableLiveData<StoreMenu>() |
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.
var는 잘못쓰인건가요??
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.
고생하셨습니다! 👍
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.
👻
개요
상점 세부 페이지 이벤트/공지 페이지 추가
메뉴 카테고리 별로 나누기
상세 작업 내용
메뉴 페이지
이벤트/공지 페이지