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

[FEAT/#5] 1주차 심화과제 (XML) #7

Open
wants to merge 13 commits into
base: develop-xml
Choose a base branch
from

Conversation

SYAAINN
Copy link
Collaborator

@SYAAINN SYAAINN commented Aug 26, 2024

📌 개요

✨ 작업 내용

  • Activiy 생명주기 알아보기 - [미완성]

  • ViewModel을 사용하여 State 관리해보기

  • UX 고려하여 기능 추가해보기

    • 뒤로가기 버튼 조정 - 메인에서 뒤로가기 클릭 시 앱 종료
    • 텍스트 입력 시 2줄 방지, 엔터키를 누르면 다음 입력칸으로 진행
    • 키보드가 올라오면 버튼도 키보드 위로 올라가도록 구현

✨ PR 포인트

  • ViewModel을 나름 분리해봤는데 Activity단에서 ViewModel을 올바르게 쓰고 있는지, 어떻게 개선할 수 있을지 짚어주시면 감사하겠습니다! 아 몰랑 다 태그해 ㅋㅋ 아무나 시간 있으신 분 놀러오세용
  • 2주차 과제때는 Fragment 분리를 중점으로 진행할 것 같습니다!

📸 스크린샷/동영상

@SYAAINN SYAAINN added ✨ FEAT 기능 개발 🔥 민재 민재 labels Aug 26, 2024
@SYAAINN SYAAINN self-assigned this Aug 26, 2024
@SYAAINN SYAAINN closed this Aug 26, 2024
@SYAAINN SYAAINN reopened this Aug 26, 2024
@SYAAINN SYAAINN changed the base branch from main-xml to develop-xml August 26, 2024 12:02
@SYAAINN SYAAINN changed the title [FEAT/#5] 1주차 심화과제 : UX 개선 [FEAT/#5] 1주차 심화과제 : UX 개선 (XML) Sep 8, 2024
@SYAAINN SYAAINN changed the title [FEAT/#5] 1주차 심화과제 : UX 개선 (XML) [FEAT/#5] 1주차 심화과제 (XML) Sep 18, 2024
@SYAAINN SYAAINN added the 심화과제 심화과제 label Sep 18, 2024
Copy link

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

막내아들 장하다 ㅜㅜ 고생하셨습니다!
XML 너무 오랜만이라 ㅜ,, 기억이 가물가물,,

Comment on lines +27 to +46
onBackPressedDispatcher.addCallback(this, object : OnBackPressedCallback(true) {
override fun handleOnBackPressed() {
if (System.currentTimeMillis() - backPressedTime < 2000) {
isEnabled = false
onBackPressedDispatcher.onBackPressed()

val intent = Intent(Intent.ACTION_MAIN)
intent.addCategory(Intent.CATEGORY_HOME)
intent.flags = Intent.FLAG_ACTIVITY_CLEAR_TOP
startActivity(intent)
finish()
} else {
showToast(
context = this@MainActivity,
message = getString(R.string.mypage_back_handler_caution)
)
backPressedTime = System.currentTimeMillis()
}
}
})

Choose a reason for hiding this comment

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

요거 모든 액티비티에서 사용된다면 확장함수로 빼는 방법도 좋을 것 같슴다!

Choose a reason for hiding this comment

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

base의 함정에 빠지지 않도록 신중히 결정해보세요!

}

private fun isIdValid(): Boolean {
return ID_VALIDATION_PATTERN.matcher(_user.value?.id ?: "").matches()

Choose a reason for hiding this comment

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

orEmpty() 에 대해 찾아보시면 좋을 것 같네요 ㅎ.ㅎ

Comment on lines +24 to +26
private fun isPasswordValid(): Boolean {
return PW_VALIDATION_PATTERN.matcher(_user.value?.password ?: "").matches()
}

Choose a reason for hiding this comment

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

Suggested change
private fun isPasswordValid(): Boolean {
return PW_VALIDATION_PATTERN.matcher(_user.value?.password ?: "").matches()
}
private fun isPasswordValid(): Boolean = PW_VALIDATION_PATTERN.matcher(_user.value?.password ?: "").matches()

이렇게 쓸 수도 있습니당

private val ID_VALIDATION_PATTERN: Pattern = Pattern.compile(ID_VALIDATION_REGEX)

private const val PW_VALIDATION_REGEX = "^[a-zA-Z0-9]{$PW_MIN_LENGTH,$PW_MAX_LENGTH}$"
private val PW_VALIDATION_PATTERN: Pattern = Pattern.compile(PW_VALIDATION_REGEX)

Choose a reason for hiding this comment

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

코틀린에서는 Pattern 말고 Regex를 이용해 정규식을 사용할 수 있는데 이에 대해 공부해보셔도 좋을 것 같아요!

android:layout_marginStart="60.dp"
android:layout_marginTop="60.dp"

Choose a reason for hiding this comment

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

Suggested change
android:layout_marginTop="60.dp"
android:layout_marginTop="60dp"

XML에서는 그냥 이렇게 적어주셔도 됩니다

@@ -10,12 +9,11 @@
android:id="@+id/ivMyPageProfile"

Choose a reason for hiding this comment

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

Suggested change
android:id="@+id/ivMyPageProfile"
android:id="@+id/iv_my_page_profile"

보통 xml에서 id를 네이밍할 때는 snake_case를 사용합니다

Copy link
Member

Choose a reason for hiding this comment

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

a: 이 부분은 각 팀에서 정하는 컨벤션이라 필수는 아닌 것 같아요 :)

Copy link

@kez-lab kez-lab 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 6 to 13
@Parcelize
data class User(
val id: String,
val password: String,
val nickname: String,
val phoneNumber: String
val id: String? = "",
val password: String? = "",
val nickname: String? = "",
val phoneNumber: String? = ""
) : Parcelable

Copy link

Choose a reason for hiding this comment

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

기본값이 empty 인데 nullable한 타입인 이유는 null로 넣어주는 상황이 있어서 그런건가요?

Comment on lines 31 to +34
private fun getUserInfo() {
user = intent.getSafeParcelable(KeyStorage.USER_INFO, User::class.java)
val user = intent.getSafeParcelable(KeyStorage.USER_INFO, User::class.java)
viewModel.setUser(user)
}
Copy link

Choose a reason for hiding this comment

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

viewModel에서 SavedStateHandle라는걸 쓰면 Intent에 직접 접근해서 값을 Setting할 수 있습니다.
번거롭게 View -> ViewModel로 Set 하는 로직이 줄어들 수 있어요

Comment on lines +8 to +13
class SignInViewModel : ViewModel() {
private val _user = MutableLiveData<User?>()
val user: LiveData<User?> = _user

private val _signInState = MutableLiveData(false)
val signInState: LiveData<Boolean> = _signInState
Copy link

Choose a reason for hiding this comment

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

Kotlin 2.0을 쓴다면? explicit backing fields 를 활용할 수 있어욥

Suggested change
class SignInViewModel : ViewModel() {
private val _user = MutableLiveData<User?>()
val user: LiveData<User?> = _user
private val _signInState = MutableLiveData(false)
val signInState: LiveData<Boolean> = _signInState
val user: LiveData<User?>
field = MutableLiveData<User?>()
val signInState: LiveData<Boolean>
field = MutableLiveData(false)

관련 문서

Comment on lines +49 to 51
viewModel.validateSignUp()
setupObservers()
}
Copy link

Choose a reason for hiding this comment

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

클릭 될때마다 옵저버가 구독되면 어떤 일이 발생할까요?!

Copy link
Member

@MoonsuKang MoonsuKang left a comment

Choose a reason for hiding this comment

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

멋있다 민재야..


onBackPressedDispatcher.addCallback(this, object : OnBackPressedCallback(true) {
override fun handleOnBackPressed() {
if (System.currentTimeMillis() - backPressedTime < 2000) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (System.currentTimeMillis() - backPressedTime < 2000) {
if (SystemClock.elapsedRealtime() - backPressedTime < 2000) {

안정성 말고 별 차이는 존재하지 않지만 간격 측정이나 타이머 같은 경우 이렇게 쓰기도 한답니다~

}
setupObservers()
Copy link
Member

Choose a reason for hiding this comment

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

ViewModel의 signInState가 변경되면 이 함수의 Observer가 변화를 자동으로 감지해서 UI 업데이트 처리를 하기 때문에 버튼 클릭 시마다 호출하지 않아도 될 것 같습니다. onCreate로 이동시켜도 괜찮을 것 같네용

@@ -24,9 +22,9 @@
android:layout_marginStart="40.dp"
Copy link
Member

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋ 컴포즈에 지배되어 버린 민재...

Choose a reason for hiding this comment

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

으악

app:layout_constraintStart_toEndOf="@+id/tvMyPagePhoneNumberTitle"
app:layout_constraintTop_toTopOf="@+id/tvMyPagePhoneNumberTitle"
app:layout_constraintBottom_toBottomOf="@+id/tvMyPagePhoneNumberTitle"/>
app:layout_constraintTop_toTopOf="@+id/tvMyPagePhoneNumberTitle" />

<Button
android:id="@+id/btnMyPageNicknameChange"
android:layout_width="match_parent"
Copy link
Member

Choose a reason for hiding this comment

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

이왕 ConstraintLayout을 사용한다면 각 View요소들의 width를 0dp로 두고 제약조건을 추가하는 방법이 좋습니다!

Copy link
Member

@junseo511 junseo511 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 +8 to +11
val id: String? = "",
val password: String? = "",
val nickname: String? = "",
val phoneNumber: String? = ""
Copy link
Member

Choose a reason for hiding this comment

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

c: empty값을 기본적으로 부여하는 이유가 있으실까요?

val id: String? = "",
val password: String? = "",
val nickname: String? = "",
val phoneNumber: String? = ""
Copy link
Member

Choose a reason for hiding this comment

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

c: 트레일링 콤마에 대해 아실까요?


onBackPressedDispatcher.addCallback(this, object : OnBackPressedCallback(true) {
override fun handleOnBackPressed() {
if (System.currentTimeMillis() - backPressedTime < 2000) {
Copy link
Member

Choose a reason for hiding this comment

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

c: 매직넘버의 상수화에 대해 아시나요?

Comment on lines +33 to +37
val intent = Intent(Intent.ACTION_MAIN)
intent.addCategory(Intent.CATEGORY_HOME)
intent.flags = Intent.FLAG_ACTIVITY_CLEAR_TOP
startActivity(intent)
finish()
Copy link
Member

Choose a reason for hiding this comment

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

c: 함수로 분리해줘도 괜찮을 것 같아요

intent.flags = Intent.FLAG_ACTIVITY_CLEAR_TOP
startActivity(intent)
finish()
} else {
Copy link
Member

Choose a reason for hiding this comment

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

c: 'else 예약어를 쓰지 않는다' 라는 말을 아시나요? when으로 분기해도 가독성이 좋아보일듯요

@@ -14,34 +15,43 @@ import com.sopt.now.presentation.utils.showToast

class SignInActivity : AppCompatActivity() {
private lateinit var binding: ActivitySigninBinding
private var user: User? = null
private lateinit var viewModel: SignInViewModel
Copy link
Member

Choose a reason for hiding this comment

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

c: 조금 게으르게 선언은 어떠심

Choose a reason for hiding this comment

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

by viewModels() 알아보세요!

}

fun validateSignIn(inputId: String, inputPassword: String) {
_signInState.value = inputId == _user.value?.id && inputPassword == _user.value?.password
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_signInState.value = inputId == _user.value?.id && inputPassword == _user.value?.password
_signInState.value = inputId == user.value?.id && inputPassword == user.value?.password

c: _가 붙는 의미가 무엇일까요?


onSignUpClicked()
}

private fun setupObservers() {
Copy link
Member

Choose a reason for hiding this comment

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

c: 여러개의 옵저버를 세팅해주고 있는지 함수명에 대해 고민해봅시다!

@@ -10,12 +9,11 @@
android:id="@+id/ivMyPageProfile"
Copy link
Member

Choose a reason for hiding this comment

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

a: 이 부분은 각 팀에서 정하는 컨벤션이라 필수는 아닌 것 같아요 :)

Copy link

@librarywon librarywon 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 +27 to +46
onBackPressedDispatcher.addCallback(this, object : OnBackPressedCallback(true) {
override fun handleOnBackPressed() {
if (System.currentTimeMillis() - backPressedTime < 2000) {
isEnabled = false
onBackPressedDispatcher.onBackPressed()

val intent = Intent(Intent.ACTION_MAIN)
intent.addCategory(Intent.CATEGORY_HOME)
intent.flags = Intent.FLAG_ACTIVITY_CLEAR_TOP
startActivity(intent)
finish()
} else {
showToast(
context = this@MainActivity,
message = getString(R.string.mypage_back_handler_caution)
)
backPressedTime = System.currentTimeMillis()
}
}
})

Choose a reason for hiding this comment

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

base의 함정에 빠지지 않도록 신중히 결정해보세요!

import androidx.appcompat.app.AppCompatActivity
import com.sopt.now.R
import com.sopt.now.data.User
import com.sopt.now.databinding.ActivityMainBinding
import com.sopt.now.presentation.utils.KeyStorage
import com.sopt.now.presentation.utils.getSafeParcelable
import com.sopt.now.presentation.utils.showToast

class MainActivity : AppCompatActivity() {
private lateinit var binding: ActivityMainBinding
private var user: User? = null

Choose a reason for hiding this comment

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

user 를 클래스의 멤버변수로 둘 정도로 활용을 많이 하는지 의문입니다. 게다가 nullable 하고 기본값은 null? 코틀린의 장점을 살리지 못한다고 생각합니다.

@@ -14,34 +15,43 @@ import com.sopt.now.presentation.utils.showToast

class SignInActivity : AppCompatActivity() {
private lateinit var binding: ActivitySigninBinding
private var user: User? = null
private lateinit var viewModel: SignInViewModel

Choose a reason for hiding this comment

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

by viewModels() 알아보세요!

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
binding = ActivitySigninBinding.inflate(layoutInflater)
setContentView(binding.root)
viewModel = ViewModelProvider(this).get(SignInViewModel::class.java)

Choose a reason for hiding this comment

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

ViewModelProvider가 어떤 친구인지 공부하면 왜 Hilt의 고마움을 느낄 수 있어요

Comment on lines +28 to +41
private fun isNicknameValid(): Boolean {
return NICKNAME_VALIDATION_PATTERN.matcher(_user.value?.nickname ?: "").matches()
}

private fun isPhoneNumberValid(): Boolean {
return PHONE_NUMBER_VALIDATION_PATTERN.matcher(_user.value?.phoneNumber ?: "").matches()
}

fun validateSignUp() {
_signUpState.value = isIdValid() &&
isPasswordValid() &&
isNicknameValid() &&
isPhoneNumberValid()
}

Choose a reason for hiding this comment

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

도메인이 생긴다면 이 로직은 어디에 두고 어떻게 사용할지 고민하는 것도 재밌는 고민이 될 것 같습니다

@@ -24,9 +22,9 @@
android:layout_marginStart="40.dp"

Choose a reason for hiding this comment

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

으악

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ FEAT 기능 개발 심화과제 심화과제 🔥 민재 민재
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants