[V3] Add getAccentColor method
#4330
Conversation
|
""" WalkthroughA new feature was introduced to retrieve the system accent color on macOS. This includes adding a Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant platformApp (macosApp)
participant macOS System
App->>platformApp (macosApp): GetAccentColor()
alt platformApp is nil
App->>App: return "rgb(0,122,255)"
else platformApp is macosApp
platformApp (macosApp)->>macOS System: getAccentColor()
macOS System-->>platformApp (macosApp): "rgb(r,g,b)"
platformApp (macosApp)-->>App: "rgb(r,g,b)"
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
Documentation
|
I'm not sure where any tests for this feature would be added in the project? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/content/docs/changelog.mdx (1)
164-164: Fix the macOS branding.Apple's operating system should be written as "macOS" not "MacOS".
-- `app.GetAccentColor()` to get the accent color of a user's system. Works on MacOS. by [@etesam913](https://github.com/etesam913) +- `app.GetAccentColor()` to get the accent color of a user's system. Works on macOS. by [@etesam913](https://github.com/etesam913)🧰 Tools
🪛 LanguageTool
[grammar] ~164-~164: The operating system from Apple is written “macOS”.
Context: ...cent color of a user's system. Works on MacOS. by [@Etesam913](https://github.com/ete...(MAC_OS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/src/content/docs/changelog.mdx(1 hunks)v3/pkg/application/application.go(3 hunks)v3/pkg/application/application_darwin.go(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/content/docs/changelog.mdx
[grammar] ~164-~164: The operating system from Apple is written “macOS”.
Context: ...cent color of a user's system. Works on MacOS. by [@Etesam913](https://github.com/ete...
(MAC_OS)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: semgrep/ci
🔇 Additional comments (4)
v3/pkg/application/application_darwin.go (2)
76-104: Excellent implementation with robust error handling.The C implementation demonstrates several best practices:
- Proper memory management with
@autoreleasepool- Version-aware API usage with fallback for older macOS versions
- Graceful handling of color space conversion failures
- Consistent RGB formatting for cross-platform compatibility
219-223: Proper CGO memory management.The Go wrapper correctly handles memory management by using
defer C.free()to ensure the allocated C string is properly freed.v3/pkg/application/application.go (2)
208-208: Interface extension follows established patterns.The addition of
getAccentColor() stringto theplatformAppinterface is consistent with other platform-specific methods.
1051-1056: Well-designed public API with sensible defaults.The method correctly handles the nil platform implementation case and provides a reasonable default blue color that matches Apple's system blue. The implementation follows the same pattern as other platform-dependent methods.
|
Thanks for opening this. Do you think it might be better to use something like |
|
@Etesam913 thoughts? |
The PR added getAccentColor() method to the interface and implemented it for macOS, but missed the Linux and Windows implementations, causing build failures. - Added getAccentColor() method to linuxApp that returns default blue color - Added getAccentColor() method to windowsApp that returns default blue color - Both implementations return "rgb(0,122,255)" as fallback since these platforms don't have unified system accent color APIs readily available Fixes build failures on Linux and Windows CI runners. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
dosubot
bot
added
the
size:M
|
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #4329
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit