[REFACTOR/#276] 일기작성 뒤로가기 수정 및 삭제 바텀시트 UI 수정을 진행합니다.#277
Conversation
WalkthroughThis change updates the diary writing feature by refining the logic for detecting changes in diary entries, adjusting the back navigation event sequence, and removing a fixed-height spacer from the delete confirmation bottom sheet UI. No public API or exported entity signatures were altered. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WriteDiaryScreen
participant WriteDiaryViewModel
participant Navigator
User->>WriteDiaryScreen: Presses Back
WriteDiaryScreen->>WriteDiaryViewModel: hasChangedFromInitial()
WriteDiaryViewModel-->>WriteDiaryScreen: Returns change status
WriteDiaryScreen->>WriteDiaryScreen: Track back event
alt No changes detected
WriteDiaryScreen->>Navigator: Navigate back
else Changes detected
WriteDiaryScreen->>WriteDiaryScreen: Show exit confirmation dialog
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/sopt/clody/presentation/ui/writediary/component/bottomsheet/DeleteWriteDiaryBottomSheet.kt(0 hunks)app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryScreen.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryViewModel.kt(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/sopt/clody/presentation/ui/writediary/component/bottomsheet/DeleteWriteDiaryBottomSheet.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryViewModel.kt (1)
181-186: Improved change detection logic with proper whitespace handling.The enhanced
hasChangedFromInitial()function correctly addresses the PR requirements:
- Early return for empty initial entries prevents false positives
- Trimming both current and initial entries ensures whitespace-only changes are treated as unchanged
- The comparison logic is more robust and user-friendly
| onClickBack = { | ||
| if (viewModel.hasChangedFromInitial()) { | ||
| AmplitudeUtils.trackEvent(AmplitudeConstraints.WRITING_DIARY_BACK) | ||
| viewModel.updateShowExitDialog(true) | ||
| } else { | ||
| AmplitudeUtils.trackEvent(AmplitudeConstraints.WRITING_DIARY_BACK) | ||
| if (!viewModel.hasChangedFromInitial()) { | ||
| navigateToPrevious() | ||
| } else { | ||
| viewModel.updateShowExitDialog(true) | ||
| } | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify consistency in event tracking between BackHandler and onClickBack.
The logic correctly implements the requirement to always track the back event and navigate immediately when no changes exist. However, there's an inconsistency with the BackHandler (lines 96-98) which only tracks the event when changes are detected, while this callback always tracks it.
Consider whether both handlers should have consistent event tracking behavior:
🏁 Script executed:
#!/bin/bash
# Search for other usages of WRITING_DIARY_BACK event to understand the intended tracking pattern
rg -A 3 -B 3 "WRITING_DIARY_BACK"Length of output: 2808
Align BackHandler’s event tracking with onClickBack
The system back handler currently only logs the back event when changes exist, whereas the UI back button always logs it. To ensure consistent analytics, move the trackEvent call outside the change check in the BackHandler.
• File: app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryScreen.kt
– BackHandler block (around lines 92–103)
Suggested diff:
BackHandler {
- if (viewModel.hasChangedFromInitial()) {
- AmplitudeUtils.trackEvent(AmplitudeConstraints.WRITING_DIARY_BACK)
- viewModel.updateShowExitDialog(true)
- } else {
- navigateToPrevious()
- }
+ AmplitudeUtils.trackEvent(AmplitudeConstraints.WRITING_DIARY_BACK)
+ if (viewModel.hasChangedFromInitial()) {
+ viewModel.updateShowExitDialog(true)
+ } else {
+ navigateToPrevious()
+ }
}🤖 Prompt for AI Agents
In
app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryScreen.kt
around lines 92 to 103, the BackHandler currently tracks the WRITING_DIARY_BACK
event only when changes exist, unlike the onClickBack handler which always
tracks it. To fix this inconsistency, move the
AmplitudeUtils.trackEvent(AmplitudeConstraints.WRITING_DIARY_BACK) call outside
and before the conditional check in the BackHandler so that the event is always
tracked regardless of changes.
📌 ISSUE
closed #276
📄 Work Description
✨ PR Point
술먹으러 가야지~
📸 ScreenShot/Video
Summary by CodeRabbit
Bug Fixes
Refactor