Add option to show current date on status bar icon#922
Open
apomerenk wants to merge 5 commits intoleits:masterfrom
Open
Add option to show current date on status bar icon#922apomerenk wants to merge 5 commits intoleits:masterfrom
apomerenk wants to merge 5 commits intoleits:masterfrom
Conversation
Adds a 'Show today's date on icon' toggle in Appearance preferences that overlays the current day-of-month number on the calendar status bar icon. Useful when there are no upcoming events and the icon would otherwise be visually empty (closes leits#654). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
the
size:M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a new Boolean preference Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as "User"
participant PrefsUI as "Preferences UI"
participant Defaults as "Defaults Store"
participant StatusBar as "StatusBarItemController"
participant Renderer as "Icon Renderer"
User->>PrefsUI: Toggle "Show date on icon"
PrefsUI->>Defaults: Set `showDateOnIcon`
Defaults->>StatusBar: Publish change
StatusBar->>Renderer: Request icon for current format
Renderer->>Renderer: Load base calendar image
alt showDateOnIcon = true and format allows
Renderer->>Renderer: Compose calendar image + overlay day text
else
Renderer->>Renderer: Return base image unchanged
end
Renderer-->>StatusBar: Return image
StatusBar->>StatusBar: Update menu bar item
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Helper was returning NSImage? but call site at line 244 assigns to NSImage. The base NSImage(named:) was already force-unwrapped by the existing code, so make the helper return non-optional to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #922 +/- ##
==========================================
+ Coverage 36.26% 37.10% +0.84%
==========================================
Files 48 49 +1
Lines 5129 5271 +142
Branches 1743 1751 +8
==========================================
+ Hits 1860 1956 +96
- Misses 3213 3258 +45
- Partials 56 57 +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.
🧹 Nitpick comments (1)
MeetingBar/UI/StatusBar/StatusBarItemController.swift (1)
529-563: Verify the composed icon stays crisp on Retina displays.
lockFocus()flattens the asset into a single composed bitmap, so this may drop higher-resolution reps and soften the status-bar icon on high-DPI screens. Please verify the visual result; if it regresses, consider a composition path that preserves the source image representations.🤖 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 529 - 563, The composed calendar icon created in calendarIconWithOptionalDate currently uses lockFocus which flattens representations and can soften Retina assets; replace the lockFocus approach with explicit high-resolution bitmap composition: create an NSBitmapImageRep sized using the current backing scale (e.g., backingScale = NSScreen.main?.backingScaleFactor ?? 1.0, pixelsWide = Int(size.width * backingScale), pixelsHigh = Int(size.height * backingScale)), add that NSBitmapImageRep to a new NSImage (the composed image), set up an NSGraphicsContext with that bitmap (NSGraphicsContext(bitmapImageRep:)), draw baseImage into the context using draw(in:from:operation:fraction:respectFlipped:hints:) (so the image uses its bestRepresentation), draw the date string with imageInterpolation = .high and correct font scaling for the scale factor, remove lockFocus usage on composed and ensure composed.isTemplate is set from baseImage.isTemplate before returning.
🤖 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/UI/StatusBar/StatusBarItemController.swift`:
- Around line 529-563: The composed calendar icon created in
calendarIconWithOptionalDate currently uses lockFocus which flattens
representations and can soften Retina assets; replace the lockFocus approach
with explicit high-resolution bitmap composition: create an NSBitmapImageRep
sized using the current backing scale (e.g., backingScale =
NSScreen.main?.backingScaleFactor ?? 1.0, pixelsWide = Int(size.width *
backingScale), pixelsHigh = Int(size.height * backingScale)), add that
NSBitmapImageRep to a new NSImage (the composed image), set up an
NSGraphicsContext with that bitmap (NSGraphicsContext(bitmapImageRep:)), draw
baseImage into the context using
draw(in:from:operation:fraction:respectFlipped:hints:) (so the image uses its
bestRepresentation), draw the date string with imageInterpolation = .high and
correct font scaling for the scale factor, remove lockFocus usage on composed
and ensure composed.isTemplate is set from baseImage.isTemplate before
returning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 615b0c08-63c1-49b7-8006-b23d066185b1
📒 Files selected for processing (4)
MeetingBar/Extensions/DefaultsKeys.swiftMeetingBar/Resources /Localization /en.lproj/Localizable.stringsMeetingBar/UI/StatusBar/StatusBarItemController.swiftMeetingBar/UI/Views/Preferences/AppearanceTab.swift
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/UI/StatusBar/StatusBarItemController.swift`:
- Around line 529-533: The function calendarIconWithOptionalDate currently only
checks Defaults[.showDateOnIcon], so dates still get overlaid when the user has
set the icon format to .none; update calendarIconWithOptionalDate to also gate
the overlay by the icon format (e.g. check Defaults[.iconFormat] != .none) and
return the baseImage early if the icon format is .none, keeping the existing
Defaults[.showDateOnIcon] check and the rest of the overlay logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 134c276e-a4a0-495b-b51d-21adf2ddf940
📒 Files selected for processing (1)
MeetingBar/UI/StatusBar/StatusBarItemController.swift
Address CodeRabbit feedback: the showDateOnIcon toggle is UI-disabled for the .none and .appicon formats, but the saved Bool would still trigger an overlay on fallback calendar icons drawn from those code paths. Match the toggle's disabled-state intent by also requiring the icon format to be .calendar or .eventtype before compositing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extract the icon-compositing logic from StatusBarItemController into a free function (makeCalendarIcon) so it can be tested without spinning up the @mainactor controller. Adds coverage for: - guard returns base image when showDate is false - guard returns base image when icon format is .appicon or .none - composed image is produced for .calendar and .eventtype formats - isTemplate flag propagates from base to composed image - two-digit day rendering does not crash Addresses the codecov/patch coverage gap on PR leits#922. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
size:L
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| size: iconSize | ||
| ) | ||
| XCTAssertEqual(result.size, iconSize) | ||
| } |
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
iconCalendarandiconCalendarCheckmarkvariants), making the icon useful at a glance even when there are no upcoming eventsCloses #654
Implementation notes
DefaultskeyshowDateOnIcon(defaultfalse) — opt-in, no behavior change for existing userscalendarIconWithOptionalDate(named:)inStatusBarItemControllercomposites the day number onto the base icon and preserves itsisTemplateflag so macOS continues to tint it correctly in light/dark menu barsAppDelegate, so the date refreshes automatically at midnight without an extra timeren.lprojonly — Weblate will pick up the new key for other localesTest plan
iconCalendarSummary by CodeRabbit
New Features
Localization
Refactor
Tests