-
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
[feat] 온보딩 추가 설명 뷰 구현 #193
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.
고생핑 ~
app/src/main/AndroidManifest.xml
Outdated
<activity | ||
android:name=".presentation.ui.onboarding.OnboardingExplanationActivity" | ||
android:exported="false" /> |
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.
android:screenOrientation="portrait"
tools:ignore="LockedOrientationActivity"
이거 추가해주고 위치 아래로 바꿔줘요 ~
app/src/main/AndroidManifest.xml
Outdated
<activity | ||
android:name=".presentation.ui.onboarding.OnboardingExplanationActivity" | ||
android:exported="false" /> |
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.
<activity | |
android:name=".presentation.ui.onboarding.OnboardingExplanationActivity" | |
android:exported="false" /> | |
<activity | |
android:name=".presentation.ui.onboarding.OnboardingExplanationActivity" | |
android:exported="false" | |
android:screenOrientation="portrait" | |
tools:ignore="LockedOrientationActivity" /> |
이렇게 바꿔주시구 OnBoardingActivity 아래로 위치 바꿔주세용
import org.sopt.pingle.R | ||
|
||
enum class OnboardingExplanationType(@StringRes val description: Int, @DrawableRes val image: Int) { | ||
// For the FIRST page, check layout > item_onboarding_expansion_tile.xml |
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.
영어 잘 하시네염 ㅋ
app/src/main/java/org/sopt/pingle/presentation/ui/auth/AuthActivity.kt
Outdated
Show resolved
Hide resolved
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.
온보딩 내에서도 onboarding과 onboardingExplanation으로 폴더링 나눠주면 좋을 것 같네용
글고 온보딩 네이밍 onboarding으로 통일시키는 거 저는 찬성이요 !
<com.google.android.material.tabs.TabLayout | ||
android:id="@+id/tl_onboarding_indicator" | ||
android:layout_width="wrap_content" | ||
android:backgroundTint="@color/g_11" |
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.
이렇게 안 하고 백그라운드 @null 로 설정하면 됨니다
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintStart_toStartOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" | ||
app:tabBackground="@drawable/selector_indicator_onboarding" |
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.
지금 배경에 지정한 게 drawable이자나요 근데 이걸 tabBackground에 지정하면 그 탭에 크기에 맞춰 꽉 차게 아이콘이 설정되는 것 같아요 (생각해보면 background니까 당연함)
그래서 svg로 적용하는 게 아니라 shape 만들어서 하면 (크기 지정 필수) 뭔가 잘 될 것 같아요 ㅋ.ㅋ
한 번 시도해보시겠서요? (안 될 수도? ㅋㅋ)
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.
아니면 tabLayout 자체에 고정 height를 줘보세염 !
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.
shape으로 만들어서 해결했습니다!!!!!!!!!!!!!!!!!!!!!!!!!!ㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜ
<TextView | ||
android:id="@+id/tv_onboarding_explanation_skip" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="@dimen/spacing34" | ||
android:layout_marginEnd="@dimen/spacing26" | ||
android:text="@string/on_boarding_explanation_skip_button" | ||
android:textAppearance="@style/TextAppearance.Pingle.Body.Med.14" | ||
android:textColor="@color/white" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" /> |
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.
넹 꽤 잘 눌리는데 함 물어볼까나요?
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.
터치영역 답변 온걸로 수정했서용
android:id="@+id/tv_onboarding_explanation_description" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="40dp" |
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.
20..?
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.
아 저기 이미지 기준 마진을 맞췄네욥 프레임 기준으로 해야되는데ㅜㅜ 감사합니당
android:id="@+id/tv_onboarding_explanation_title" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="@dimen/spacing24" |
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.
4 아닌갑쇼?
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.
고생하셨어요~!~
companion object { | ||
const val TAG = "AuthActivity" | ||
const val KAKAO_LOGIN_LADING = "Kakao Login Lading..." | ||
const val KAKAO_LOGIN_LOADING = "Kakao Login Loading..." |
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.
감삼핑 ㅋㅎㅋㅋ
@@ -87,7 +87,7 @@ class MoreFragment : BindingFragment<FragmentMoreBinding>(R.layout.fragment_more | |||
when (withDrawState) { | |||
is UiState.Success -> { | |||
kakaoAuthService.withdrawKakao() | |||
moveToSign() | |||
moveToOnboardingExplanation() |
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.
훨씬 직관적이네요
moveToOnboardingExplanation() | |
navigateToOnboardingExplanation() |
다른부분들에서 네이밍을 move 대신 navigate로 하니깐 요렇게 하는게 어떨까요?
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.
넹 다 navigate로 통일했습니다 ~!
app/src/main/java/org/sopt/pingle/presentation/ui/onboarding/OnboardingExplanationActivity.kt
Outdated
Show resolved
Hide resolved
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.
코리 감사합니다! 반영해쑵니다
app/src/main/java/org/sopt/pingle/presentation/ui/auth/AuthActivity.kt
Outdated
Show resolved
Hide resolved
@@ -87,7 +87,7 @@ class MoreFragment : BindingFragment<FragmentMoreBinding>(R.layout.fragment_more | |||
when (withDrawState) { | |||
is UiState.Success -> { | |||
kakaoAuthService.withdrawKakao() | |||
moveToSign() | |||
moveToOnboardingExplanation() |
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.
넹 다 navigate로 통일했습니다 ~!
app/src/main/java/org/sopt/pingle/presentation/ui/onboarding/OnboardingExplanationActivity.kt
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
override fun getItemCount(): Int = ONBOARDING_SIZE |
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.
이유는 모르겠는데 이걸 안 쓰면 리사이클러뷰 아이템들이 뷰에 안 끼워지더라구요ㅜ
private fun navigateToAuth() { | ||
Intent(this, AuthActivity::class.java).apply { | ||
startActivity(this) | ||
} | ||
} | ||
|
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.
돌아와야 해서 flag 없이 보냈고 뒤로가기 누르면 그냥 finish 돼서 보고 있던 온보딩 페이지로 돌아오도록 했숩니당!
<TextView | ||
android:id="@+id/tv_onboarding_explanation_skip" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="@dimen/spacing34" | ||
android:layout_marginEnd="@dimen/spacing26" | ||
android:text="@string/on_boarding_explanation_skip_button" | ||
android:textAppearance="@style/TextAppearance.Pingle.Body.Med.14" | ||
android:textColor="@color/white" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" /> |
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.
넹 꽤 잘 눌리는데 함 물어볼까나요?
<androidx.constraintlayout.widget.ConstraintLayout | ||
android:id="@+id/layout_onboarding" | ||
android:layout_width="match_parent" | ||
android:layout_height="wrap_content" | ||
android:layout_marginHorizontal="@dimen/spacing16" | ||
app:layout_constraintBottom_toBottomOf="parent" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintStart_toStartOf="parent"> |
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.
영역 잡아서 indicator랑 위에 리사이클러뷰 뜰 텍스트 사이 마진 잡으려고 한번 묶었습니다!
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintStart_toStartOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" | ||
app:tabBackground="@drawable/selector_indicator_onboarding" |
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.
shape으로 만들어서 해결했습니다!!!!!!!!!!!!!!!!!!!!!!!!!!ㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜㅜ
android:id="@+id/tv_onboarding_explanation_description" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="40dp" |
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.
아 저기 이미지 기준 마진을 맞췄네욥 프레임 기준으로 해야되는데ㅜㅜ 감사합니당
android:id="@+id/tv_onboarding_explanation_title" | ||
android:layout_width="wrap_content" | ||
android:layout_height="wrap_content" | ||
android:layout_marginTop="@dimen/spacing24" |
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.
쏘굿도리 ~
super.onDestroy() | ||
binding.vpOnboardingExplanation.adapter = 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.
이거 안정성을 위해 순서 바꿔주시는 게 좋을 것 같습니다 ~
} | ||
|
||
private fun initAdapter() { | ||
runCatching { |
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.
쏘 굿도리 ~
onContentsTheSame = { old, new -> old == new }, | ||
onItemsTheSame = { old, new -> old == new } |
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.
onContentsTheSame와 onItemsTheSame의 차이는 무엇일까요? 차이점을 바탕으로 고쳐주시면 좋을 것 같네요 ~
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.
onItemsTheSame은 Referential equality를 갖는지 판정, onContentsTheSame은 두 객체의 아이템이 Structural equality를 갖는지 판정하는 역할입니다! 구조적 동등성은 equals()를 확인하는 거구 참조적 동등성은 old Item과 new Item이 동일한 객체를 가리키고 있는지 확인하는 거라구 하네용.
같은 객체인지도 확인해주고 같은 내용인지도 확인해줌으로써 확실하게 old item과 new item이 같은지 확인해주는 거군용
<androidx.constraintlayout.widget.ConstraintLayout | ||
android:id="@+id/layout_onboarding" | ||
android:layout_width="match_parent" | ||
android:layout_height="wrap_content" | ||
android:layout_marginHorizontal="@dimen/spacing16" | ||
app:layout_constraintBottom_toBottomOf="parent" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintStart_toStartOf="parent"> |
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.
움 이해가 잘 안 가는데,, 버튼 기준으로 인디케이터 배치하고 건너뛰기 텍스트 배치한 것과 인디케이터 사이 간격 만큼 ViewPager 배치하면 될 것 같은데 아닌가요???
<data> | ||
|
||
<variable | ||
name="page" |
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.
네이밍을 그대로 OnboardingExplanationType으로 가져가는 게 직관적일 것 같다는 생각입니다
쏘굿 |
# Conflicts: # app/src/main/java/org/sopt/pingle/presentation/ui/main/more/MoreFragment.kt
Related issue 🛠
Work Description ✏️
Screenshot 📸
XRecorder_26022024_165333.mp4
Uncompleted Tasks 😅
To Reviewers 📢
헤헤..