Feat: Next event flip preview in status bar#905
Open
jclawson-atlassian wants to merge 1 commit intoleits:masterfrom
Open
Feat: Next event flip preview in status bar#905jclawson-atlassian wants to merge 1 commit intoleits:masterfrom
jclawson-atlassian wants to merge 1 commit intoleits:masterfrom
Conversation
WalkthroughThis pull request introduces a "flip" feature for the status bar that alternates between displaying the current event and an upcoming event within a time threshold. It adds configuration options, helper logic to identify current and upcoming events, UI updates, and comprehensive test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
📝 Coding Plan
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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
dosubot
Bot
added
the
size:XL
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| button.toolTip = event.title | ||
| } | ||
|
|
||
| private func renderNormalTitle() { |
Check warning
Code scanning / Swiftlint (reported by Codacy)
Function should have complexity 15 or less: currently complexity equals 18 Warning
| func test_skipsAllDayCurrentEvent() { | ||
| let allDay = makeFakeEvent( | ||
| id: "AD1", | ||
| start: now.addingTimeInterval(-43200), |
Check warning
Code scanning / Swiftlint (reported by Codacy)
Underscores should be used as thousand separator in large decimal numbers. Warning
| let allDay = makeFakeEvent( | ||
| id: "AD1", | ||
| start: now.addingTimeInterval(-43200), | ||
| end: now.addingTimeInterval(43200), |
Check warning
Code scanning / Swiftlint (reported by Codacy)
Underscores should be used as thousand separator in large decimal numbers. Warning
There was a problem hiding this comment.
Tailor (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #905 +/- ##
===========================================
+ Coverage 26.30% 38.71% +12.40%
===========================================
Files 36 49 +13
Lines 5645 5440 -205
Branches 2210 1780 -430
===========================================
+ Hits 1485 2106 +621
+ Misses 4103 3278 -825
+ Partials 57 56 -1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
MeetingBar/UI/StatusBar/StatusBarItemController.swift (2)
219-233: Redundant MainActor.run inside@MainActorclass.Since
StatusBarItemControlleris already marked@MainActor, theMainActor.runcall is unnecessary. The class guarantees all instance methods run on the main actor.♻️ Simplified flip timer
flipTask = Task { [weak self] in while let self, !Task.isCancelled { let interval = TimeInterval(Defaults[.nextEventFlipIntervalSeconds]) try? await Task.sleep(nanoseconds: UInt64(interval * Double(NSEC_PER_SEC))) guard !Task.isCancelled else { break } - await MainActor.run { - self.showingUpcomingEvent.toggle() - self.updateTitle() - } + self.showingUpcomingEvent.toggle() + self.updateTitle() } }🤖 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 219 - 233, The MainActor.run call inside startFlipTimer is redundant because StatusBarItemController is `@MainActor`; remove the await MainActor.run { ... } wrapper and perform self.showingUpcomingEvent.toggle() and self.updateTitle() directly inside the Task loop (keeping the existing [weak self] capture, flipTask handling, and Task cancellation checks). Update startFlipTimer so the Task closure toggles showingUpcomingEvent and calls updateTitle without dispatching to MainActor.run.
254-270: Consider extracting icon handling to a helper.The icon-setting logic (lines 254-270) is duplicated almost identically in
renderEvent()(lines 311-326). Extracting this to a shared helper would reduce duplication and make future changes easier.♻️ Extract helper method
private func configureIcon(for event: MBEvent, on button: NSStatusBarButton) { if Defaults[.eventTitleIconFormat] != .none { let image: NSImage if Defaults[.eventTitleIconFormat] == .eventtype { image = getIconForMeetingService(event.meetingLink?.service) } else { image = NSImage(named: Defaults[.eventTitleIconFormat].rawValue)! } button.image = image button.image?.size = MenuStyleConstants.iconSize } if button.image?.name() == "no_online_session" { button.imagePosition = .noImage } else { button.imagePosition = .imageLeft } }🤖 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 254 - 270, The icon-setting block in StatusBarItemController (used in renderNextUpcomingItem() and renderEvent()) should be extracted into a single helper to remove duplication; add a private method like configureIcon(for event: MBEvent?, on button: NSStatusBarButton) that encapsulates the Defaults[.eventTitleIconFormat] check, uses getIconForMeetingService(event?.meetingLink?.service) when format == .eventtype or NSImage(named: Defaults[.eventTitleIconFormat].rawValue) otherwise, sets button.image and button.image?.size = MenuStyleConstants.iconSize, then sets button.imagePosition to .noImage if button.image?.name() == "no_online_session" else .imageLeft, and replace the duplicated blocks in renderNextUpcomingItem()/renderEvent() with calls to this new helper.MeetingBar/Core/Models/MBEvent+Helpers.swift (1)
87-118: Consider array ordering assumptions.The method uses
first(where:)to find both current and upcoming events. If the events array is not sorted by start date, this may return an arbitrary matching event rather than the earliest one.If the array is expected to be pre-sorted (as other methods like
nextEvent()seem to assume with their iteration logic), consider adding a doc comment noting this precondition. Otherwise, consider sorting or usingmin(by:)to ensure deterministic selection.♻️ Optional: Use min(by:) for deterministic selection
- let current = first(where: { + let current = filter({ !$0.isAllDay && $0.startDate <= now && $0.endDate > now && $0.participationStatus != .declined && $0.status != .canceled }) + .min(by: { $0.startDate < $1.startDate })Apply similar change for
upcomingif needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MeetingBar/Core/Models/MBEvent`+Helpers.swift around lines 87 - 118, The currentAndUpcomingEvent method uses first(where:) which depends on array order and can return arbitrary matches; make selection deterministic by explicitly choosing the correct event from matching candidates: for the ongoing event (current) filter events matching the ongoing criteria and pick the one with the latest startDate (use max(by: { $0.startDate < $1.startDate }) or sorted last), and for the upcoming event filter candidates that start after now and within gapThreshold and pick the earliest start (use min(by: { $0.startDate < $1.startDate }) or sorted first) using the same predicates currently in the closures; alternatively, if the collection is guaranteed pre-sorted, add a doc comment on currentAndUpcomingEvent (and similar methods like nextEvent()) stating that precondition instead of changing logic.
🤖 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 278-294: The hardcoded UI strings ("Next: " and the tooltip "Now:
...\nNext: ...") must be localized: replace the literal "Next: " used when
appending to menuTitle with a localized string (e.g. NSLocalizedString key like
"StatusBar.NextPrefix") and build eventText as before; and replace the literal
tooltip construction with a localized format string (e.g. NSLocalizedString key
like "StatusBar.Tooltip.NowNext" = "Now: %@\nNext: %@") and call
String(format:localizedTooltip, current.title, upcoming.title). Update usages
around menuTitle, button.attributedTitle and button.toolTip (and keep
Default[.eventTimeFormat] logic and attributes) so all user-facing text is
loaded via NSLocalizedString keys with appropriate placeholders for
upcomingTitle/upcomingTime/current.title/upcoming.title.
In `@MeetingBar/UI/Views/Preferences/AppearanceTab.swift`:
- Around line 251-269: The new picker labels and help strings in the UI (the
Picker with label "Next event preview:", the Text tags "Disabled", "Show after
start", "Show after 10 min", the conditional Picker label "Flip every" and their
.help(...) calls) are hardcoded and must be localized like the rest of the file;
replace those raw strings with the localized equivalents using .loco() (or the
existing localization helper) when creating the Picker label and each Text(...)
tag, and wrap the help messages with .loco() as well, keeping the bindings
(nextEventFlipMode, NextEventFlipMode.*, nextEventFlipIntervalSeconds,
ongoingEventVisibility) and conditional logic unchanged.
In `@MeetingBarTests/CurrentAndUpcomingEventTests.swift`:
- Around line 239-246: The helper shouldFlip(mode: NextEventFlipMode,
currentStart: Date) must mirror production by also checking
Defaults[.ongoingEventVisibility]; update its signature to accept an
ongoingEventVisibility parameter (or read Defaults inside the helper) and return
false unless ongoingEventVisibility == .showTenMinBeforeNext in addition to mode
!= .disabled and the time check; update all tests that call shouldFlip and any
uses in CurrentAndUpcomingEventTests to pass/verify different
ongoingEventVisibility values and add tests covering visibility values other
than .showTenMinBeforeNext to ensure behavior matches
StatusBarItemController.updateTitle.
---
Nitpick comments:
In `@MeetingBar/Core/Models/MBEvent`+Helpers.swift:
- Around line 87-118: The currentAndUpcomingEvent method uses first(where:)
which depends on array order and can return arbitrary matches; make selection
deterministic by explicitly choosing the correct event from matching candidates:
for the ongoing event (current) filter events matching the ongoing criteria and
pick the one with the latest startDate (use max(by: { $0.startDate <
$1.startDate }) or sorted last), and for the upcoming event filter candidates
that start after now and within gapThreshold and pick the earliest start (use
min(by: { $0.startDate < $1.startDate }) or sorted first) using the same
predicates currently in the closures; alternatively, if the collection is
guaranteed pre-sorted, add a doc comment on currentAndUpcomingEvent (and similar
methods like nextEvent()) stating that precondition instead of changing logic.
In `@MeetingBar/UI/StatusBar/StatusBarItemController.swift`:
- Around line 219-233: The MainActor.run call inside startFlipTimer is redundant
because StatusBarItemController is `@MainActor`; remove the await MainActor.run {
... } wrapper and perform self.showingUpcomingEvent.toggle() and
self.updateTitle() directly inside the Task loop (keeping the existing [weak
self] capture, flipTask handling, and Task cancellation checks). Update
startFlipTimer so the Task closure toggles showingUpcomingEvent and calls
updateTitle without dispatching to MainActor.run.
- Around line 254-270: The icon-setting block in StatusBarItemController (used
in renderNextUpcomingItem() and renderEvent()) should be extracted into a single
helper to remove duplication; add a private method like configureIcon(for event:
MBEvent?, on button: NSStatusBarButton) that encapsulates the
Defaults[.eventTitleIconFormat] check, uses
getIconForMeetingService(event?.meetingLink?.service) when format == .eventtype
or NSImage(named: Defaults[.eventTitleIconFormat].rawValue) otherwise, sets
button.image and button.image?.size = MenuStyleConstants.iconSize, then sets
button.imagePosition to .noImage if button.image?.name() == "no_online_session"
else .imageLeft, and replace the duplicated blocks in
renderNextUpcomingItem()/renderEvent() with calls to this new helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b25aff31-df25-489e-88ea-7bec5e76fab0
📒 Files selected for processing (7)
MeetingBar/Core/Models/MBEvent+Helpers.swiftMeetingBar/Extensions/DefaultsKeys.swiftMeetingBar/UI/StatusBar/StatusBarItemController.swiftMeetingBar/UI/Views/Preferences/AppearanceTab.swiftMeetingBar/Utilities/Constants.swiftMeetingBarTests/CurrentAndUpcomingEventTests.swiftMeetingBarTests/TimelineLogicTests.swift
Comment on lines
+278
to
+294
| menuTitle.append(NSAttributedString( | ||
| string: "Next: ", | ||
| attributes: [.font: boldFont] | ||
| )) | ||
|
|
||
| var eventText = upcomingTitle | ||
| if Defaults[.eventTimeFormat] == .show { | ||
| eventText += " " + upcomingTime | ||
| } | ||
|
|
||
| menuTitle.append(NSAttributedString( | ||
| string: eventText, | ||
| attributes: [.font: regularFont] | ||
| )) | ||
|
|
||
| button.attributedTitle = menuTitle | ||
| button.toolTip = "Now: \(current.title)\nNext: \(upcoming.title)" |
There was a problem hiding this comment.
Hardcoded UI strings should be localized.
The "Next: " prefix (line 279) and tooltip format "Now: ... / Next: ..." (line 294) are hardcoded strings that won't be translated for non-English users.
🌐 Example fix
menuTitle.append(NSAttributedString(
- string: "Next: ",
+ string: "status_bar_flip_next_prefix".loco(),
attributes: [.font: boldFont]
))
...
- button.toolTip = "Now: \(current.title)\nNext: \(upcoming.title)"
+ button.toolTip = "status_bar_flip_tooltip".loco(current.title, upcoming.title)🤖 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 278 -
294, The hardcoded UI strings ("Next: " and the tooltip "Now: ...\nNext: ...")
must be localized: replace the literal "Next: " used when appending to menuTitle
with a localized string (e.g. NSLocalizedString key like "StatusBar.NextPrefix")
and build eventText as before; and replace the literal tooltip construction with
a localized format string (e.g. NSLocalizedString key like
"StatusBar.Tooltip.NowNext" = "Now: %@\nNext: %@") and call
String(format:localizedTooltip, current.title, upcoming.title). Update usages
around menuTitle, button.attributedTitle and button.toolTip (and keep
Default[.eventTimeFormat] logic and attributes) so all user-facing text is
loaded via NSLocalizedString keys with appropriate placeholders for
upcomingTitle/upcomingTime/current.title/upcoming.title.
Comment on lines
+251
to
+269
| Picker("Next event preview:", selection: $nextEventFlipMode) { | ||
| Text("Disabled").tag(NextEventFlipMode.disabled) | ||
| Text("Show after start").tag(NextEventFlipMode.showAfterStart) | ||
| Text("Show after 10 min").tag(NextEventFlipMode.showAfterTenMin) | ||
| } | ||
| .fixedSize() | ||
| .disabled(ongoingEventVisibility != .showTenMinBeforeNext) | ||
| .help(ongoingEventVisibility != .showTenMinBeforeNext | ||
| ? "Requires \"Ongoing event visibility\" to be set to \"hide 10 min before next event\"" | ||
| : "Alternate between showing the current and upcoming event in the status bar") | ||
|
|
||
| if nextEventFlipMode != .disabled && ongoingEventVisibility == .showTenMinBeforeNext { | ||
| Picker("Flip every", selection: $nextEventFlipIntervalSeconds) { | ||
| ForEach([2, 3, 5, 7, 10, 15], id: \.self) { sec in | ||
| Text("\(sec)s").tag(sec) | ||
| } | ||
| } | ||
| .fixedSize() | ||
| .help("How often to alternate between current and upcoming event") |
There was a problem hiding this comment.
Hardcoded strings should be localized.
The new UI strings ("Next event preview:", "Disabled", "Show after start", "Show after 10 min", "Flip every") are hardcoded while the rest of this file uses .loco() for localization. This will cause issues for non-English users.
🌐 Example fix for localization
- Picker("Next event preview:", selection: $nextEventFlipMode) {
- Text("Disabled").tag(NextEventFlipMode.disabled)
- Text("Show after start").tag(NextEventFlipMode.showAfterStart)
- Text("Show after 10 min").tag(NextEventFlipMode.showAfterTenMin)
+ Picker("preferences_appearance_status_bar_flip_mode_title".loco(), selection: $nextEventFlipMode) {
+ Text("preferences_appearance_status_bar_flip_mode_disabled".loco()).tag(NextEventFlipMode.disabled)
+ Text("preferences_appearance_status_bar_flip_mode_after_start".loco()).tag(NextEventFlipMode.showAfterStart)
+ Text("preferences_appearance_status_bar_flip_mode_after_ten_min".loco()).tag(NextEventFlipMode.showAfterTenMin)
}Apply similar changes to the help strings and "Flip every" picker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBar/UI/Views/Preferences/AppearanceTab.swift` around lines 251 - 269,
The new picker labels and help strings in the UI (the Picker with label "Next
event preview:", the Text tags "Disabled", "Show after start", "Show after 10
min", the conditional Picker label "Flip every" and their .help(...) calls) are
hardcoded and must be localized like the rest of the file; replace those raw
strings with the localized equivalents using .loco() (or the existing
localization helper) when creating the Picker label and each Text(...) tag, and
wrap the help messages with .loco() as well, keeping the bindings
(nextEventFlipMode, NextEventFlipMode.*, nextEventFlipIntervalSeconds,
ongoingEventVisibility) and conditional logic unchanged.
Comment on lines
+239
to
+246
| /// Simulates the flip activation guard from StatusBarItemController. | ||
| /// Returns true if the flip should be active for the given mode. | ||
| private func shouldFlip(mode: NextEventFlipMode, currentStart: Date) -> Bool { | ||
| guard mode != .disabled else { return false } | ||
| let minutesIntoCurrent = now.timeIntervalSince(currentStart) / 60 | ||
| let minMinutesIn: Double = (mode == .showAfterTenMin) ? 10.0 : 0.0 | ||
| return minutesIntoCurrent >= minMinutesIn | ||
| } |
There was a problem hiding this comment.
Test simulation is incomplete - missing ongoingEventVisibility check.
The shouldFlip() helper simulates the flip activation guard but is missing a critical condition. The production code in StatusBarItemController.updateTitle() requires both:
flipMode != .disabledDefaults[.ongoingEventVisibility] == .showTenMinBeforeNext
The test only verifies condition (1). This means tests could pass even if the visibility check were broken in production.
💚 Proposed fix to match production logic
/// Simulates the flip activation guard from StatusBarItemController.
/// Returns true if the flip should be active for the given mode.
- private func shouldFlip(mode: NextEventFlipMode, currentStart: Date) -> Bool {
- guard mode != .disabled else { return false }
+ private func shouldFlip(
+ mode: NextEventFlipMode,
+ ongoingVisibility: OngoingEventVisibility,
+ currentStart: Date
+ ) -> Bool {
+ guard mode != .disabled,
+ ongoingVisibility == .showTenMinBeforeNext else { return false }
let minutesIntoCurrent = now.timeIntervalSince(currentStart) / 60
let minMinutesIn: Double = (mode == .showAfterTenMin) ? 10.0 : 0.0
return minutesIntoCurrent >= minMinutesIn
}Then update tests to pass the visibility parameter and add tests for when visibility is set to other values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBarTests/CurrentAndUpcomingEventTests.swift` around lines 239 - 246,
The helper shouldFlip(mode: NextEventFlipMode, currentStart: Date) must mirror
production by also checking Defaults[.ongoingEventVisibility]; update its
signature to accept an ongoingEventVisibility parameter (or read Defaults inside
the helper) and return false unless ongoingEventVisibility ==
.showTenMinBeforeNext in addition to mode != .disabled and the time check;
update all tests that call shouldFlip and any uses in
CurrentAndUpcomingEventTests to pass/verify different ongoingEventVisibility
values and add tests covering visibility values other than .showTenMinBeforeNext
to ensure behavior matches StatusBarItemController.updateTitle.
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
Adds a new "next event preview" feature that alternates the status bar text between the current meeting and the upcoming meeting, giving users an at-a-glance preview of what's coming up before their current meeting ends.
Problem
When you're in a meeting and the next one is approaching, MeetingBar only shows the current meeting until the "ongoing event visibility" threshold kicks in (e.g., 10 min before the next event), at which point it abruptly switches. There's no advance preview of what's coming up next.
Solution
When there's an ongoing meeting and another meeting starting soon (within 15 minutes of the current meeting's end), the status bar now alternates between showing:
The flip happens on a configurable interval (default: every 5 seconds) and is controlled by a new preference.
Preferences (Appearance → Status Bar)
A new dropdown appears below "Ongoing event visibility":
The dropdown is automatically disabled when "Ongoing event visibility" is not set to "hide 10 min before next event", since the feature only makes sense with that visibility mode.
The "Flip every" dropdown (2s, 3s, 5s, 7s, 10s, 15s) controls how fast the alternation occurs and only appears when the feature is enabled.
Display Details
NSAttributedStringto visually distinguish it from the current meeting displayFiles Changed
MeetingBar/Core/Models/MBEvent+Helpers.swiftcurrentAndUpcomingEvent(gapThreshold:)methodMeetingBar/Extensions/DefaultsKeys.swiftnextEventFlipModeandnextEventFlipIntervalSecondskeysMeetingBar/Utilities/Constants.swiftNextEventFlipModeenumMeetingBar/UI/StatusBar/StatusBarItemController.swiftrenderUpcomingEvent()with bold prefix, refactoredupdateTitle()MeetingBar/UI/Views/Preferences/AppearanceTab.swiftMeetingBarTests/CurrentAndUpcomingEventTests.swiftMeetingBarTests/TimelineLogicTests.swiftSwiftUICoreimportSummary by CodeRabbit
Release Notes
New Features
Tests