Skip to content

Add custom color setting#188

Merged
pascalpp merged 27 commits into
mainfrom
pascal/186-ruler-color
Jun 12, 2026
Merged

Add custom color setting#188
pascalpp merged 27 commits into
mainfrom
pascal/186-ruler-color

Conversation

@pascalpp

@pascalpp pascalpp commented Jun 12, 2026

Copy link
Copy Markdown
Owner
  • Add customizable ruler color preference
  • Add reset button for ruler color
  • Update PreferencesController.xib
  • Update RuleView.swift
  • Update prefs XIB layout and color mixing
  • Normalize selected ruler colors to device RGB with opaque alpha so opacity remains controlled by the opacity sliders
  • Add/adjust unit test coverage for ruler color normalization behavior

pascalpp added 5 commits June 11, 2026 23:43
Introduce a user-configurable ruler color and wire it through UI, prefs, rendering, and tests.

- Preferences UI: add a NSColorWell to PreferencesController.xib and expose outlet/action in PreferencesController to update prefs continuously.
- Prefs: store rulerColor as an NSColor (archived/unarchived via keyed archiver), provide a default color and a defaultRulerFillColor accessor, and persist changes to UserDefaults.
- AppDelegate: observe rulerColor changes and trigger ruler redraws; initialize prefs with defaultRulerFillColor.
- RuleView: use prefs.rulerColor for fill and derive contrasting numbers/ticks/mouse/handle colors; add redrawForPreferenceChange and NSColor helpers (isLightColor, mixed) for color math.
- Tests: add unit tests verifying default fill color and contrasting foreground derivation, plus color/luminance helpers.
- Localization: add localized title for the new "Ruler Color" label in several .strings files.

These changes enable users to pick a custom ruler color that is persisted and used to compute readable foreground/tick colors, with UI and tests updated accordingly.
Add a dedicated reset control and logic to restore the default ruler color. UI: preferences XIB now includes a reset button next to the color well, updated constraints/frames, and sets the window restorable attribute to NO. Localization: add "Reset ruler color" strings for multiple locales. Code: expose the reset button outlet, wire a resetRulerColor: IBAction that sets prefs.rulerColor to Prefs.defaultRulerFillColor, configure the button appearance/tool tip/accessibility (uses SF Symbol arrow.counterclockwise), make the color well the initial responder, and hide the reset button when the current color matches the default via a colorsMatch helper.
Adjust PreferencesController.xib button position and constraints (mark button as misplaced, tweak frame and leading constraint constant) to correct layout. Update RuleView.contrastingColor to store the mixed color in a variable and only apply boosted saturation for light fills, avoiding oversaturation for dark colors and simplifying the return logic.
Copilot AI review requested due to automatic review settings June 12, 2026 05:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a user-configurable ruler fill color preference (with reset) and updates the ruler rendering to derive contrasting foreground/handle colors from the chosen fill. This fits into the existing Preferences UI + Prefs KVO bridge pattern and updates ruler redraw behavior when preferences change.

Changes:

  • Add Prefs.rulerColor persistence, preferences UI controls (color well + reset), and localization strings.
  • Update ruler drawing colors to be derived dynamically from the chosen fill color (with contrast logic).
  • Add unit tests validating default fill color and contrasting foreground behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
