Repository navigation
[feature/enhance-ui] UI improvements to home and alarm screen - #4
Conversation
This reverts commit eb139dc.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe alarm store now tracks and dismisses snoozed alarms with generation checks. The home screen provides a confirmation flow. The alarm screen shows live time, and alarm cards show task-specific icons and accessibility text. Scheduling tests cover exact-time and boundary behavior. ChangesSnoozed alarm flow and presentation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant HomeScreen
participant alarmStore
participant alarmRegistrationStore
participant alarmScheduler
HomeScreen->>alarmStore: subscribe to snoozedCount
HomeScreen->>alarmStore: confirm and dismissAllSnoozedAlarms
alarmStore->>alarmScheduler: cancel snooze notifications
alarmStore->>alarmRegistrationStore: remove snooze registrations
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/AlarmCard.tsx`:
- Around line 46-53: Update the Pressable accessibilityLabel in AlarmCard to
include alarm.task alongside the existing time and days information, ensuring
assistive technology announces the task represented by the icon.
In `@src/store/alarmStore.ts`:
- Around line 153-165: Update the bulk-dismiss flow around snoozedIds and its
Promise.allSettled callbacks to capture each snooze registration’s generation
token, then cancel and remove only when that token still matches the current
registration for the alarm. Preserve the required AlarmSchedulerModule
identifier scheme and ensure a newer snooze is never cancelled or deregistered
by an older dismissal.
- Around line 168-178: Update the dismissal action containing the failed count
calculation so it propagates partial failures to its caller instead of resolving
normally. When failed is greater than zero, reject after retaining the existing
failure details; preserve the successful return value when all snoozed alarms
are dismissed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f39ed250-0148-4772-92db-ab5add942767
📒 Files selected for processing (4)
src/app/(alarm)/alarm.tsxsrc/app/(main)/index.tsxsrc/components/AlarmCard.tsxsrc/store/alarmStore.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/time.ts (1)
63-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd boundary tests for
computeNextAlarminsrc/utils/time.ts. Cover candidates equal tonow, one minute afternow, and one minute beforenow. The equality and past cases must roll to the next week. Runnpm run lintandnpx tsc --noEmit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/time.ts` around lines 63 - 65, Add boundary tests for computeNextAlarm covering candidates equal to now, one minute after now, and one minute before now; assert equality and past candidates roll forward by one week while the future candidate does not. Run npm run lint and npx tsc --noEmit to verify the changes.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/store/alarmRegistrationsStore.ts`:
- Around line 96-118: Update the upsert flow to allocate generation atomically
in SQLite: replace the pre-write existing-record calculation with an insert
conflict update that increments the stored generation and returns the resulting
row/generation. Use the returned generation when updating Zustand records,
including calls concurrent with fetchRecords(), and preserve the existing
alarmId/type uniqueness target.
---
Nitpick comments:
In `@src/utils/time.ts`:
- Around line 63-65: Add boundary tests for computeNextAlarm covering candidates
equal to now, one minute after now, and one minute before now; assert equality
and past candidates roll forward by one week while the future candidate does
not. Run npm run lint and npx tsc --noEmit to verify the changes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd3e5b22-b350-4fe9-9a62-a926aed8d969
📒 Files selected for processing (9)
drizzle/0004_striped_exiles.sqldrizzle/meta/0004_snapshot.jsondrizzle/meta/_journal.jsondrizzle/migrations.jssrc/components/AlarmCard.tsxsrc/data/schema.tssrc/store/alarmRegistrationsStore.tssrc/store/alarmStore.tssrc/utils/time.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/store/alarmStore.ts
Replace pre-read of generation from Zustand state with SQL-level `generation + 1` in onConflictDoUpdate, returning the result row to eliminate a race window where concurrent upserts could compute the same generation number from stale in-memory state.
Cover candidate == now (rolls forward 7 days), candidate one minute after now (keeps same day), and candidate one minute before now (rolls forward 6d 23h 59m).
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary by CodeRabbit
New Features
Bug Fixes