[UI] 공통 알림 페이지 구현 완료 #42
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough알림(Notifications) 페이지 UI가 새롭게 구현되었습니다. 알림 화면, 알림 필터 행, 알림 데이터 모델, 새로운 색상 및 문자열 리소스가 추가되었고, 기존 알림 카드의 스타일이 일부 수정되었습니다. 이 변경들은 알림 관련 화면 및 컴포넌트의 신설과 시각적 개선을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AlarmScreen
participant AlarmFilterRow
participant AlarmItemList
participant CardAlarm
User->>AlarmScreen: 화면 진입
AlarmScreen->>AlarmFilterRow: 필터 행 표시(selectedStates)
User->>AlarmFilterRow: 필터 버튼 클릭
AlarmFilterRow->>AlarmScreen: onToggle(index)
AlarmScreen->>AlarmItemList: 알림 데이터 필터링
loop 알림 리스트 표시
AlarmScreen->>CardAlarm: 알림 카드 렌더링(AlarmItem)
User->>CardAlarm: 알림 카드 클릭
CardAlarm->>AlarmScreen: onCardClick(AlarmItem)
AlarmScreen->>AlarmItemList: isRead 상태 갱신
end
alt 알림 없음
AlarmScreen->>User: 빈 상태 메시지 및 아이콘 표시
end
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt (1)
17-20: 매개변수 순서를 개선할 수 있습니다.코드 가독성을 위해 콜백 함수를 마지막에 배치하는 것이 일반적입니다.
@Composable fun AlarmFilterRow( - selectedStates: BooleanArray, - onToggle: (Int) -> Unit + selectedStates: BooleanArray, + onToggle: (Int) -> Unit ) {app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
42-47: 필터링 로직 가독성 개선현재 필터링 로직은 정확하지만 가독성을 위해 더 명확하게 표현할 수 있습니다.
- val filteredList = when { - selectedStates[0] && !selectedStates[1] -> alarms.filter { it.badgeText == stringResource(R.string.alarm_feed) } - !selectedStates[0] && selectedStates[1] -> alarms.filter { it.badgeText == stringResource(R.string.alarm_group) } - selectedStates[0] && selectedStates[1] -> alarms - else -> alarms - } + val isFeedSelected = selectedStates[0] + val isGroupSelected = selectedStates[1] + val feedText = stringResource(R.string.alarm_feed) + val groupText = stringResource(R.string.alarm_group) + + val filteredList = when { + isFeedSelected && !isGroupSelected -> alarms.filter { it.badgeText == feedText } + !isFeedSelected && isGroupSelected -> alarms.filter { it.badgeText == groupText } + else -> alarms + }
128-132: 프리뷰 데이터의 하드코딩된 문자열프리뷰에서 하드코딩된 한국어 문자열을 사용하고 있습니다. 이전 학습 내용에 따르면 사용자는 문자열 리소스 추출을 선호합니다.
하드코딩된 문자열들을
strings.xml로 추출하여 일관성을 유지하고 유지보수성을 향상시키는 것을 고려해보세요:
- "내 글을 좋아합니다."
- "user123님이 내 글에 좋아요를 눌렀어요."
- "같이 읽기를 시작했어요!"
- 기타 프리뷰 문자열들
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.idea/misc.xml(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/mock/AlarmItem.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardAlarm.kt(3 hunks)app/src/main/res/drawable/ic_notification.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/res/values/strings.xml (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
🔇 Additional comments (12)
.idea/misc.xml (2)
1-1: XML 선언 추가는 문제없음UTF-8 선언을 추가한 변경은 무해하며 파일 가독성을 높입니다.
6-6: IDE 전용 설정 커밋 여부 재확인 필요
ExternalStorageConfigurationManager컴포넌트는 개인 IDE 환경(예: 외부 스토리지 위치 설정)에만 영향을 주는 항목일 가능성이 큽니다. 팀 전체에 필요하지 않은 설정이라면.idea/하위 파일을.gitignore에 추가하거나, 공유가 필요한 설정만 제한적으로 커밋하는 것이 충돌과 불필요한 diff를 줄이는 데 도움이 됩니다.app/src/main/res/drawable/ic_notification.xml (1)
1-9: 알림 아이콘 구현이 깔끔합니다.벡터 드로어블 아이콘이 올바른 형식으로 구현되어 있고, 빈 상태 UI에서 사용할 목적에 적합합니다.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/mock/AlarmItem.kt (1)
3-10: 알림 데이터 모델이 잘 설계되었습니다.알림에 필요한 모든 속성이 포함되어 있고,
isRead를var로 선언하여 읽음 상태를 변경할 수 있도록 한 것이 적절합니다.app/src/main/java/com/texthip/thip/ui/common/cards/CardAlarm.kt (5)
90-90: 간격 조정이 적절합니다.뱃지와 내용 사이의 간격을 8.dp로 설정하여 적절한 여백을 제공합니다.
94-95: 레이아웃 구조가 개선되었습니다.
weight(1f)와verticalArrangement.Center를 사용하여 콘텐츠 영역의 레이아웃을 명확하게 정의했습니다.
103-103: 텍스트 스타일 적용이 적절합니다.제목 텍스트에
view_m500_s14스타일을 적용하여 일관된 디자인을 유지합니다.
126-126: 시간 표시 텍스트 스타일이 적절합니다.
timedate_r400_s11스타일을 사용하여 시간 정보를 명확하게 표시합니다.
134-134: 메시지 영역 간격이 적절합니다.메시지 텍스트 위에 12.dp의 간격을 두어 내용 구분을 명확하게 했습니다.
app/src/main/res/values/strings.xml (1)
169-174: 알림 관련 문자열 리소스가 적절히 추가되었습니다.새로운 알림 기능에 필요한 문자열들이 올바르게 추가되었고, 한국어 번역도 자연스럽습니다. 문자열을 외부화하여 유지보수성과 다국어 지원에 도움이 됩니다.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
1-34: 전체적인 구조 승인패키지 구조와 import 문들이 잘 정리되어 있고, 필요한 모든 종속성이 포함되어 있습니다. 문자열 리소스 사용도 좋은 관례를 따르고 있습니다.
97-115: LazyColumn 구현 승인LazyColumn에서
key매개변수를 사용하여 성능을 최적화하고, 적절한 간격과 패딩을 설정한 점이 좋습니다. 알림 읽음 상태 업데이트 로직도 올바르게 구현되어 있습니다.
| OptionChipButton( | ||
| text = stringResource(R.string.alarm_feed), | ||
| isFilled = true, | ||
| onClick = { onToggle(0) } | ||
| ) | ||
| OptionChipButton( | ||
| text = stringResource(R.string.alarm_group), | ||
| isFilled = true, | ||
| onClick = { onToggle(1) } | ||
| ) |
There was a problem hiding this comment.
필터 상태가 UI에 반영되지 않는 문제가 있습니다.
selectedStates 매개변수를 받고 있지만 실제로는 사용하지 않고 있습니다. 모든 버튼이 항상 isFilled=true로 설정되어 있어 필터의 선택 상태가 시각적으로 표현되지 않습니다.
다음과 같이 수정하여 선택 상태를 반영하도록 해주세요:
OptionChipButton(
text = stringResource(R.string.alarm_feed),
- isFilled = true,
+ isFilled = selectedStates[0],
onClick = { onToggle(0) }
)
OptionChipButton(
text = stringResource(R.string.alarm_group),
- isFilled = true,
+ isFilled = selectedStates[1],
onClick = { onToggle(1) }
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OptionChipButton( | |
| text = stringResource(R.string.alarm_feed), | |
| isFilled = true, | |
| onClick = { onToggle(0) } | |
| ) | |
| OptionChipButton( | |
| text = stringResource(R.string.alarm_group), | |
| isFilled = true, | |
| onClick = { onToggle(1) } | |
| ) | |
| OptionChipButton( | |
| text = stringResource(R.string.alarm_feed), | |
| isFilled = selectedStates[0], | |
| onClick = { onToggle(0) } | |
| ) | |
| OptionChipButton( | |
| text = stringResource(R.string.alarm_group), | |
| isFilled = selectedStates[1], | |
| onClick = { onToggle(1) } | |
| ) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt
around lines 24 to 33, the OptionChipButton components always have isFilled set
to true, ignoring the selectedStates parameter, so the UI does not reflect the
filter selection state. Update the isFilled property of each OptionChipButton to
use the corresponding value from selectedStates to visually indicate whether
each filter is selected or not.
|
|
||
| @Composable | ||
| fun AlarmScreen( | ||
| alarmItems: List<AlarmItem>, onCardClick: (AlarmItem) -> Unit = {} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
사용되지 않는 매개변수 처리
onCardClick 매개변수가 제공되지만 실제로는 사용되지 않고 있습니다. 카드 클릭 시 내부 로직만 실행됩니다.
onClick = {
+ onCardClick(alarm)
alarms = alarms.map {
if (it.id == alarm.id) it.copy(isRead = true) else it
}
}또는 이 매개변수가 필요하지 않다면 제거하는 것을 고려해보세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt
at line 37, the parameter onCardClick is declared but not used anywhere in the
function. Either remove the onCardClick parameter entirely if it is unnecessary,
or update the code to invoke onCardClick when a card is clicked to ensure the
parameter is utilized properly.
| fun AlarmScreen( | ||
| alarmItems: List<AlarmItem>, onCardClick: (AlarmItem) -> Unit = {} | ||
| ) { | ||
| var selectedStates by remember { mutableStateOf(booleanArrayOf(false, false)) } | ||
| var alarms by remember { mutableStateOf(alarmItems) } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 관리 개선 필요
remember를 사용한 상태 관리는 화면 회전 등 설정 변경 시 상태가 초기화됩니다. 알림 읽음 상태를 보존하기 위해 rememberSaveable이나 ViewModel 사용을 고려해보세요.
- var alarms by remember { mutableStateOf(alarmItems) }
+ var alarms by rememberSaveable { mutableStateOf(alarmItems) }또는 ViewModel 패턴을 사용하여 상태를 관리하는 것이 더 나은 접근 방법입니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt
around lines 36 to 40, the current use of remember for state management causes
state loss on configuration changes like screen rotation. Replace remember with
rememberSaveable to preserve state across such changes, or refactor the state
management to use a ViewModel to hold and manage the alarm read states
persistently.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt
Outdated
Show resolved
Hide resolved
| selectedStates[0] && selectedStates[1] -> alarms | ||
| else -> alarms |
|
|
||
| Column( | ||
| Modifier | ||
| .background(colors.Black) |
There was a problem hiding this comment.
이거 화면 배경색 black인거 제가 theme으로 적용해놔서 따로 설정 안해도 됩니다 ! 프리뷰에서도 그렇게 보고싶으면 ThipTheme으로 감싸주면 됩니다 ~
| ) | ||
| Column( | ||
| Modifier | ||
| .background(colors.Black) |
| Column( | ||
| modifier = Modifier.fillMaxSize(), | ||
| verticalArrangement = Arrangement.Center, | ||
| horizontalAlignment = Alignment.CenterHorizontally | ||
| ) { | ||
| Column( | ||
| modifier = Modifier.fillMaxSize(), | ||
| verticalArrangement = Arrangement.Center, | ||
| horizontalAlignment = Alignment.CenterHorizontally | ||
| ) { |
There was a problem hiding this comment.
어 머야 잘못 된거 같네요 수정하겠습니다! 코드 래빗 리뷰는 탑앱바 얘긴줄 알고 무시했는데 ㅋㅋㅋ
| Spacer(modifier = Modifier.width(12.dp)) | ||
| Spacer(modifier = Modifier.width(8.dp)) | ||
|
|
||
| // 내용 (제목, 빨간 점, 시간, 메시지) |
There was a problem hiding this comment.
스크린샷에서 빨간점의 위치가 피그마랑 살짝 다른거같은데 .. 제 눈이 이상한걸수도 있고 .. ㅎ.ㅎ... 작아서 잘 모르겠긴 한데 .. 아무튼 그렇습니다 수정할 수 있으면 수정해주세욧
There was a problem hiding this comment.
이전에 했던거라 생각 못했는데 확인하고 수정하겠습니다!
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (3)
37-37: 사용되지 않는 매개변수 처리 필요
onCardClick매개변수가 함수 시그니처에 정의되어 있지만 실제로는 사용되지 않고 있습니다. 카드 클릭 시에는 내부 상태만 업데이트하고 있습니다.
39-40: 상태 관리 개선 필요
remember를 사용한 상태 관리는 화면 회전 등 설정 변경 시 상태가 초기화됩니다. 알림 읽음 상태를 보존하기 위해rememberSaveable이나 ViewModel 사용을 고려해보세요.
100-104: 콜백 함수 호출 누락카드 클릭 시 내부 상태만 업데이트하고 있지만, 매개변수로 전달받은
onCardClick콜백을 호출하지 않고 있습니다. 외부에서 클릭 이벤트를 처리할 수 있도록 콜백을 호출해야 합니다.onClick = { + onCardClick(alarm) alarms = alarms.map { if (it.id == alarm.id) it.copy(isRead = true) else it } }
🧹 Nitpick comments (2)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
42-46: 필터링 로직 개선 가능현재 필터링 로직에서
stringResource를 반복 호출하고 있습니다. 성능 최적화를 위해 리소스 값을 미리 가져와서 사용하는 것이 좋습니다.+ val feedText = stringResource(R.string.alarm_feed) + val groupText = stringResource(R.string.alarm_group) + val filteredList = when { - selectedStates[0] && !selectedStates[1] -> alarms.filter { it.badgeText == stringResource(R.string.alarm_feed) } - !selectedStates[0] && selectedStates[1] -> alarms.filter { it.badgeText == stringResource(R.string.alarm_group) } + selectedStates[0] && !selectedStates[1] -> alarms.filter { it.badgeText == feedText } + !selectedStates[0] && selectedStates[1] -> alarms.filter { it.badgeText == groupText } else -> alarms }
118-123: 하드코딩된 문자열 사용프리뷰에서 하드코딩된 문자열을 사용하고 있습니다. 유지보수성과 다국어 지원을 위해 string 리소스를 사용하는 것이 좋습니다.
실제 앱에서는 string 리소스를 사용하고 있으므로, 프리뷰에서도 일관성을 위해 string 리소스 사용을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/cards/CardAlarm.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/theme/Color.kt(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/texthip/thip/ui/theme/Color.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/texthip/thip/ui/common/cards/CardAlarm.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (1)
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (1)
113-136: 프리뷰 함수들이 잘 구성됨두 가지 상태(알림 있음/없음)에 대한 프리뷰를 제공하여 다양한 시나리오를 확인할 수 있도록 구성했습니다.
ThipTheme으로 적절히 래핑되어 있어 테마가 올바르게 적용됩니다.
2 participants
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
알림 페이지 구현
알림이 없을 때
알림이 있을 때
😢 해결하지 못한 과제
[] TASK
📢 리뷰어들에게
Summary by CodeRabbit
신규 기능
스타일
문서화
기타