Add options for 5 days, 7 days, and work week #893
Open
CampAsAChamp wants to merge 1 commit intoleits:masterfrom
Open
Add options for 5 days, 7 days, and work week #893CampAsAChamp wants to merge 1 commit intoleits:masterfrom
CampAsAChamp wants to merge 1 commit intoleits:masterfrom
Conversation
- EventManager and MBEvent+Helpers updates - StatusBarItemController enhancements - Appearance preferences and localization - Constants and NextEventTests updates
dosubot
Bot
added
the
size:L
WalkthroughThis pull request introduces three new time period options for event display filtering: 5 days, 7 days, and work week (Monday–Friday). The implementation extends the ShowEventsForPeriod enum and adds corresponding date-range calculation logic across event management and UI components, with localization strings and test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| XCTAssertEqual(array.nextEvent(), running) | ||
| } | ||
|
|
||
| func test_fiveDaysPeriod_includesEventWithinFiveDays_excludesEventAfter() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
| XCTAssertEqual(array.nextEvent(), withinFive) | ||
| } | ||
|
|
||
| func test_sevenDaysPeriod_includesEventWithinSevenDays() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MeetingBar/UI/StatusBar/StatusBarItemController.swift`:
- Around line 324-349: daysInPeriod currently returns only today for the
.today_n_tomorrow case; update the .today_n_tomorrow branch in the
daysInPeriod(_:,from:,calendar:) function so it returns an array containing
today and tomorrow by using calendar.date(byAdding: .day, value: 1, to: today)
to compute tomorrow (preserve ordering [today, tomorrow] and safe unwrap similar
to other cases).
- Around line 398-417: The section title for multi-day periods uses a full date
string but buildDateSection(date: title: events:) already appends the formatted
date, causing duplicated labels; change the else branch that sets sectionTitle
(using dateFormatter) to produce only the weekday (e.g., use a DateFormatter
with format "E" or fetch the localized weekday name) so sectionTitle is just the
weekday name, keeping the existing special-cases for today/tomorrow and leaving
builder.buildDateSection as-is.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
MeetingBar/Core/Managers/EventManager.swiftMeetingBar/Core/Models/MBEvent+Helpers.swiftMeetingBar/Resources /Localization /en.lproj/Localizable.stringsMeetingBar/UI/StatusBar/StatusBarItemController.swiftMeetingBar/UI/Views/Preferences/AppearanceTab.swiftMeetingBar/Utilities/Constants.swiftMeetingBarTests/NextEventTests.swift
Comment on lines
+324
to
+349
| /// Returns the start-of-day dates for each day in the given period, from today (or next Monday for work week on weekend). | ||
| private static func daysInPeriod(_ period: ShowEventsForPeriod, from today: Date, calendar: Calendar) -> [Date] { | ||
| switch period { | ||
| case .fiveDays: | ||
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: today)! } | ||
| case .sevenDays: | ||
| return (0 ..< 7).map { calendar.date(byAdding: .day, value: $0, to: today)! } | ||
| case .workWeek: | ||
| let weekday = calendar.component(.weekday, from: today) // 1 = Sunday, 7 = Saturday | ||
| if weekday == 1 { | ||
| // Sunday: next week Mon–Fri | ||
| let nextMonday = calendar.date(byAdding: .day, value: 1, to: today)! | ||
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: nextMonday)! } | ||
| } else if weekday == 7 { | ||
| // Saturday: next week Mon–Fri | ||
| let nextMonday = calendar.date(byAdding: .day, value: 2, to: today)! | ||
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: nextMonday)! } | ||
| } else { | ||
| // Monday (2) through Friday (6): today through Friday | ||
| let daysUntilFriday = 6 - weekday | ||
| return (0 ... daysUntilFriday).map { calendar.date(byAdding: .day, value: $0, to: today)! } | ||
| } | ||
| case .today, .today_n_tomorrow: | ||
| return [today] | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix .today_n_tomorrow to include tomorrow in daysInPeriod.
The helper’s switch returns only today for .today_n_tomorrow, which is incorrect if the helper is reused for that period.
✅ Suggested fix
- case .today, .today_n_tomorrow:
- return [today]
+ case .today:
+ return [today]
+ case .today_n_tomorrow:
+ return [today, calendar.date(byAdding: .day, value: 1, to: today)!]📝 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.
Suggested change
| /// Returns the start-of-day dates for each day in the given period, from today (or next Monday for work week on weekend). | |
| private static func daysInPeriod(_ period: ShowEventsForPeriod, from today: Date, calendar: Calendar) -> [Date] { | |
| switch period { | |
| case .fiveDays: | |
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: today)! } | |
| case .sevenDays: | |
| return (0 ..< 7).map { calendar.date(byAdding: .day, value: $0, to: today)! } | |
| case .workWeek: | |
| let weekday = calendar.component(.weekday, from: today) // 1 = Sunday, 7 = Saturday | |
| if weekday == 1 { | |
| // Sunday: next week Mon–Fri | |
| let nextMonday = calendar.date(byAdding: .day, value: 1, to: today)! | |
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: nextMonday)! } | |
| } else if weekday == 7 { | |
| // Saturday: next week Mon–Fri | |
| let nextMonday = calendar.date(byAdding: .day, value: 2, to: today)! | |
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: nextMonday)! } | |
| } else { | |
| // Monday (2) through Friday (6): today through Friday | |
| let daysUntilFriday = 6 - weekday | |
| return (0 ... daysUntilFriday).map { calendar.date(byAdding: .day, value: $0, to: today)! } | |
| } | |
| case .today, .today_n_tomorrow: | |
| return [today] | |
| } | |
| } | |
| /// Returns the start-of-day dates for each day in the given period, from today (or next Monday for work week on weekend). | |
| private static func daysInPeriod(_ period: ShowEventsForPeriod, from today: Date, calendar: Calendar) -> [Date] { | |
| switch period { | |
| case .fiveDays: | |
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: today)! } | |
| case .sevenDays: | |
| return (0 ..< 7).map { calendar.date(byAdding: .day, value: $0, to: today)! } | |
| case .workWeek: | |
| let weekday = calendar.component(.weekday, from: today) // 1 = Sunday, 7 = Saturday | |
| if weekday == 1 { | |
| // Sunday: next week Mon–Fri | |
| let nextMonday = calendar.date(byAdding: .day, value: 1, to: today)! | |
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: nextMonday)! } | |
| } else if weekday == 7 { | |
| // Saturday: next week Mon–Fri | |
| let nextMonday = calendar.date(byAdding: .day, value: 2, to: today)! | |
| return (0 ..< 5).map { calendar.date(byAdding: .day, value: $0, to: nextMonday)! } | |
| } else { | |
| // Monday (2) through Friday (6): today through Friday | |
| let daysUntilFriday = 6 - weekday | |
| return (0 ... daysUntilFriday).map { calendar.date(byAdding: .day, value: $0, to: today)! } | |
| } | |
| case .today: | |
| return [today] | |
| case .today_n_tomorrow: | |
| return [today, calendar.date(byAdding: .day, value: 1, to: today)!] | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBar/UI/StatusBar/StatusBarItemController.swift` around lines 324 -
349, daysInPeriod currently returns only today for the .today_n_tomorrow case;
update the .today_n_tomorrow branch in the daysInPeriod(_:,from:,calendar:)
function so it returns an array containing today and tomorrow by using
calendar.date(byAdding: .day, value: 1, to: today) to compute tomorrow (preserve
ordering [today, tomorrow] and safe unwrap similar to other cases).
Comment on lines
+398
to
+417
| case .fiveDays, .sevenDays, .workWeek: | ||
| let calendar = Calendar.current | ||
| let dayDates = Self.daysInPeriod(Defaults[.showEventsForPeriod], from: today, calendar: calendar) | ||
| let dateFormatter = DateFormatter() | ||
| dateFormatter.dateFormat = "E, d MMM" | ||
| dateFormatter.locale = I18N.instance.locale | ||
| for (index, dayStart) in dayDates.enumerated() { | ||
| if index > 0 { | ||
| statusItemMenu.addItem(NSMenuItem.separator()) | ||
| } | ||
| let dayEvents = events.filter { calendar.isDate($0.startDate, inSameDayAs: dayStart) } | ||
| let sectionTitle: String | ||
| if calendar.isDate(dayStart, inSameDayAs: today) { | ||
| sectionTitle = "status_bar_section_today".loco() | ||
| } else if calendar.isDate(dayStart, inSameDayAs: tomorrow) { | ||
| sectionTitle = "status_bar_section_tomorrow".loco() | ||
| } else { | ||
| sectionTitle = dateFormatter.string(from: dayStart) | ||
| } | ||
| statusItemMenu.items += builder.buildDateSection(date: dayStart, title: sectionTitle, events: dayEvents) |
There was a problem hiding this comment.
Avoid duplicated date labels in multi‑day sections.
buildDateSection already appends the formatted date, so using a full date as sectionTitle yields headers like “Tue, 25 Feb (Tue, 25 Feb):”. Consider passing a weekday-only title instead.
✅ Suggested fix
- let dateFormatter = DateFormatter()
- dateFormatter.dateFormat = "E, d MMM"
- dateFormatter.locale = I18N.instance.locale
+ let weekdayFormatter = DateFormatter()
+ weekdayFormatter.dateFormat = "EEEE"
+ weekdayFormatter.locale = I18N.instance.locale
for (index, dayStart) in dayDates.enumerated() {
@@
} else if calendar.isDate(dayStart, inSameDayAs: tomorrow) {
sectionTitle = "status_bar_section_tomorrow".loco()
} else {
- sectionTitle = dateFormatter.string(from: dayStart)
+ sectionTitle = weekdayFormatter.string(from: dayStart)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBar/UI/StatusBar/StatusBarItemController.swift` around lines 398 -
417, The section title for multi-day periods uses a full date string but
buildDateSection(date: title: events:) already appends the formatted date,
causing duplicated labels; change the else branch that sets sectionTitle (using
dateFormatter) to produce only the weekday (e.g., use a DateFormatter with
format "E" or fetch the localized weekday name) so sectionTitle is just the
weekday name, keeping the existing special-cases for today/tomorrow and leaving
builder.buildDateSection as-is.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #893 +/- ##
===========================================
+ Coverage 26.30% 36.40% +10.10%
===========================================
Files 36 48 +12
Lines 5645 5216 -429
Branches 2210 1771 -439
===========================================
+ Hits 1485 1899 +414
+ Misses 4103 3260 -843
Partials 57 57 ☔ View full report in Codecov by Sentry. |
CampAsAChamp
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Event and status bar UI improvements.
Changes
Summary by CodeRabbit