[ui] 다이얼로그 / 다이얼로그 디자인 통일 #116
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leeeha
approved these changes
Aug 16, 2023
| app:layout_constraintStart_toEndOf="@id/btn_dialog_negative" | ||
| app:layout_constraintTop_toBottomOf="@id/tv_dialog_sub" /> | ||
|
|
||
| </androidx.constraintlayout.widget.ConstraintLayout> |
There was a problem hiding this comment.
Ctrl + Alt + L 로 코드 자동 정렬하는 습관 들입시다~!
그리고 tools:text 속성에 하드코딩 한 문자열도 strings.xml 파일에 따로 추출하는 게 좋을 거 같네용
| private const val DIALOG_TITLE = "정말 로그아웃 하시겠어요?" | ||
| private const val DIALOG_SUB_TITLE = "로그아웃 후 장기간 미접속 시\n레벨이 내려갈 수 있습니다." | ||
| private const val DIALOG_NEGATIVE_BUTTON_LABEL = "취소" | ||
| private const val DIALOG_POSITIVE_BUTTON_LABEL = "로그아웃하기" | ||
| } |
There was a problem hiding this comment.
프래그먼트에서는 컨텍스트를 참조할 수 있으니까 util 패키지에 있는 stringOf 확장함수 사용해서
strings.xml 파일에 정의한 문자열을 가져오는 방식은 어떨까요??
There was a problem hiding this comment.
ui에 쓰이는 문자열은 string.xml 리소스에 추가해주는편이 더 좋을것 같습니다 감사합니다 !
| handleNegativeButton = {}, | ||
| handlePositiveButton = { viewModel.postLogout() } | ||
| ) | ||
| dialog.show(this.childFragmentManager, dialog.tag) | ||
| } |
There was a problem hiding this comment.
로그아웃에 성공했을 때만 로그인 화면으로 넘어가는 코드는 메인 액티비티에 있네요! 화긴화긴
|
|
||
| initDialogText(title, subTitle, positiveButtonLabel, negativeButtonLabel) | ||
| initNegativeButton(handleNegativeButton) | ||
| initPositiveButton(handlePositiveButton) |
There was a problem hiding this comment.
init[what]ButtonClickListener 라는 네이밍이 좀더 구체적이어서 좋은 거 같네용!
버튼을 초기화 한다고 네이밍 하면 클릭 리스너 외의 다른 동작도 해당 함수에 다 넣을 수 있을 것처럼 느껴져서요..!
ex) 버튼 텍스트/테두리/색상 초기화 등등
sxunea
approved these changes
Aug 16, 2023
|
|
||
| private fun initPositiveButton(handlePositiveButton: () -> Unit) { | ||
| binding.btnDialogPositive.setOnClickListener { | ||
| handlePositiveButton.invoke() |
| android:layout_height="wrap_content" | ||
| android:paddingHorizontal="14dp" | ||
| android:paddingBottom="16dp" | ||
| android:elevation="0dp" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Work Description
다이얼로그 디자인 통일
목표설정불가 다이얼로그 수정
📣 To Reviewers
리뷰 오늘까지 부탁드립니다 !
기존에 쓰던 다이얼로그에서 통일된 다이얼로그로 다음과 같이 사용하시면 됩니다 !
혹시 지금 방법보다 더 나은방법이 있다면 말씀 부탁드립니다 !