FreeRulerTests/RulerCoreTests.swift Adds tests for default fill color and contrast derivation.
Free Ruler/RuleView.swift Switches to preference-driven fill color and computed contrasting colors.
Free Ruler/Prefs.swift Adds rulerColor preference with UserDefaults persistence via archiving.
Free Ruler/PreferencesController.swift Adds color well + reset button wiring and UI behavior.
Free Ruler/Base.lproj/PreferencesController.xib Adds/positions the new color controls in Preferences.
Free Ruler/Localizable.xcstrings Adds localized string for “Reset ruler color”.
Free Ruler/de.lproj/PreferencesController.strings Adds localized “Ruler Color” label.
Free Ruler/es.lproj/PreferencesController.strings Adds localized “Ruler Color” label.
Free Ruler/fi.lproj/PreferencesController.strings Adds localized “Ruler Color” label.
Free Ruler/ja.lproj/PreferencesController.strings Adds localized “Ruler Color” label.
Free Ruler/zh-hans.lproj/PreferencesController.strings Adds localized “Ruler Color” label.
Free Ruler/AppDelegate.swift Redraws rulers when rulerColor changes; resets rulerColor in UI test reset.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Free Ruler/Prefs.swift Outdated
Comment thread Free Ruler/Prefs.swift
Comment thread Free Ruler/RuleView.swift
Comment thread Free Ruler/RuleView.swift
Comment thread Free Ruler/Base.lproj/PreferencesController.xib Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Comment thread Free Ruler/Prefs.swift Outdated
Comment thread Free Ruler/Prefs.swift
Comment thread Free Ruler/Base.lproj/PreferencesController.xib Outdated
Copilot AI review requested due to automatic review settings June 12, 2026 06:08
@pascalpp

Copy link
Copy Markdown
Owner Author

[codex] @copilot please take another look when you have a chance. The currently actionable review comments have been addressed and pushed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Comment thread Free Ruler/Prefs.swift
Comment thread Free Ruler/RuleView.swift
Comment thread FreeRulerTests/RulerCoreTests.swift
Comment thread FreeRulerTests/RulerCoreTests.swift Outdated
Copilot AI review requested due to automatic review settings June 12, 2026 06:18
@pascalpp

Copy link
Copy Markdown
Owner Author

[codex] @copilot please take another look. I addressed the follow-up review comments and pushed the fixes.

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown

[codex] @copilot please take another look when you have a chance. The currently actionable review comments have been addressed and pushed.

The four new review comments have been addressed in commit 1947b50:

  • Prefs.swift:106-111: Added colorsMatch helper that compares colors component-by-component; the rulerColor observer now normalizes the in-memory value to deviceRGB at the time it changes (reassigns before archiving, avoiding a double-persist).
  • RuleView.swift:117-120: redrawForPreferenceChange() now uses setNeedsDisplay(visibleRect) instead of invalidating the full bounds.
  • RulerCoreTests.swift:671-684: assertColor replaced force-unwraps with two separate guard let + XCTFail statements with specific diagnostic messages.
  • RulerCoreTests.swift:686-691: relativeLuminance replaced force-unwrap with guard let + XCTFail, includes file:line: parameters, and returns .nan so downstream assertions also fail informatively.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread Free Ruler/Prefs.swift
Comment thread Free Ruler/PreferencesController.swift Outdated

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown

[codex] @copilot please take another look. I addressed the follow-up review comments and pushed the fixes.

[codex] Addressed in ac4c52b with two follow-ups: ruler colors are now normalized to opaque device-RGB (so opacity stays controlled by the opacity prefs), and PreferencesController now reuses Prefs.colorsMatch to avoid duplicate comparison logic.

Copilot AI review requested due to automatic review settings June 12, 2026 07:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 21 changed files in this pull request and generated 4 comments.

Comment thread FreeRulerTests/RulerCoreTests.swift Outdated
Comment thread FreeRulerTests/RulerCoreTests.swift Outdated
Comment thread FreeRulerTests/RulerCoreTests.swift Outdated
Comment thread FreeRulerTests/RulerCoreTests.swift Outdated
pascalpp added 4 commits June 12, 2026 04:07
Force the app to use opaque (no-alpha) color picking for ruler colors and add a custom RulerColorWell UI.

Changes include:
- Add configureOpaqueColorPicking() and setColorPickingIgnoresAlpha(_:) (uses ObjC runtime to call the deprecated setIgnoresAlpha: switch) to ensure AppKit color wells ignore alpha.
- Introduce RulerColorWell subclass with custom drawing, activation/mouse handling, and forced supportsAlpha = false to keep the color panel opaque.
- Wire opaque color-picking setup into AppDelegate.applicationDidFinishLaunching and PreferencesController lifecycle (showWindow/init), and call configureOpaqueColorPicking at key points.
- Subscribe to NSColorPanel.colorDidChangeNotification to update prefs.rulerColor when the color panel changes, and add cleanup in deinit.
- Add closeRulerColorPanel() helper and ensure NSColorPanel is non-animated and uses continuous updates.

