feat: upcoming event page time filter pill#252
Conversation
williscool
changed the title
Code Coverage Report
Files
|
📊 Code Coverage Summary
|
also tweak dialog a bit
android/app/src/main/java/com/github/quarck/calnotify/prefs/NavigationSettingsFragmentX.kt
Show resolved
Hide resolved
android/app/src/main/java/com/github/quarck/calnotify/ui/UpcomingTimeFilterBottomSheet.kt
Outdated
Show resolved
Hide resolved
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
android/app/src/main/java/com/github/quarck/calnotify/ui/UpcomingTimeFilterBottomSheet.kt
Show resolved
Hide resolved
📊 Code Coverage Summary
|
android/app/src/main/java/com/github/quarck/calnotify/ui/UpcomingTimeFilterBottomSheet.kt
Outdated
Show resolved
Hide resolved
📊 Code Coverage Summary
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| id = DAY_BOUNDARY_RADIO_ID | ||
| val hourStr = DateTimeUtils.formatHourOfDay(settings.upcomingEventsDayBoundaryHour) | ||
| text = getString(R.string.upcoming_time_day_boundary, hourStr) | ||
| setPadding(0, 24, 0, 24) |
There was a problem hiding this comment.
Hardcoded pixel values cause density-dependent sizing
Medium Severity
setPadding(0, 24, 0, 24) on the RadioButtons and the divider height of 2 use raw pixel values instead of density-independent pixels. On xxxhdpi screens (4x), 24px is only ~6dp (very cramped), while on mdpi screens (1x) it's a generous 24dp. The rest of the codebase correctly uses resources.getDimensionPixelSize(R.dimen....) for programmatic dimensions (e.g., PreActionActivity.kt). These values need dp-to-px conversion or dimen resources to render consistently across devices.
Additional Locations (2)
| return if (minutes == 1L) "1 minute" else "$minutes minutes" | ||
| } | ||
| return "$seconds seconds" | ||
| } |
There was a problem hiding this comment.
Human-readable time strings are hardcoded in English
Low Severity
formatPresetHumanReadable returns hardcoded English strings like "hours", "days", "weeks" that appear in user-facing chip text and the bottom sheet options. Unlike other user-facing text in the app which uses Android string resources (getString(R.string.…)), this function isn't localizable. Since it's a PreferenceUtils utility without Context, it can't access resources — the callers in MainActivityModern and UpcomingTimeFilterBottomSheet (which do have Context) would need to handle formatting instead, or the function signature would need to accept a Context parameter.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Preset preference class duplicates existing SnoozePresetPreferenceX
Low Severity
UpcomingTimePresetPreferenceX is a near-duplicate of the existing SnoozePresetPreferenceX. Both extend DialogPreference, have an inner Dialog class extending PreferenceDialogFragmentCompat, bind an EditText, parse via PreferenceUtils.parseSnoozePresets(), reformat on save, persist via persistString, and show error alerts. The differences (label TextView, negative-value filtering, max-count threshold) are minor enough to share a base class or parameterized implementation.
| val presets = PreferenceUtils.parseSnoozePresets(value) | ||
| if (presets != null) { | ||
| // Filter out negative values | ||
| val validPresets = presets.filter { it > 0 } |
There was a problem hiding this comment.
Dialog validates positivity but not max lookahead constraint
Medium Severity
The dialog's validPresets filter only checks it > 0, but Settings.upcomingTimePresets filters with it > 0 && it <= MAX_LOOKAHEAD_MILLIS. If a user enters presets that are all positive but exceed 30 days (e.g., "5w, 6w"), the dialog accepts and persists them (non-empty positive list), but the read path silently drops all of them. This leaves the UpcomingTimeFilterBottomSheet with zero fixed interval options and no way to select a fixed mode — despite the dialog telling the user the save succeeded.
Additional Locations (1)
📊 Code Coverage Summary
|
1 participant
fixes: #216
Note
Medium Risk
Touches persistent settings and upcoming lookahead calculation that controls which events are queried, so mis-parsing/clamping or UI wiring bugs could change the visible upcoming window; changes are localized and covered by new tests.
Overview
Adds an Upcoming tab “time” chip that opens a new
UpcomingTimeFilterBottomSheetto switch between Day boundary mode and fixed lookahead presets; selection is persisted toSettings(notFilterState) and triggers an upcoming list refresh.Extends settings to support millisecond-based fixed lookahead (
upcomingEventsFixedLookaheadMillis, clamped to a 30-day scan window with legacy fallback fromupcomingEventsFixedHours) and configurable preset lists stored as a comma-separated string; updates Preferences UI to edit these presets via a newUpcomingTimePresetPreferenceXdialog.Enhances duration parsing/formatting to support weeks (
w) and adds human-readable preset labels; includes new layouts/strings and expands unit tests around presets, clamping, and lookahead calculation.Written by Cursor Bugbot for commit ac0e865. This will update automatically on new commits. Configure here.