-
Notifications
You must be signed in to change notification settings - Fork 1
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
[FEAT/YAF-116] 알림이 울릴 때 해제 혹은 미루기가 가능한 UI를 표시한다 #102
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GOOD
val backgroundStrokePx = backgroundWidth.toPx() | ||
val progressStrokePx = progressWidth.toPx() | ||
|
||
val offset = (backgroundStrokePx - progressStrokePx) / 2 | ||
|
||
val backgroundColor = Color.White.copy(alpha = 0.2f) | ||
|
||
val progressBlurEffect = BlurMaskFilter( | ||
progressBlurRadius.toPx(), | ||
BlurMaskFilter.Blur.NORMAL, | ||
) | ||
|
||
val progressPaint = Paint().apply { | ||
color = OrbitTheme.colors.main.toArgb() | ||
maskFilter = progressBlurEffect | ||
style = Paint.Style.STROKE | ||
strokeWidth = progressStrokePx | ||
strokeCap = Paint.Cap.ROUND | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3
이거 Canvas의 draw Scope범위 내로 옮기는게 좋지 않을까요?
toPx같은 경우는 Density에 의존하게 되는데 Canvas의 Scope 내부에서는 이미 Density 컨텍스트가 설정되어 있어서, toPx 변환이 더 최적화될 수 있을 것 같다고 생각해서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅇㅈ
totalSeconds = 300, | ||
) | ||
|
||
Spacer(modifier = Modifier.weight(1f)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4
이거 빼고
Column(
verticalArrangement = Arrangement.SpaceBetween
)
로 해서 같은 결과라면, 이게 더 좋은 구조일 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private fun formatSecondsToTime(seconds: Int): String { | ||
val minutes = seconds / 60 | ||
val remainingSeconds = seconds % 60 | ||
return String.format(Locale.getDefault(), "%02d:%02d", minutes, remainingSeconds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p4
val locale = Locale.getDefault()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
locale을 여러 곳이 아니라 한 곳에서만 사용한다면 inline variable이라도 상관없다는 생각
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매번 Locale.getDefault()를 호출하는 대신
toString().padStart()를 활용하도록 변경했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
Related issue 🛠
closed #89
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
2025-02-09.2.00.17.mov
Uncompleted Tasks 😅
N/A
To Reviewers 📢
자기 전에 깃허브에서 여러 알람앱을 찾아봤는데 결국에는 알람 해제 액티비티를 따로 분리해야할 것 같아요
그래서 추후에 AlarmAction, AlarmSnoozeTimer는 따로 모듈화 예정입니다.