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

[Scrum 44] 뽀모도르 알림 UI/UX 개선 #145

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Conversation

lee-ji-hoon
Copy link
Collaborator

@lee-ji-hoon lee-ji-hoon commented Nov 9, 2024

작업 내용

  • 뽀모도르 잠금화면 알림 구현

상세한 코드의 내용은 코멘트에 추가해둘게

체크리스트

  • 빌드 확인
  • 타이머 기본적인 기능들 확인
  • 집중, 휴식 알림이 정상적으로 오는지
  • 타이머 노티가 정상적으로 떠있다가 제거까지 되는지

동작 화면

Screen_recording_20241109_215246.mp4

살려주세요

정말 많은 일이 있었어

@lee-ji-hoon lee-ji-hoon changed the title Scrum 44 [Scrum 44] 뽀모도르 알림 UI/UX 개선 Nov 9, 2024
Comment on lines +14 to +16
internal class PomodoroNotificationBitmapGenerator @Inject constructor(
@ApplicationContext private val context: Context
) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RemoteViews가 때려 죽여도 TextView에 Style이 안먹고 시스템 폰트로 가버려서 억지로 Text를 Canvas에 그리고 그 Bitmap을 ImageView에 세팅하는 형태로 구현

Copy link
Member

Choose a reason for hiding this comment

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

..오.... 세상에나...... 내 문해력이 부족해서 잘못 이해하고 있나 했어 진짜 많은 일이 있었구나....

Comment on lines 18 to 32
private val timeBitmaps: Map<String, Bitmap> by lazy {
generateNumberBitmaps(R.font.pretendard_bold, 40f, R.color.notification_pomodoro_time)
}

private val overtimeNumberBitmaps: Map<String, Bitmap> by lazy {
generateNumberBitmaps(R.font.pretendard_semibold, 18f, R.color.notification_pomodoro_over_time)
}

private val colonBitmap: Bitmap by lazy {
createColonBitmap(R.font.pretendard_bold, 40f, R.color.notification_pomodoro_time)
}

