#13 [feat] 온보딩 테마 선택 뷰 #16

emjayMJkim · 2024-01-09T18:23:49Z

📑 Work Description

온보딩 뷰 테마 선택 구현 완
테마 4번째 아이템 클릭 비활성화 + 토스트 메시지 띄우기
테마 3개 선택 시 버튼 활성화

🛠️ Issue

closed [feat] 온보딩 - 테마 선택 #13

📷 Screenshot

feat.onboarding-theme.choice.webm

💬 To Reviewers

stellar-halo

넘 든든하네요,, 최고!

변수는 항상 class의 맨 위에 있어야 합니다!
반복되는 부분은 함수나 변수로 빼주세요! 코틀린은 변수에 함수도 넣을 수 있으니까요~ 🧸

stellar-halo · 2024-01-10T04:28:21Z

app/src/main/java/com/sopetit/softie/ui/onboarding/OnboardingViewModel.kt

+    private val _layoutTranslucent: MutableLiveData<Boolean> = MutableLiveData(false)
+    val layoutTranslucent: LiveData<Boolean>
+        get() = _layoutTranslucent


변수 맨 위로 올려주세요~

헉 이게 무슨 일이죠... 바로 고치겠습니닷

stellar-halo · 2024-01-10T04:32:34Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeAdapter.kt

+    private var onItemClickListener: ((Theme) -> Unit)? = null
+    fun setOnThemeClickListener(listener: (Theme) -> Unit) {
+        onItemClickListener = listener
+    }
+
+    var selectedThemeArray = arrayListOf<Int>()


마찬가지 입니다! 변수들은 클래스의 맨 위에 올려주세요!

오우 감사합니다!!

stellar-halo · 2024-01-10T04:33:26Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeAdapter.kt

