Backlog fixes: menu warnings, OKLCH formatting, Xcode settings, context menu shortcuts - #186
Merged
Conversation
…shortcut - AppDelegate: use computed activation policy instead of .prohibited in applicationWillFinishLaunching to prevent NSMenu hierarchy warnings - NSColor+Lab: reduce OKLCH chroma precision from 4 to 3 decimal places - Xcode: enable ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS - StatusBarController: add ⌘Q keyboard shortcut to Quit menu item Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t menu - Extract CGFloat.strippedDecimalString(maxDecimalPlaces:) to shared utility (CGFloat+Format.swift), replacing private helper in NSColor+Lab - Apply trailing-zero stripping to both toOklchString and toLabString - Add CGFloatFormatTests unit tests for the new utility - Add OKLCH-specific format tests for trailing zero behaviour - Rebuild status bar context menu with all keyboard shortcuts: Pick (⌘D/⇧⌘D), Copy (⌘C/⇧⌘C), System picker (⌘S/⇧⌘S), Swap (X), Preferences (⌘,), Quit (⌘Q) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR bundles several backlog fixes across app startup/menu behavior, color string formatting, Xcode build settings, and status bar context menu shortcuts.
Changes:
- Adjusts activation policy during
applicationWillFinishLaunchingto avoid NSMenu hierarchy warnings during launch. - Introduces
CGFloat.strippedDecimalString(maxDecimalPlaces:)and applies it to LAB/OKLCH formatting to strip trailing zeros. - Rebuilds the status bar context menu to include key equivalents for common actions; updates Xcode project settings to generate Swift asset symbol extensions; adds unit tests for the new formatting behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Pika/AppDelegate.swift | Sets activation policy earlier in app lifecycle based on Defaults[.appMode]. |
| Pika/Extensions/CGFloat+Format.swift | Adds numeric formatting helper used by color formatting. |
| Pika/Extensions/NSColor+Lab.swift | Switches LAB/OKLCH formatting to use stripped decimal strings. |
| Pika/Services/StatusBarController.swift | Rebuilds status bar context menu with key equivalents. |
| PikaTests/CGFloatFormatTests.swift | Adds unit tests covering the new CGFloat formatting helper. |
| PikaTests/NSColorLabTests.swift | Adds OKLCH formatting tests focused on trailing-zero stripping. |
| Pika.xcodeproj/project.pbxproj | Wires in new files and enables asset symbol extension generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- AppDelegate: remove setActivationPolicy from applicationWillFinishLaunching (setupAppMode in applicationDidFinishLaunching sets the correct policy); the early policy change was triggering NSMenu hierarchy consistency warnings - NavigationMenuItems: add .keyboardShortcut() to all action buttons so shortcuts are visible in the in-app gear menu dropdown Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Menu warnings: restoring .prohibited in applicationWillFinishLaunching — removing it didn't fix the warnings (macOS-internal NSMenu consistency check fires during storyboard menu setup, not fixable at this level). In-app keyboard shortcuts: reverting .keyboardShortcut() additions to NavigationMenuItems — SwiftUI registers these as global commands, which (1) requires two clicks for shortcuts to appear due to lazy evaluation, and (2) removes items from the Dock's contextual menu. Both are known SwiftUI on macOS limitations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…penGL float format - Add `import Foundation` to CGFloat+Format.swift for String(format:) - Normalize -0.0 to +0.0 in strippedDecimalString to avoid "-0" output - Fix toOpenGLString to ensure float notation (0.0 not 0) via glFloat helper - Convert Substring to String before calling components(separatedBy:) in tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
Fixed all three review suggestions in e2323d8: added import Foundation to CGFloat+Format.swift, normalized -0.0 in strippedDecimalString to prevent -0 output, updated toOpenGLString with a glFloat helper that ensures float notation (0.0 not 0), and converted Substring to String before .components(separatedBy:) in the tests. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace 3- and 4-member return tuples with named structs (RGBAComponents, HSBComponents, HSLComponents, XYZComponents, LabComponents, OklchComponents) to resolve large_tuple violations without affecting any call sites - Remove redundant trailing_comma disable in Constants.swift (already globally disabled in .swiftlint.yml) - Rename loop variable i to idx in ClosestVector.swift to remove identifier_name disable/enable block - Add explanatory comments to remaining identifier_name and line_length disables that cannot be removed (mathematical single-letter names, unavoidably long strings) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
identifier_name: - NSColor+Hex: rename x → value in roundToHex - NSColor+Init: rename c → char, R/G/B → red/green/blue; targeted disable for r/g/b/a params (public API labels NSColor(r:g:b:a:) that cannot be renamed) - NSColor+Luminance: rename v → val, c → component, L1/L2 → lum1/lum2 line_length: - Break long SwiftUI call sites across lines in ColorPickOverlay, ComplianceToggleGroup, ComplianceButtons, KeyboardShortcutGrid, ContentView, Eyedroppers, ColorPickOverlayWindow function_body_length: - ColorPickOverlayWindow: extract makeInfoPanel, makeCrosshairPanel, positionPanels helpers cyclomatic_complexity: - URLSchemeHandler: delegate each URL action to private handleFormat/Pick/System/Copy methods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…turn types Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
String interpolation cannot span multiple lines in Swift. Removed the unnecessary "\(...)" wrapper since Exporter.toText/toJSON already return String. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consistent comma spacing in lab unformatted style to match oklch. Move closing parentheses to their own lines for multiline function calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
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
.prohibitedactivation policy with computed policy inapplicationWillFinishLaunchingso menus stay attached during launchCGFloat.strippedDecimalString(maxDecimalPlaces:)utility toCGFloat+Format.swift, applied to bothtoOklchStringandtoLabString(e.g.0.5000→0.5,0.0000→0)ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YESto all targetsTest plan
0not0.0000for chroma0.3not0.3000)CGFloatFormatTests,NSColorLabTests)🤖 Generated with Claude Code