These changes prevent users from selecting/transmitting alpha for ruler fills and keep the color panel behavior consistent across the app.
Ensure archived ruler colors are normalized when loaded by replacing the direct unarchive call with Prefs.rulerFillColor(fromArchivedData:), which returns a normalized color or the default. Add a helper to perform unarchiving + normalization. Also update the Preferences XIB to use the custom RulerColorWell class (Free_Ruler target) for the colorWell so the UI control is backed by the custom subclass.
Add a unit test that archives an NSColor with alpha 0.35 and verifies Prefs.rulerFillColor(fromArchivedData:) returns the same RGB values with alpha normalized to 1. Ensures colors loaded from archived preference data are made fully opaque for ruler fill.
Copilot AI review requested due to automatic review settings June 12, 2026 08:09
@pascalpp

Copy link
Copy Markdown
Owner Author

[codex] @copilot please give another code review. do not make code changes. I addressed the four latest test-cleanup comments in commit 6da2301; the ruler-color tests now restore or remove the persisted rulerColor defaults entry according to its prior state.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 21 changed files in this pull request and generated 1 comment.

Comment thread Free Ruler/AppStoreScreenshotPreview.swift Outdated

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown

[codex] @copilot please give another code review. do not make code changes. I addressed the four latest test-cleanup comments in commit 6da2301; the ruler-color tests now restore or remove the persi...

[codex] The test-cleanup changes in 6da2301 look correct. withRestoredRulerColorPreference properly captures both the in-memory color and whether the rulerColor key existed in the persistent domain before each test, then restores both in defer — including removing the key when it wasn't there to begin with. The fallback branch for a nil bundle identifier is also handled correctly.

One open item from the automated reviewer: AppStoreScreenshotPreview.swift:81-84 — the previewName strings for .screen2 ("Screen 3 - Units") and .screen3 ("Screen 2 - Colors") are numbered contrary to their case names. The output filenames follow the same reversed order intentionally, but the previewName strings (which appear in Xcode preview panes and debug output) could be updated to match the case name number for clarity (e.g., "Screen 2 - Units" and "Screen 3 - Colors") without affecting the output filenames. No changes made per your request.

Copilot AI review requested due to automatic review settings June 12, 2026 08:19
@pascalpp

Copy link
Copy Markdown
Owner Author

[codex] @copilot please give another code review. do not make code changes. I addressed the latest screenshot preview label comment in commit 6fa09bd; .screen2 now reports "Screen 2 - Units" and .screen3 reports "Screen 3 - Colors" while leaving output filenames unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 21 changed files in this pull request and generated 1 comment.

Comment thread Free Ruler/PreferencesController.swift
@pascalpp

Copy link
Copy Markdown
Owner Author

[codex] @copilot please give another code review. do not make code changes. I addressed the Objective-C runtime import comment in commit 200df9e; PreferencesController.swift now imports ObjectiveC.runtime for the runtime APIs used by the color well alpha configuration.

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown

[codex] @copilot please give another code review. do not make code changes. I addressed the latest screenshot preview label comment in commit 6fa09bd; .screen2 now reports "Screen 2 - Units" and `...

[codex] Reviewed commit 6fa09bd. The open automated-review comment about a missing ObjC runtime import is already resolved — import ObjectiveC.runtime is present on line 2 of PreferencesController.swift. The previewName label fix (.screen2 → "Screen 2 - Units", .screen3 → "Screen 3 - Colors") looks correct with output filenames unchanged. No new issues found.

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown

[codex] @copilot please give another code review. do not make code changes. I addressed the Objective-C runtime import comment in commit 200df9e; PreferencesController.swift now imports `Objective...

[codex] Reviewed the latest changes. import ObjectiveC.runtime is correctly present on line 2 of PreferencesController.swift. No new issues found — all prior review comments have been addressed.

@pascalpp pascalpp merged commit 57b2794 into main Jun 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants