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

6주차 세미나 과제 #12

Merged
merged 18 commits into from
Jun 14, 2023
Merged

6주차 세미나 과제 #12

merged 18 commits into from
Jun 14, 2023

Conversation

b1urrrr
Copy link
Member

@b1urrrr b1urrrr commented Jun 2, 2023

⛳️ Work Description

Essential

  • 홈 화면도 UI Layer 만들어보기
  • 경고 정도는 이제 쉽죠?

Advanced

  • (합동세미나 과제 연동) 기존 코드를 UI Layer를 활용해서 코드 분리하기 🔗
  • 양방향 데이터바인딩으로 코드 로직 개선
  • 로딩뷰 구현

Challenge

  • Data Layer 분리

📸 Screenshot

Signup

Android.Emulator.-.Galaxy_S21_API_31_5554.2023-06-03.02-19-25.mp4

Loading Dialog

Android.Emulator.-.Galaxy_S21_API_31_5556.2023-06-03.02-01-28.mp4

📢 To Reviewers

  • 이전에 미리 작업해놓은 것들이 많아서 경고와 로딩 뷰만 구현했습니다.
  • 로딩 뷰 처음 구현해봤는데 개선할 부분이 있는지 확인 부탁드립니다!
  • 정규식에서 '{' can be simplified to '*'라는 경고가 뜨는데 지금 상태보다 더 간략하게 만들 수 있는 걸까요? 더 간단한 정규식 표기법을 알고 계신다면 알려주세요!

@b1urrrr b1urrrr self-assigned this Jun 2, 2023
@b1urrrr b1urrrr added essential 필수 과제 advanced 심화 과제 challenge 도전 과제 labels Jun 2, 2023
Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

항상 배워갈 게 많은 채연이 코드!! 이번주도 고생했어요 :)

import org.android.go.sopt.util.binding.BindingDialogFragment

class LoadingDialogFragment :
BindingDialogFragment<FragmentLoadingDialogBinding>(R.layout.fragment_loading_dialog) {
Copy link
Member

Choose a reason for hiding this comment

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

오호 다이얼로그 프래그먼트도 이렇게 base를 만들어서 사용할 수 있네요! 리팩토링 때 반영하겠습니다 :)

@@ -16,12 +17,15 @@ import org.android.go.sopt.util.extension.showSnackbar
import org.android.go.sopt.util.extension.showToast
import org.android.go.sopt.util.state.RemoteUiState.Error
import org.android.go.sopt.util.state.RemoteUiState.Failure
import org.android.go.sopt.util.state.RemoteUiState.Loading
Copy link
Member

@leeeha leeeha Jun 4, 2023

Choose a reason for hiding this comment

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

Loading도 UiState 중 하나로 분류해서 하니까 코드가 간결하네요! 구웃~~


private var _loadingDialog: LoadingDialogFragment? = null
private val loadingDialog
get() = requireNotNull(_loadingDialog) { getString(R.string.dialog_not_initialized_error_msg) }
Copy link
Member

Choose a reason for hiding this comment

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

오 이렇게 backing properties를 사용하니까 변수에 대한 불변성과 널 안정성이 더 높아진 거 같네요! 👍

@@ -40,36 +43,28 @@ class SignupViewModel @Inject constructor(
private val specialty: String
get() = _specialty.value?.trim() ?: ""

private fun isValidId() = id.isNotBlank() && id.length in MIN_ID_LENGTH..MAX_ID_LENGTH
val isValidId: LiveData<Boolean> = _id.map { id -> checkId(id) }
val isValidPwd: LiveData<Boolean> = _pwd.map { pwd -> checkPwd(pwd) }
Copy link
Member

Choose a reason for hiding this comment

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

LiveData의 map 사용법 배워갑니다 👍

Copy link

Choose a reason for hiding this comment

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

저도 쇽샥해야곘네요!

private const val REGEX_ID_PATTERN =
"""^(?=.*[a-zA-Z])(?=.*\d).{$MIN_ID_LENGTH,$MAX_ID_LENGTH}$"""
private const val REGEX_PWD_PATTERN =
"""^(?=.*[a-zA-Z])(?=.*\d)(?=.*[~!@#$%^&*()?]).{$MIN_PWD_LENGTH,$MAX_PWD_LENGTH}$"""
Copy link
Member

Choose a reason for hiding this comment

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

아이디, 비밀번호 길이의 최소 최대 값 상수화 어떻게 시켜야 하는지 몰랐는데 배워갑니다!!

this,
getColorStateList(context, R.color.coral_700),
)
}
Copy link
Member

@leeeha leeeha Jun 4, 2023

Choose a reason for hiding this comment

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

오호 EditText의 확장함수를 만들어서 테두리 색상을 바꾸는 방법도 있네요! 머티리얼 컴포넌트인 Text Field를 사용하면 app:errorEnabled="true" 이 한 줄로 편하게 에러 상태를 표시할 수 있더라구요! 참고하면 좋을 거 같아요 :)

Copy link
Member Author

@b1urrrr b1urrrr Jun 10, 2023

Choose a reason for hiding this comment

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

커스텀이 까다롭다고 느껴져 EditText를 활용해서 구현했었는데 머터리얼 컴포넌트에서도 안내하고 있는 컴포넌트였군요 😮 유용한 리포지토리와 속성 알려주셔서 감사합니다 💖 기회가 되면 활용해봐야겠어요!

app:layout_constraintTop_toTopOf="parent"
app:lottie_autoPlay="true"
app:lottie_loop="true"
app:lottie_rawRes="@raw/ic_square_loading" />
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

Choose a reason for hiding this comment

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

로티애니매이션뷰 처음 봤는데 찾아봐야겠네요 ! 굿굿

Copy link

