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

[mod] 알림 권한 거부 시 뜨는 스낵바 디자인 변경 #237

Merged
merged 20 commits into from
Feb 4, 2024

Conversation

leeeha
Copy link
Member

@leeeha leeeha commented Jan 23, 2024

📝 Work Description

  • WineySnackbar 클래스에서 SnackbarType 에 따라 스낵바 디자인 바뀌도록 구현
  • 알림 권한 거부 시, 시스템 설정창으로 바로 이동하는 스낵바 액션 추가
  • 마이페이지 프래그먼트 코드를 약간 리팩토링 했어요! (함수 이름 및 정의 순서 변경)

📣 To Reviewers

이번주 평일 내로 리뷰 부탁드립니다!

@leeeha leeeha added mod 〰 코드 및 내부 파일 수정 하은 🐰 labels Jan 23, 2024
@leeeha leeeha requested a review from sxunea January 23, 2024 08:37
@leeeha leeeha self-assigned this Jan 23, 2024
@leeeha leeeha requested a review from Sangwook123 January 23, 2024 09:30
stringOf(R.string.snackbar_feed_delete_success)
anchorView = binding.root,
message = stringOf(R.string.snackbar_feed_delete_success),
type = SnackbarType.WineyFeedResult(isSuccess = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

위니에서도 named Argument 적극 도입해보자 아자잣 👍

package org.go.sopt.winey.util.view

sealed class SnackbarType {
data class WineyFeedResult(
Copy link
Contributor

Choose a reason for hiding this comment

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

WineyFeedResult와 NotiPermission을 data class로 특별한 이유가 있나요 ?

Copy link
Member Author

@leeeha leeeha Jan 27, 2024

Choose a reason for hiding this comment

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

UiState에서 자식 클래스들이 Loading, Empty, Success, Failure 상태를 나타냈던 것처럼,

제가 만든 SnackbarType의 자식 클래스들도 위니피드의 결과 (업로드, 삭제 등), 알림 권한과 같이 '어떤 스낵바 타입인지'를 나타내는데 사용됩니다. 그리고 멤버 변수로는 각 타입에 따라 isSuccess, onActionClicked 같은 정보를 포함하고 있습니다.

즉, 이 클래스들은 일반 클래스와 달리 어떤 함수도 갖지 않으며, 자신의 스낵바 타입이 무엇인지 나타내는 용도로 사용되기 때문에 데이터 클래스로 선언하는 게 적합하다 생각했습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

데이터 클래스도 안에 함수를 가질 수 있기도 하고, WineyFeedResult가 데이터보다는 특정 타입이다보니 클래스로 만들어도 될 것 같아서요 ! 다만 구현 방향이 바뀌었다보니 이젠 의미가 없어진 것 같긴하네요 ㅎㅎㅎ 수정한거 얼른 읽어보겠습니다 👍

Copy link
Member Author

@leeeha leeeha Jan 27, 2024

Choose a reason for hiding this comment

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

오호! 저는 sealed class 안에 있는 WineyFeedResult, NotiPermission 클래스가 자신의 멤버 변수를 함수에서 따로 사용하지 않고, 단지 자신의 타입과 관련된 '데이터를 쥐고 있는 역할'만 하고 있다보니! 일반 클래스 보다는 데이터 클래스 느낌이 조금 더 강하다고 느꼈습니다!

깊은 고민 없이 평소 스타일대로 구현했는데, 코멘트 덕분에 조금 더 생각해볼 수 있는 계기가 되었네요 :) 감사합니다 😄

Comment on lines 64 to 92
private fun initResultIcon() {
when (type) {
is SnackbarType.WineyFeedResult -> {
if (type.isSuccess) {
binding.ivSnackbarResult.setImageDrawable(context.drawableOf(R.drawable.ic_snackbar_success))
} else {
binding.ivSnackbarResult.setImageDrawable(context.drawableOf(R.drawable.ic_snackbar_fail))
}
}

is SnackbarType.NotiPermission -> {
binding.ivSnackbarResult.visibility = View.GONE
}
}
}

private fun initActionText() {
when (type) {
is SnackbarType.WineyFeedResult -> {
binding.tvSnackbarAction.visibility = View.INVISIBLE
}

is SnackbarType.NotiPermission -> {
binding.tvSnackbarAction.apply {
visibility = View.VISIBLE
paintFlags = Paint.UNDERLINE_TEXT_FLAG
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

initResultIcon(), initActionText(), setAction()은 타입에 상관없이 모두 실행되어야 하니까 이걸 인터페이스로 묶고, WineyFeedResultNotiPermission 을 클래스로 변경한다음 인터페이스를 구현하는 방식으로 변경하는 건 어떨까요? 그렇게 되면 타입이 추가되더라도 WineySnackbar의 코드가 변경될 일이 없어서 확장성까지 얻을 수 있을 것 같아서요

Copy link
Member Author

@leeeha leeeha Jan 27, 2024

Choose a reason for hiding this comment

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

오호 인터페이스로 만들 생각은 못했는데 시도해볼게요! 👍

결국 핵심은 뷰 타입이 추가되더라도 WineySnackbar 클래스 내용에 변경사항이 없도록 하는 것이겠네요! 저도 그에 대한 필요성에 공감합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

WineySnackber 클래스를 추상 클래스로 만들고, initResultIcon(), setAction() 함수를 추상 함수로 만들어서

스낵바의 타입에 따라 오버라이딩 하는 함수 내용이 다르도록 구현해봤습니다! 푸시한 코드 확인 부탁드려요 :)

공통적인 내용은 WineySnackbar 클래스에서 상속 받고, 스낵바의 각 타입에 따라 자식 클래스에서 오버라이딩 하는 함수 내용이 다르도록 변경
@leeeha leeeha requested a review from sxunea February 4, 2024 13:55
Copy link
Contributor

@sxunea sxunea left a comment

Choose a reason for hiding this comment

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

확인완료 ! 고생했습니다 ~ 👍

…o feature/ui-noti-permission-denied-snackbar

# Conflicts:
#	app/src/main/res/values/strings.xml
@leeeha leeeha merged commit 674de19 into develop Feb 4, 2024
1 check passed
@leeeha leeeha deleted the feature/ui-noti-permission-denied-snackbar branch February 22, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod 〰 코드 및 내부 파일 수정 하은 🐰
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[mod] 알림 권한 거부 시 뜨는 스낵바 디자인 변경
2 participants