+    var selectedThemeArray = arrayListOf<Int>()
+
+    private fun themeSelection(binding: ItemOnboardingChoiceThemeBinding, theme: Theme) {
+        if (selectedThemeArray.size == 3) {


3도 MAXIMUM_어쩌구,, 같이 companion으로 빼주세요!

정말 이런 발견 너무 감사합니다..

stellar-halo · 2024-01-10T04:37:10Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeAdapter.kt

+
+    private fun themeSelection(binding: ItemOnboardingChoiceThemeBinding, theme: Theme) {
+        if (selectedThemeArray.size == 3) {
+            if (selectedThemeArray.contains(theme.themeId)) {


var isSelected = selectedThemeArray.contains(theme.themeId)

이렇게 boolean을 가지는 변수는 is로 시작하는 것이 좋습니다! (+ 가독성, 재사용성을 위해 변수로 뺀 다음 if문에 넣어주세요~

네엡! 수정했습니당

stellar-halo · 2024-01-10T04:38:15Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeAdapter.kt

+                selectedThemeArray.removeAt(selectedThemeArray.indexOf(theme.themeId))
+                changeThemeBackground(binding, false)
+            }
+        } else {
+            if (selectedThemeArray.contains(theme.themeId)) {
+                selectedThemeArray.removeAt(selectedThemeArray.indexOf(theme.themeId))
+                changeThemeBackground(binding, false)


selectedThemeArray.removeAt(selectedThemeArray.indexOf(theme.themeId)) changeThemeBackground(binding, false)

반복되는 코드이니 함수로 따로 빼는 것이 어떨까요~?
fun removeItem 같이 따로 빼주세요!

함수로 따로 빼주는 게 더 좋을 것 같네요! 수정하겠습니다

stellar-halo · 2024-01-10T04:42:32Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeFragment.kt

+            binding.tvOnboardingChoiceThemeSpeech.text =
+                getString(R.string.onboarding_choice_theme_speech_after)


바꾸라는 건 아니고 전 삼항 연산자^^,,를 선호하는 편이긴 합니다!
click하면 liveData 값을 true로 바꾼 뒤(or false) xml에서 string을 처리하긴 해요
이건 스타일 선호 차이니까 선호하시는 거로 작업하세요~

stellar-halo · 2024-01-10T04:43:14Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeFragment.kt

+
+    private fun setThemeBtn() {
+        viewModel.selectedThemeArray.observe(viewLifecycleOwner) {
+            if (it.size == 3) {


3 마찬가지입니다~ 여기서 따로 빼고 adapter에서 받아서 써도 되겠네요!

stellar-halo · 2024-01-10T04:44:32Z

app/src/main/res/layout/activity_onboarding.xml

+                android:layout_width="0dp"
+                android:layout_height="0dp"
+                android:background="@color/onboarding_translucent"
+                android:visibility="@{viewModel.layoutTranslucent ? View.VISIBLE : View.GONE}"


^--^
여기서 이미 layoutTranslucent으로 삼항 연산자를 처리하고 계시네요!
그렇다면 이참에 string 값도 layoutTranslucent값을 이용해서 삼항 연산자로 처리해볼까요?

pump9918

background 잘 설정한 거 같은데 왜 시연영상은 색이 다르게 보이는걸까요... 디스플레이 이슈인가 고생하셨습니다!

pump9918 · 2024-01-10T04:54:05Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeAdapter.kt

@@ -16,10 +18,55 @@ class ChoiceThemeAdapter :
        )
    ) {

-    class ChoiceThemeViewHolder(private val binding: ItemOnboardingChoiceThemeBinding) :
+    inner class ChoiceThemeViewHolder(private val binding: ItemOnboardingChoiceThemeBinding) :


내부에 inner 클래스를 선언하는게 ViewHolder를 따로 파는 것과 차이가 있나요?(그냥 궁금)

사실 코틀린에서 inner class를 사용하는 것은 권장되지는 않습니다!

https://shinjekim.github.io/kotlin/2019/08/29/Kotlin-%EB%82%B4%EB%B6%80-%ED%81%B4%EB%9E%98%EC%8A%A4(inner-class)%EC%99%80-%EC%A4%91%EC%B2%A9-%ED%81%B4%EB%9E%98%EC%8A%A4(nested-class)/

이거 읽어보세요~

저는 clicklistener 때문에 inner class로 만들었습니다! 메모리 누수 문제는 destroy되면 메모리 해제할 수 있도록 수정했습니다!

minemi00

수고하셨습니당!

minemi00 · 2024-01-10T04:52:58Z

app/src/main/java/com/sopetit/softie/ui/onboarding/themechoice/ChoiceThemeAdapter.kt

+            if (selectedThemeArray.contains(theme.themeId)) {
+                selectedThemeArray.removeAt(selectedThemeArray.indexOf(theme.themeId))
+                changeThemeBackground(binding, false)
+            } else {
+                selectedThemeArray.add(theme.themeId)
+                changeThemeBackground(binding, true)


선택된 테마 id가 포함되어 있으면 해당 id의 인덱스가 삭제되는 건가여../?

네 맞습니당!

stellar-halo

고생했어요~!~!

stellar-halo · 2024-01-10T06:10:33Z

app/src/main/res/layout/fragment_onboarding_choice_theme.xml

@@ -42,7 +88,7 @@
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:layout_marginVertical="14dp"
-            android:text="@string/onboarding_choice_theme_speech"
+            android:text="@{themeViewModel.layoutTranslucent ? @string/onboarding_choice_theme_speech : @string/onboarding_choice_theme_speech_after}"


💞💞💞

emjayMJkim added 11 commits January 9, 2024 23:07

#13 [add] 테마 선택 btn background 생성

1bb9c91

#13 [feat] adapter item selection 구현

5e8c0c1

#13 [feat] theme selection 구현

cb10dec

#13 [feat] 테마 3개 선택시 버튼 활성화 & 버튼 클릭시 화면 전환

da8c6ee

#13 [feat] 선택된 테마 drawable 변경 구현

3d84caa

#13 [add] 테마 3개 이상 선택 시 로직 추가

0d0af43

#13 [fix] merge conflict 해결

e8d0d95

#13 [mod] 선택된 테마 넘기기 로직 수정

a7d9ab8

#13 [ui] 반투명 background 사라질 시 말풍선 text 변화

1de5f19

#13 [ui] 반투명 background top bar 적용 문제 해결

e965643

#13 [ui] 온보딩 top bar 위치 조정

7e63e76

emjayMJkim requested review from stellar-halo, minemi00 and pump9918 January 9, 2024 18:23

emjayMJkim self-assigned this Jan 9, 2024

emjayMJkim added Pull Request pr 날림! Feat 새로운 기능 추가 민정🦊 민정이가 작업함! labels Jan 9, 2024

#13 [add] 테마 3개 이상 선택 시 toast 추가

21d7286

stellar-halo requested changes Jan 10, 2024

View reviewed changes

pump9918 approved these changes Jan 10, 2024

View reviewed changes

minemi00 approved these changes Jan 10, 2024

View reviewed changes

emjayMJkim added 2 commits January 10, 2024 14:20

#13 [mod] 코드 리뷰 반영 코드 수정

fbe5c4e

#13 [mod] 코드 리뷰 반영 코드 수정

1a8455a

stellar-halo approved these changes Jan 10, 2024

View reviewed changes

emjayMJkim merged commit 3a962a1 into develop Jan 10, 2024
1 check passed

emjayMJkim deleted the feature/#13-onboarding-theme branch January 10, 2024 06:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#13 [feat] 온보딩 테마 선택 뷰 #16

#13 [feat] 온보딩 테마 선택 뷰 #16

emjayMJkim commented Jan 9, 2024 •

edited

Loading

stellar-halo left a comment

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024

stellar-halo Jan 10, 2024

stellar-halo Jan 10, 2024

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024

pump9918 left a comment

pump9918 Jan 10, 2024

stellar-halo Jan 10, 2024

emjayMJkim Jan 10, 2024 •

edited

Loading

minemi00 left a comment

minemi00 Jan 10, 2024

emjayMJkim Jan 10, 2024

stellar-halo left a comment

stellar-halo Jan 10, 2024

		binding.tvOnboardingChoiceThemeSpeech.text =
		getString(R.string.onboarding_choice_theme_speech_after)

#13 [feat] 온보딩 테마 선택 뷰 #16

#13 [feat] 온보딩 테마 선택 뷰 #16

Conversation

emjayMJkim commented Jan 9, 2024 • edited Loading

📑 Work Description

🛠️ Issue

📷 Screenshot

💬 To Reviewers

stellar-halo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pump9918 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emjayMJkim Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

minemi00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stellar-halo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emjayMJkim commented Jan 9, 2024 •

edited

Loading

emjayMJkim Jan 10, 2024 •

edited

Loading