@taeheeL taeheeL 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 +24 to +30
private var _repoItemAdapter: RepoItemAdapter? = null
private val repoItemAdapter
get() = requireNotNull(_repoItemAdapter) { getString(R.string.adapter_not_initialized_error_msg) }

private var _loadingDialog: LoadingDialogFragment? = null
private val loadingDialog
get() = requireNotNull(_loadingDialog) { getString(R.string.dialog_not_initialized_error_msg) }
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 Author

Choose a reason for hiding this comment

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

by lazyval로만 선언이 가능해서 리소스 해제를 못 하겠더라구요
by lazyby ViewModels() 말고는 위임을 제대로 활용해본 적이 없는데 위임으로 처리 가능한 다른 방법이 있다면 알려주세요!

Comment on lines +49 to +51
private fun checkId(id: String): Boolean {
return id.isEmpty() || Pattern.matches(REGEX_ID_PATTERN, id)
}
Copy link

Choose a reason for hiding this comment

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

blank을 사용하시지 않은 이유가 있나여?

Choose a reason for hiding this comment

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

현재 조건이 비어있거나, 패턴에 맞을 때 true -> 에러가 없는 것 같은데
공백만 있는 상황에 error를 넣을지, 넣지 말지를 생각해봐야겠네요 ! 요건 아무래도 개인차인듯?

Copy link
Member Author

Choose a reason for hiding this comment

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

별 생각 없이 isEmpty로 썼는데, id 가져올 때 trim() 처리를 하도록 구현해놔서 isBlank랑 똑같을 것 같네여

Copy link
Member

@Marchbreeze Marchbreeze left a comment

Choose a reason for hiding this comment

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

훔쳐갈 좋은 소스들이 많네요 ㅎㅎㅎㅎㅎ 최고

is Error -> {
dismissLoadingDialog()
showSnackbar(binding.root, getString(R.string.unknown_error_msg))
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

아하 로딩 다이얼로그를 따로 프래그먼트로 세워두고 적용시키는 방식이군요 !! 훔쳐갈게요

@@ -1,6 +1,7 @@
package org.android.go.sopt.util.state

sealed class RemoteUiState {
object Loading : RemoteUiState()
object Success : RemoteUiState()
data class Failure(val code: Int?) : RemoteUiState()
object Error : RemoteUiState()
Copy link
Member

Choose a reason for hiding this comment

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

이것도 같이 훔쳐갈게요 ㅎㅎ State들을 따로 저장해둘수가 있네요 !

Choose a reason for hiding this comment

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

저도 쇽삭하겠습니다 감사합니다 ㅋ

Copy link

@amourxyoung amourxyoung left a comment

Choose a reason for hiding this comment

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

갓채연 ! 갓채연 ! 갓채연 ! 갓채연 ! 갓채연 ! 갓채연 ! 갓채연 ! 갓채연 ! 갓채연 ! 갓채연 !

loadingDialog.show(parentFragmentManager, TAG_LOADING_DIALOG)
}

is Success -> {

Choose a reason for hiding this comment

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

오,,, 각 상태별로 로직을 처리하니까 가독성이 훨씬 좋네요 👍

@@ -1,6 +1,7 @@
package org.android.go.sopt.util.state

sealed class RemoteUiState {
object Loading : RemoteUiState()
object Success : RemoteUiState()
data class Failure(val code: Int?) : RemoteUiState()
object Error : RemoteUiState()

Choose a reason for hiding this comment

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

저도 쇽삭하겠습니다 감사합니다 ㅋ


if (!isValidInput()) {
_loginState.value = Failure(CODE_INVALID_INPUT)
return@launch

Choose a reason for hiding this comment

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

혹시 이거는 무슨 의미인지 알 수 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

로그인 할 때 아이디랑 비밀번호 중 하나라도 비워져있으면 오류 메시지를 띄우도록 하는 코드입니다!

Copy link

@lsakee lsakee left a comment

Choose a reason for hiding this comment

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

채연이 화이팅 !! 배워갑니다!

@@ -40,36 +43,28 @@ class SignupViewModel @Inject constructor(
private val specialty: String
get() = _specialty.value?.trim() ?: ""

private fun isValidId() = id.isNotBlank() && id.length in MIN_ID_LENGTH..MAX_ID_LENGTH
val isValidId: LiveData<Boolean> = _id.map { id -> checkId(id) }
val isValidPwd: LiveData<Boolean> = _pwd.map { pwd -> checkPwd(pwd) }
Copy link

Choose a reason for hiding this comment

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

저도 쇽샥해야곘네요!

app:layout_constraintTop_toTopOf="parent"
app:lottie_autoPlay="true"
app:lottie_loop="true"
app:lottie_rawRes="@raw/ic_square_loading" />
Copy link

Choose a reason for hiding this comment

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

로티애니매이션뷰 처음 봤는데 찾아봐야겠네요 ! 굿굿

import org.android.go.sopt.databinding.FragmentLoadingDialogBinding
import org.android.go.sopt.util.binding.BindingDialogFragment

class LoadingDialogFragment :

Choose a reason for hiding this comment

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

혹시 LoadingDialogFragment의 BindingDialogFragment를 따로 만들어둔 특별한 이유가 있는지 궁금합니당 어차피 하나의 LoadingDialogFragment로만 계속 사용하지 않나용?

Copy link
Member Author

Choose a reason for hiding this comment

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

로그아웃 경고 문구 구현할 때에도 BindingDialogFragment를 활용했었습니당
다른 DialogFragment가 추가되는 경우를 고려하면 유지보수에도 좋을 것 같네요!

@b1urrrr b1urrrr merged commit 83558e2 into develop Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advanced 심화 과제 challenge 도전 과제 essential 필수 과제
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants