[feat] 푸쉬알림 / 알림권한 요청 #231
Conversation
| private val notiType by lazy { intent.extras?.getString(KEY_NOTI_TYPE, "") } | ||
| private val feedId by lazy { intent.extras?.getString(KEY_FEED_ID) } | ||
| private val notificationPermissionLauncher = | ||
| registerForActivityResult(ActivityResultContracts.RequestPermission()) {} |
There was a problem hiding this comment.
어라랏 여기서 유저의 퍼미션 허용 여부에 따라 처리해주는 코드가 있어야 할 거 같은데 왜 내용이 비어있는지 알 수 있을까요?!
// 예시 코드
private val cameraPermissionLauncher =
registerForActivityResult(ActivityResultContracts.RequestPermission()) { isGranted ->
if (isGranted) {
startCamera(binding.viewFinder)
} else {
Toast.makeText(this, "권한을 받아오지 못했습니다.", Toast.LENGTH_SHORT).show()
finish()
}
}There was a problem hiding this comment.
퍼미션 허용 처리후 처리해야할 사항이 정해져있지않아서 내용을 비워두었습니다. 스낵바 메시지로 유저한테 허용여부를 알려주는 방식으로 수정하겠습니다.
There was a problem hiding this comment.
아아 근데 실행시켜봤는데 스낵바 띄우는 건 기디랑 같이 논의된 사항이 아니어서 아무것도 안 띄우는 게 맞는 거 같습니다!
| NotificationType.LIKE_NOTIFICATION, NotificationType.COMMENT_NOTIFICATION | ||
| -> navigateToDetail(feedId?.toInt()) | ||
|
|
||
| else -> navigateToLevelupHelp() |
There was a problem hiding this comment.
앱을 처음 실행시킬 때, 여기 else 브랜치에 걸려서 스플래시 다음에 항상 레벨업 가이드 화면이 뜨더라구요!
분기 처리를 더 해줘야 할 거 같아요!
|
| dialog.show( | ||
| parentFragmentManager, | ||
| TAG_NOTIFICATION_OFF_DIALOG | ||
| ) |
There was a problem hiding this comment.
넵 그게 더 좋을 것 같습니다 !
|
|
||
| private fun initNotificationPermissionChangeButtonClickListener() { | ||
| binding.llMypageAgreePermissionChange.setOnClickListener { | ||
| presentNotificationSetting(requireContext()) |
There was a problem hiding this comment.
여기서 present 동사는 show의 의미로 쓰인 건가요??
present가 여러 의미를 갖고 있다보니 약간 헷갈려서 물어봅니다!
There was a problem hiding this comment.
혼동의 여지가 있어 navigateToNotificationSetting으로 변경했습니다.
| } | ||
| } | ||
|
|
||
| fun notificationSettingOreo(context: Context): Intent { |
There was a problem hiding this comment.
저희의 컨벤션 문서에 명시된 내용은 아니지만, 함수는 동사로 시작하는 경우가 많아서 네이밍도 조금 더 신경써주면 좋을 거 같아요 :)
| binding.ivMypageAgree.isVisible = true | ||
| binding.llMypageAgreePermissionChange.isGone = true | ||
| binding.tvMypageAgreePermission.isGone = true | ||
| when (data.fcmIsAllowed) { |
There was a problem hiding this comment.
isNotificationAllowed vs. data.fcmIsAllowed 이 두가지가 어떤 차이가 있는건지 혼동할 여지가 조금 있어 보입니다!
isNotificationAllowed 는 알림 권한과 관련되어 있으니 permission 이라는 단어를 포함시키는 건 어떨까요??
| checkFromWineyFeed() | ||
| } | ||
|
|
||
| private fun initCheckNotificationPermission() { |
There was a problem hiding this comment.
init, check 동사가 연달아 있으니까 뭔가 어색한데 😅
initNotificationPermissionState() 이런 네이밍은 어떨까요?!
| binding.ivMypageAgree.isGone = true | ||
| binding.llMypageAgreePermissionChange.isVisible = true | ||
| binding.tvMypageAgreePermission.isVisible = true | ||
| } |
There was a problem hiding this comment.
퍼미션 허용 여부에 따라 버튼 뷰를 변경하는 코드이므로, changeNotiButtonByPermission 이런 이름은 어떨까요?!
| @@ -215,6 +223,8 @@ | |||
| <string name="snackbar_delete_fail">죄송합니다. 다시 시도해주세요.</string> | |||
| <string name="snackbar_report_success">정상적으로 신고되었습니다 :)</string> | |||
| <string name="snackbar_report_fail">죄송합니다. 신고접수에 실패하였습니다.</string> | |||
| <string name="snackbar_notification_permission_fail">"기기 설정을 변경해야 알림을 받을 수 있어요"</string> | |||
| <string name="snackbar_mypage_permission_fail">"기기 설정을 변경해야 알림을 받을 수 있어요"</string> | |||
|
|
|||
There was a problem hiding this comment.
동일한 문자열 리소스는 하나만 만들어도 될 거 같아요! 그리고 문자열 리소스에 큰 따옴표는 없어도 될 거 같습니다 :)
| private const val VAL_MY_FEED_SCREEN = "MyFeedFragment" | ||
|
|
||
| private const val CODE_NOTIFICATION_PERMISSION = 1020 |
| binding.root, | ||
| true, | ||
| stringOf(R.string.snackbar_notification_permission_fail) | ||
| ) |
There was a problem hiding this comment.
named argument 적용해주면 좋을 거 같아요!
저도 위니 코드 작성할 때 놓치고 있던 부분인데, 어떤 인자인지 바로 알아챌 수 있어서 좋더라구요!
그리고 권한 거부된 경우에는 스낵바의 isSuccess 인자를 false로 설정하는 게 자연스러울 거 같아요!
📝 Work Description
📸 Screenshot