private val overtimeColonBitmap: Bitmap by lazy {
createColonBitmap(R.font.pretendard_semibold, 18f, R.color.notification_pomodoro_over_time)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

자주 사용되는 타이머의 text들은 Bitmap을 미리 갖고 있다가 재사용하는 형태로 구현해서 최소한의 리소스로 UI 표현되게 구현


예를 들어서 number 같은 경우에는 0~9 까지 10개의 Bitmap 만으로도 표현이 가능하기에 Map으로 갖고 있게 했읍니다..

Comment on lines +13 to +16
internal class PomodoroNotificationContentFactory @Inject constructor(
@ApplicationContext private val context: Context,
private val bitmapGenerator: PomodoroNotificationBitmapGenerator
) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Service에서 Notification의 Content를 만드는 부분만 로직 분리

Copy link
Member

Choose a reason for hiding this comment

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

oop 맛집이네요

val TIMER_DELAY = if (BuildConfig.DEBUG) 10L else 1_000L
val TIMER_DELAY = if (BuildConfig.DEBUG) 100L else 1_000L
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

debug라고 해도 10L은 너무 빠른거 같아서 수정

Copy link
Member

Choose a reason for hiding this comment

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

debug모드 위젯 타이머에서는 늘려도 ui 싱크가 조금 밀리긴하네 릴리즈는 잘 돌아가니까 okok

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이게 알림 notify를 너무 자주 하면 OS 상에서 그냥 막아버리는거 같더라고 무슨 경고가 로그캣에 계속 떠있더라고

Comment on lines +11 to +13
internal abstract class BasePomodoroTimer : PomodoroTimer {
private var timer: Timer? = null
private var timeElapsed = 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기본적인 Timer의 로직은 Focus나 Rest나 동일하기에 Base로 하나 만들고 Focus, Rest 각각 로직이 생기면 추가할 수 있게 구현

Comment on lines 25 to 30
<ImageView
android:id="@+id/text_status"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="4dp"
android:layout_marginTop="1dp" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ImageView 처럼 보이지만 실제는 10:00 이런 형태의 타이머를 표시하는 중

@lee-ji-hoon lee-ji-hoon marked this pull request as ready for review November 9, 2024 13:02
@github-actions github-actions bot requested a review from HyomK November 9, 2024 13:02
@lee-ji-hoon
Copy link
Collaborator Author

아직 우측 이미지나, 일부 텍스트가 피그마에 미구현인게 있어서 반영되면 바로 수정할게~!

@lee-ji-hoon lee-ji-hoon added the feature 기능 개발 label Nov 9, 2024
@@ -27,6 +27,8 @@ internal object ServiceModule {
): NotificationCompat.Builder = NotificationCompat.Builder(context, POMODORO_NOTIFICATION_CHANNEL_ID)
.setContentTitle(context.getString(R.string.app_name))
.setSmallIcon(R.drawable.ic_app_notification)
.setVibrate(null)
.setDefaults(0)
Copy link
Member

Choose a reason for hiding this comment

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

기본값 뭘 준 건지 찾아보다가 봤는데
https://developer.android.com/reference/androidx/core/app/NotificationCompat.Builder#setDefaults(int)

26이상부터는 해당이 없는것 같아

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
Collaborator Author

Choose a reason for hiding this comment

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

엉 맞어 노티 알림&진동 없이 주려고 한건데 없어도 되는지만 확인하고 빼버릴게!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

정상 동작 확인해서 아래와 같이 반영했어!

refactor: setDefaults(0) 제거

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
Collaborator Author

Choose a reason for hiding this comment

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

정말 많은 일이 있었다는 증명의 클래스야

Comment on lines +13 to +16
internal class PomodoroNotificationContentFactory @Inject constructor(
@ApplicationContext private val context: Context,
private val bitmapGenerator: PomodoroNotificationBitmapGenerator
) {
Copy link
Member

Choose a reason for hiding this comment

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

oop 맛집이네요

val contentView = createContentView(isRest)
val bigContentView = createBigContentView(category, time, overtime)
val notification = buildNotification(contentView, bigContentView)
notificationManager.notify(POMODORO_NOTIFICATION_ID, notification)
Copy link
Member

Choose a reason for hiding this comment

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

초당 notify 주는 방식 밖에 없는게 맞구나

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이거 말고 이것저것 해봤는데 notify가 최선인거 같더라고...

이미 알림에 RemoteView는 세팅된 값이라서 그 값만 수정해도 반영이 안되고...

val TIMER_DELAY = if (BuildConfig.DEBUG) 10L else 1_000L
val TIMER_DELAY = if (BuildConfig.DEBUG) 100L else 1_000L
Copy link
Member

Choose a reason for hiding this comment

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

debug모드 위젯 타이머에서는 늘려도 ui 싱크가 조금 밀리긴하네 릴리즈는 잘 돌아가니까 okok

@HyomK
Copy link
Member

HyomK commented Nov 12, 2024

내 기기(API 33) 에서는 위젯 아이콘이 이렇게 나온다
이거 디자인 대응 급하게 아니어서 저번에 넘겼던거 같은데 같이 들어가면 좋을 것 같습니다....!
image

@HyomK
Copy link
Member

HyomK commented Nov 12, 2024

브랜치 feature/ 안으로 안들어가도 괜찮나?

@lee-ji-hoon
Copy link
Collaborator Author

브랜치 feature/ 안으로 안들어가도 괜찮나?

아 이거 지라 티켓네임 그대로 들고왔는데 feature 붙이는걸로 할까?

내 기기(API 33) 에서는 위젯 아이콘이 이렇게 나온다
이거 디자인 대응 급하게 아니어서 저번에 넘겼던거 같은데 같이 들어가면 좋을 것 같습니다....!

이거 그럼 이번에 들어가자! 어떻게 수정해야 하는지 보고 디자인팀에 요청할게~!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants