Add option to show fullscreen notifications on all screens - #907
Add option to show fullscreen notifications on all screens#907JonathanGuberman wants to merge 1 commit into
Conversation
Users with multiple displays only see the fullscreen meeting notification on their main screen, making it easy to miss. This adds a toggleable "Show on all screens" setting that creates a notification window on every connected display with synchronized dismissal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dosubot
Bot
added
the
size:L
WalkthroughThe PR adds multi-screen support for fullscreen notifications. When the new Changes
Sequence DiagramsequenceDiagram
participant AppDelegate
participant Defaults
participant NSScreen
participant NSWindow
participant FullscreenNotification
AppDelegate->>Defaults: Check fullscreenNotificationAllScreens
Defaults-->>AppDelegate: Setting value
alt Setting enabled
AppDelegate->>NSScreen: Get NSScreen.screens
else Setting disabled
AppDelegate->>NSScreen: Get NSScreen.main
end
NSScreen-->>AppDelegate: Selected screens list
loop For each screen
AppDelegate->>NSWindow: Create new NSWindow
AppDelegate->>NSWindow: Configure (appearance, collectionBehavior, etc.)
AppDelegate->>FullscreenNotification: Initialize with event and windows array
AppDelegate->>NSWindow: Order front and show
end
FullscreenNotification-->>AppDelegate: Notification UI displayed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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}
|
|
||
| class FullscreenNotificationTests: BaseTestCase { | ||
|
|
||
| func test_fullscreenNotificationAllScreens_defaultsToFalse() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
| XCTAssertFalse(Defaults[.fullscreenNotificationAllScreens]) | ||
| } | ||
|
|
||
| func test_fullscreenNotificationAllScreens_canBeEnabled() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
| XCTAssertTrue(Defaults[.fullscreenNotificationAllScreens]) | ||
| } | ||
|
|
||
| func test_fullscreenNotificationAllScreens_independentOfMainToggle() { |
Check warning
Code scanning / Tailor (reported by Codacy)
Function names should be lowerCamelCase Warning
|
|
||
| XCTAssertFalse(Defaults[.fullscreenNotification]) | ||
| XCTAssertTrue(Defaults[.fullscreenNotificationAllScreens]) | ||
| } |
Check warning
Code scanning / Tailor (reported by Codacy)
Function should have at least one blank line after it Warning
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #907 +/- ##
===========================================
+ Coverage 26.30% 36.49% +10.19%
===========================================
Files 36 49 +13
Lines 5645 5121 -524
Branches 2210 1740 -470
===========================================
+ Hits 1485 1869 +384
+ Misses 4103 3196 -907
+ 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: 1
🤖 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/App/AppDelegate.swift`:
- Around line 260-261: The second assignment to window.collectionBehavior
overwrites the first, dropping .canJoinAllSpaces; change the code that sets
window.collectionBehavior (in AppDelegate.swift where window.collectionBehavior
is assigned) to combine the options instead of replacing them—use the OptionSet
union (e.g., set both flags together or call insert on
window.collectionBehavior) so the window keeps both .canJoinAllSpaces and
.moveToActiveSpace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0352fd25-4093-4fc6-85c6-61d09fc7cd27
📒 Files selected for processing (26)
MeetingBar/App/AppDelegate.swiftMeetingBar/Extensions/DefaultsKeys.swiftMeetingBar/Resources /Localization /cs.lproj/Localizable.stringsMeetingBar/Resources /Localization /de.lproj/Localizable.stringsMeetingBar/Resources /Localization /en.lproj/Localizable.stringsMeetingBar/Resources /Localization /es.lproj/Localizable.stringsMeetingBar/Resources /Localization /fr.lproj/Localizable.stringsMeetingBar/Resources /Localization /he.lproj/Localizable.stringsMeetingBar/Resources /Localization /hr.lproj/Localizable.stringsMeetingBar/Resources /Localization /hu.lproj/Localizable.stringsMeetingBar/Resources /Localization /it.lproj/Localizable.stringsMeetingBar/Resources /Localization /ja.lproj/Localizable.stringsMeetingBar/Resources /Localization /ko.lproj/Localizable.stringsMeetingBar/Resources /Localization /nb-NO.lproj/Localizable.stringsMeetingBar/Resources /Localization /nl.lproj/Localizable.stringsMeetingBar/Resources /Localization /pl.lproj/Localizable.stringsMeetingBar/Resources /Localization /pt-BR.lproj/Localizable.stringsMeetingBar/Resources /Localization /pt.lproj/Localizable.stringsMeetingBar/Resources /Localization /sk.lproj/Localizable.stringsMeetingBar/Resources /Localization /ta.lproj/Localizable.stringsMeetingBar/Resources /Localization /tr.lproj/Localizable.stringsMeetingBar/Resources /Localization /uk.lproj/Localizable.stringsMeetingBar/Resources /Localization /zh-Hans.lproj/Localizable.stringsMeetingBar/UI/Views/FullscreenNotification.swiftMeetingBar/UI/Views/Shared.swiftMeetingBarTests/FullscreenNotificationTests.swift
| window.collectionBehavior = .canJoinAllSpaces | ||
| window.collectionBehavior = .moveToActiveSpace |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In AppKit, is NSWindow.collectionBehavioranOptionSet, and does assigning it twice replace previously set flags? What is the correct way to combine .canJoinAllSpacesand.moveToActiveSpace?
💡 Result:
Yes, NSWindow.collectionBehavior is an NSWindow.CollectionBehavior type, which is a Swift OptionSet (struct with OptionSet conformance, supporting bitwise operations). Assigning it multiple times replaces the previous value entirely, as it is a simple property setter (var collectionBehavior: NSWindow.CollectionBehavior { get set }). The correct way to combine .canJoinAllSpaces and .moveToActiveSpace is to use the bitwise OR operator: window.collectionBehavior = [.canJoinAllSpaces, .moveToActiveSpace]. This creates a single value with both flags set. Note that some behaviors are mutually exclusive (e.g., within Spaces group), but .canJoinAllSpaces and .moveToActiveSpace can be combined as they are distinct options, though their effects may interact (window appears in all spaces but moves when activated). Example: window.collectionBehavior = [.canJoinAllSpaces, .moveToActiveSpace]
Citations:
- 1: https://developer.apple.com/documentation/appkit/nswindow/collectionbehavior-swift.struct
- 2: https://developer.apple.com/documentation/appkit/nswindow/collectionbehavior-swift.property?changes=_7_5
- 3: https://learn.microsoft.com/ms-my/dotnet/api/appkit.nswindowcollectionbehavior?view=net-macos-26.1-10.0
- 4: https://developer.apple.com/documentation/appkit/nswindow/collectionbehavior-swift.struct?changes=_9
Combine the collection behavior flags instead of overwriting them.
The second assignment replaces the first one, so these windows lose .canJoinAllSpaces. That can keep fullscreen notifications from following the same Spaces/fullscreen behavior as before.
Suggested fix
- window.collectionBehavior = .canJoinAllSpaces
- window.collectionBehavior = .moveToActiveSpace
+ window.collectionBehavior = [.canJoinAllSpaces, .moveToActiveSpace]📝 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.
| window.collectionBehavior = .canJoinAllSpaces | |
| window.collectionBehavior = .moveToActiveSpace | |
| window.collectionBehavior = [.canJoinAllSpaces, .moveToActiveSpace] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MeetingBar/App/AppDelegate.swift` around lines 260 - 261, The second
assignment to window.collectionBehavior overwrites the first, dropping
.canJoinAllSpaces; change the code that sets window.collectionBehavior (in
AppDelegate.swift where window.collectionBehavior is assigned) to combine the
options instead of replacing them—use the OptionSet union (e.g., set both flags
together or call insert on window.collectionBehavior) so the window keeps both
.canJoinAllSpaces and .moveToActiveSpace.
…eits#907) Original PR: leits#907 by JonathanGuberman Ported onto WindowCoordinator (the multi-monitor-aware fullscreen alert system introduced by leits#941, which the original PR predates) rather than reverting to a simpler per-call NSScreen.main lookup: - openFullscreenNotificationWindow now opens on every NSScreen.screens entry when fullscreenNotificationAllScreens is on, or on the existing preferredFullscreenScreen() otherwise. - FullscreenNotification now takes windows: [NSWindow] instead of a single optional window, so dismiss/join closes the whole group. - All opened windows are still registered in fullscreenNotificationWindows so leits#941's disconnect/reconnect repositioning keeps working. - New fullscreenNotificationAllScreens Defaults key + toggle under the existing fullscreen notification section.
…its#866) Original PR: leits#866 by awonnacott Ported onto the StatusBarPresentation policy/render split (this PR predates it, like leits#922/leits#913/leits#907): - New EventLocationFormat enum (show/show_under_title/hide) + Defaults key, mirroring the existing EventTimeFormat. - StatusBarPresentation gains location/locationDisplay fields, plus an explicit timeDisplay field (previously only derivable from the coarse 'layout' enum) so the renderer can correctly combine time and location on the stacked second line regardless of which combination the user picked. - titleLayout() now also switches to the stacked (2-line) layout when location is 'show under title', independent of the time setting. - StatusBarTitlePolicy.processedLocation() truncates the event's first location line to the same length limit as the title. - Inline renderer appends location with a bullet separator; the stacked renderer (new stackedTitleLines helper) builds both lines from whichever combination of time/location the user placed on each. - New preference picker (PreferencesStatusBarLocationOption, mirroring PreferencesStatusBarTimeOption) + localization strings. - Fixed 5 test-constructor call sites across MenuBuilderTests.swift and StatusBarPresentationPolicyTests.swift for the new required fields.
2 participants
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Localization