Add 'Join Current Meeting' keyboard shortcut#897
Add 'Join Current Meeting' keyboard shortcut#897taakmatheus wants to merge 3 commits intoleits:masterfrom
Conversation
Add support for joining an in-progress meeting directly. Introduces Array.currentEvent(linkRequired:) to find the currently running event while respecting visibility/dismissal rules, a new KeyboardShortcuts.Name.joinCurrentEventShortcut, and UI/behavior changes: MenuBuilder now selects joinCurrent vs joinNext action/shortcut when the next event has already started; StatusBarItemController registers the new shortcut and implements joinCurrentMeeting() (opens current event or posts a notification if none); Preferences General tab gains recorder entries for the new shortcut.
dosubot
bot
added
the
size:M
WalkthroughAdds a new MBEvent.currentEvent(...) helper, keyboard shortcuts (including join-current), menu/status-bar logic to prefer joining an in-progress meeting, UI to configure the new shortcut, and tests validating current-event behavior and menu actions. Changes
Sequence DiagramsequenceDiagram
actor User
participant Keyboard as Keyboard Shortcut
participant Controller as StatusBarItemController
participant Events as MBEvent Array (currentEvent)
participant Launcher as Meeting Launcher
User->>Keyboard: Press joinCurrentEventShortcut
Keyboard->>Controller: invoke joinCurrentMeeting()
Controller->>Events: currentEvent(linkRequired: Bool)
alt event found
Events-->>Controller: MBEvent
Controller->>Launcher: open meeting link
Launcher-->>User: join meeting
else no event
Events-->>Controller: nil
Controller-->>User: show notification (no current meeting)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #897 +/- ##
===========================================
+ Coverage 26.30% 36.83% +10.52%
===========================================
Files 36 48 +12
Lines 5645 5172 -473
Branches 2210 1756 -454
===========================================
+ Hits 1485 1905 +420
+ Misses 4103 3204 -899
- Partials 57 63 +6 ☔ 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.
🧹 Nitpick comments (3)
MeetingBar/UI/Views/Preferences/GeneralTab.swift (2)
46-47: Inconsistent label formatting with trailing colon.The label appends a colon (
+ ":") while other shortcut labels in this section (e.g., line 49preferences_general_shortcut_join_next) don't include a trailing colon. Consider removing the colon for consistency, or adding a dedicated localization key that includes proper punctuation.✨ Suggested fix
- Text("status_bar_section_join_current_meeting".loco() + ":") + Text("status_bar_section_join_current_meeting".loco())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MeetingBar/UI/Views/Preferences/GeneralTab.swift` around lines 46 - 47, The label Text("status_bar_section_join_current_meeting".loco() + ":") is inconsistent with other shortcut labels; remove the hard-coded trailing colon by changing the Text call to use the localized string without appending ":" (i.e., use "status_bar_section_join_current_meeting".loco()) so it matches other entries like the KeyboardShortcuts.Recorder(for: .joinCurrentEventShortcut); alternatively, if punctuation must be part of the localized text, replace the concatenation with a dedicated localization key (e.g., add a key that includes the colon) and use that key instead.
80-84: Same trailing colon inconsistency in modal.Apply the same fix here for consistency with other shortcuts in the modal list.
✨ Suggested fix
HStack { - Text("status_bar_section_join_current_meeting".loco() + ":") + Text("status_bar_section_join_current_meeting".loco()) Spacer() KeyboardShortcuts.Recorder(for: .joinCurrentEventShortcut) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MeetingBar/UI/Views/Preferences/GeneralTab.swift` around lines 80 - 84, The Text in the HStack for the join shortcut currently appends a hardcoded ":" which causes trailing-colon inconsistency; update the HStack so Text("status_bar_section_join_current_meeting".loco()) does not concatenate ":", leaving punctuation to the localized string (or add the colon into the localization if needed) and keep the rest intact (HStack and KeyboardShortcuts.Recorder(for: .joinCurrentEventShortcut)) to match the other modal shortcut entries.MeetingBar/Core/Models/MBEvent+Helpers.swift (1)
176-213: MissingpersonalEventsAppereancefilter that exists innextEvent().The
nextEvent()method (lines 107-109) filters out personal events whenDefaults[.personalEventsAppereance] != .show_active. The newcurrentEvent()method lacks this filter, which could cause inconsistent behavior: an event might be hidden fromnextEvent()but returned bycurrentEvent().Consider adding the same filter for consistency:
♻️ Proposed fix
func currentEvent(linkRequired: Bool = false) -> MBEvent? { let now = Date() + // Filter out personal events if not marked as 'active' + let candidates: [MBEvent] + if Defaults[.personalEventsAppereance] != .show_active { + candidates = self.filter { !$0.attendees.isEmpty } + } else { + candidates = self + } + - for event in self { + for event in candidates { guard event.startDate <= now, event.endDate > now else { continue }🤖 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 176 - 213, currentEvent(linkRequired:) is missing the same personal-events filter used in nextEvent(), causing inconsistent results; update currentEvent to skip events where event.isPersonal is true when Defaults[.personalEventsAppereance] != .show_active (same check/logic used in nextEvent()) so personal events are filtered out consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@MeetingBar/Core/Models/MBEvent`+Helpers.swift:
- Around line 176-213: currentEvent(linkRequired:) is missing the same
personal-events filter used in nextEvent(), causing inconsistent results; update
currentEvent to skip events where event.isPersonal is true when
Defaults[.personalEventsAppereance] != .show_active (same check/logic used in
nextEvent()) so personal events are filtered out consistently.
In `@MeetingBar/UI/Views/Preferences/GeneralTab.swift`:
- Around line 46-47: The label
Text("status_bar_section_join_current_meeting".loco() + ":") is inconsistent
with other shortcut labels; remove the hard-coded trailing colon by changing the
Text call to use the localized string without appending ":" (i.e., use
"status_bar_section_join_current_meeting".loco()) so it matches other entries
like the KeyboardShortcuts.Recorder(for: .joinCurrentEventShortcut);
alternatively, if punctuation must be part of the localized text, replace the
concatenation with a dedicated localization key (e.g., add a key that includes
the colon) and use that key instead.
- Around line 80-84: The Text in the HStack for the join shortcut currently
appends a hardcoded ":" which causes trailing-colon inconsistency; update the
HStack so Text("status_bar_section_join_current_meeting".loco()) does not
concatenate ":", leaving punctuation to the localized string (or add the colon
into the localization if needed) and keep the rest intact (HStack and
KeyboardShortcuts.Recorder(for: .joinCurrentEventShortcut)) to match the other
modal shortcut entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 42c2745c-4392-4ebe-a42f-a9b49692d8e1
📒 Files selected for processing (5)
MeetingBar/Core/Models/MBEvent+Helpers.swiftMeetingBar/Extensions/KeyboardShortcutsNames.swiftMeetingBar/UI/StatusBar/MenuBuilder.swiftMeetingBar/UI/StatusBar/StatusBarItemController.swiftMeetingBar/UI/Views/Preferences/GeneralTab.swift
Update MBEvent+Helpers to document currentEvent and add a linkRequired parameter (default false) so callers can opt to consider only events with meeting links. Add descriptive comments for global keyboard shortcut names in KeyboardShortcutsNames. Also add documentation for joinCurrentMeeting in StatusBarItemController to clarify behavior when no active event is present. Changes are focused on widening API clarity and improving inline documentation across these components.
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/Extensions/KeyboardShortcutsNames.swift`:
- Around line 18-19: Update the documentation for the static constant
joinEventShortcut to reflect its actual behavior: change the comment so it
states that this global shortcut triggers joining the next meeting (not the
nearest/current), since StatusBarItemController.setupKeyboardShortcuts maps
.joinEventShortcut to joinNextMeeting(); locate the constant in
KeyboardShortcutsNames.swift (joinEventShortcut) and replace the misleading
phrase "nearest meeting (current or next)" with wording that clearly indicates
it starts/join the next scheduled meeting.
In `@MeetingBar/UI/StatusBar/StatusBarItemController.swift`:
- Line 415: Replace the generic next-meeting copy in the current-meeting "no
active event" branch by using a current-meeting-specific localization key
instead of "next_meeting_empty_message" in the sendNotification call; update the
call in StatusBarItemController where
sendNotification("status_bar_section_join_current_meeting".loco(),
"next_meeting_empty_message".loco()) is used to reference a dedicated key like
"current_meeting_empty_message" (and add that localization entry if it doesn’t
exist) so the title ("status_bar_section_join_current_meeting") and message
reflect the current-meeting context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3dc24c78-754d-4b09-ba45-e00b1683ee53
📒 Files selected for processing (3)
MeetingBar/Core/Models/MBEvent+Helpers.swiftMeetingBar/Extensions/KeyboardShortcutsNames.swiftMeetingBar/UI/StatusBar/StatusBarItemController.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- MeetingBar/Core/Models/MBEvent+Helpers.swift
| /// Global shortcut used to join the nearest meeting (current or next). | ||
| static let joinEventShortcut = Self("joinEventShortcut") |
There was a problem hiding this comment.
Fix joinEventShortcut doc to match actual behavior.
The comment says “nearest meeting (current or next)”, but current wiring in MeetingBar/UI/StatusBar/StatusBarItemController.swift (setupKeyboardShortcuts) maps .joinEventShortcut to joinNextMeeting(). Update the doc to avoid API confusion.
✏️ Suggested doc fix
- /// Global shortcut used to join the nearest meeting (current or next).
+ /// Global shortcut used to join the next meeting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBar/Extensions/KeyboardShortcutsNames.swift` around lines 18 - 19,
Update the documentation for the static constant joinEventShortcut to reflect
its actual behavior: change the comment so it states that this global shortcut
triggers joining the next meeting (not the nearest/current), since
StatusBarItemController.setupKeyboardShortcuts maps .joinEventShortcut to
joinNextMeeting(); locate the constant in KeyboardShortcutsNames.swift
(joinEventShortcut) and replace the misleading phrase "nearest meeting (current
or next)" with wording that clearly indicates it starts/join the next scheduled
meeting.
| if let currentEvent = events.currentEvent() { | ||
| currentEvent.openMeeting() | ||
| } else { | ||
| sendNotification("status_bar_section_join_current_meeting".loco(), "next_meeting_empty_message".loco()) |
There was a problem hiding this comment.
Use current-meeting-specific notification copy here.
The “no active event” branch uses "next_meeting_empty_message", which can produce misleading text in the current-meeting path. Prefer dedicated current-meeting title/message localization keys for this branch.
💡 Suggested adjustment
- sendNotification("status_bar_section_join_current_meeting".loco(), "next_meeting_empty_message".loco())
+ sendNotification("current_meeting_empty_title".loco(), "current_meeting_empty_message".loco())📝 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.
| sendNotification("status_bar_section_join_current_meeting".loco(), "next_meeting_empty_message".loco()) | |
| sendNotification("current_meeting_empty_title".loco(), "current_meeting_empty_message".loco()) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBar/UI/StatusBar/StatusBarItemController.swift` at line 415, Replace
the generic next-meeting copy in the current-meeting "no active event" branch by
using a current-meeting-specific localization key instead of
"next_meeting_empty_message" in the sendNotification call; update the call in
StatusBarItemController where
sendNotification("status_bar_section_join_current_meeting".loco(),
"next_meeting_empty_message".loco()) is used to reference a dedicated key like
"current_meeting_empty_message" (and add that localization entry if it doesn’t
exist) so the title ("status_bar_section_join_current_meeting") and message
reflect the current-meeting context.
Add unit tests covering currentEvent() behavior and menu join actions. NextEventTests: verify currentEvent() returns a running event and that the linkRequired parameter filters out events without links. MenuBuilderTests: assert the current meeting menu item uses the joinCurrentMeeting selector and add a test ensuring a future event produces the "status_bar_section_join_next_meeting" title and uses the joinNextMeeting selector.
dosubot
bot
added
size:L
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| let future = makeFakeEvent( | ||
| id: "F", | ||
| start: Date().addingTimeInterval(300), | ||
| end: Date().addingTimeInterval(1_200) |
Check warning
Code scanning / Swiftlint (reported by Codacy)
Underscores should be used as thousand separator in large decimal numbers. Warning
| XCTAssertEqual(array.nextEvent(), running) | ||
| } | ||
|
|
||
| func test_currentEvent_returnsRunningEvent() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
| XCTAssertEqual([future, running].currentEvent(), running) | ||
| } | ||
|
|
||
| func test_currentEvent_respectsLinkRequirement() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MeetingBarTests/StatusBarItem/MenuBuilderTests.swift (1)
47-57:⚠️ Potential issue | 🟡 MinorPotential flaky test due to timing edge case.
The event is created with
start: Date()(line 49). ThebuildJoinSectionlogic atMenuBuilder.swift:74comparesnextEvent.startDate < nowwherenowis captured inside the method. If any time elapses between event creation and the comparison (even microseconds), the condition becomestrueandjoinCurrentMeetingis selected. However, if executed fast enough, the times could be equal, making the conditionfalseand selectingjoinNextMeetinginstead.To make this deterministic, offset the start time slightly into the past:
Proposed fix
func test_joinSectionHasCreateAndJoin() { let next = makeFakeEvent(id: "J", - start: Date(), end: Date().addingTimeInterval(60)) + start: Date().addingTimeInterval(-1), end: Date().addingTimeInterval(60)) let items = MenuBuilder(target: Dummy()) .buildJoinSection(nextEvent: next)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MeetingBarTests/StatusBarItem/MenuBuilderTests.swift` around lines 47 - 57, The test test_joinSectionHasCreateAndJoin is flaky because makeFakeEvent uses start: Date() and MenuBuilder.buildJoinSection compares nextEvent.startDate < now; change the test to create the event with a small negative offset (e.g. start: Date().addingTimeInterval(-1)) so nextEvent.startDate is guaranteed to be before now, keeping the assertions for StatusBarItemController.joinCurrentMeeting and createMeetingAction deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@MeetingBarTests/StatusBarItem/MenuBuilderTests.swift`:
- Around line 47-57: The test test_joinSectionHasCreateAndJoin is flaky because
makeFakeEvent uses start: Date() and MenuBuilder.buildJoinSection compares
nextEvent.startDate < now; change the test to create the event with a small
negative offset (e.g. start: Date().addingTimeInterval(-1)) so
nextEvent.startDate is guaranteed to be before now, keeping the assertions for
StatusBarItemController.joinCurrentMeeting and createMeetingAction
deterministic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f2594337-45df-4ee6-9cc6-b16f19d66826
📒 Files selected for processing (2)
MeetingBarTests/NextEventTests.swiftMeetingBarTests/StatusBarItem/MenuBuilderTests.swift
|
@leits please could you verify this PR? |
1 participant
Add support for joining an in-progress meeting directly. Introduces Array.currentEvent(linkRequired:) to find the currently running event while respecting visibility/dismissal rules, a new KeyboardShortcuts.Name.joinCurrentEventShortcut, and UI/behavior changes: MenuBuilder now selects joinCurrent vs joinNext action/shortcut when the next event has already started; StatusBarItemController registers the new shortcut and implements joinCurrentMeeting() (opens current event or posts a notification if none); Preferences General tab gains recorder entries for the new shortcut.
Status
READY/IN DEVELOPMENT/HOLD
Description
A few sentences describing the overall goals of the pull request's commits.
Checklist
Steps to Test or Reproduce
Outline the steps to test or reproduce the PR here.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests