Skip to content

Split ruler defaults from live settings#241

Closed
pascalpp wants to merge 5 commits into
codex/231-primary-grouped-ruler-uifrom
codex/232-ruler-settings-split
Closed

Split ruler defaults from live settings#241
pascalpp wants to merge 5 commits into
codex/231-primary-grouped-ruler-uifrom
codex/232-ruler-settings-split

Conversation

@pascalpp

Copy link
Copy Markdown
Owner

Implements #232 as part of #228. Threads per-ruler settings through grouped windows and rule views so Preferences edits defaults for future rulers without mutating existing managed rulers.

@pascalpp

Copy link
Copy Markdown
Owner Author

[codex reviewer] Two issues I would fix in this phase:

  1. Active ruler background opacity can leave the foreground window faded. In RulerSettingsController.setBackgroundOpacity (Free Ruler/PreferencesController.swift:441) the controller writes settings.backgroundOpacity, then sets rulerController?.opacity to the background value. Unlike the global Preferences flow, the ruler settings sheet does not post the preferences-window closed notification or otherwise restore foreground opacity in windowWillClose (Free Ruler/PreferencesController.swift:424). Repro: open Ruler Settings for a visible ruler, move Background Opacity to 5%, close the sheet. The active ruler stays at 5% alpha until app activation changes it. I would reset to state.settings.foregroundOpacity when the sheet closes, or only preview background opacity while the app is actually backgrounded. Please add a regression test around closing the settings controller after changing the background slider.

  2. The new static Ruler Settings... menu item is only added to Base.lproj/MainMenu.xib (Free Ruler/Base.lproj/MainMenu.xib:107). I do not see matching rSt-Tg-232.title entries in de/es/fi/ja/zh-hans.lproj/MainMenu.strings, and the string catalog entry for the settings window title will not localize a XIB menu title. Non-English menus will show the English label. Please add the XIB localization entries or set this title programmatically with NSLocalizedString.

@pascalpp pascalpp force-pushed the codex/232-ruler-settings-split branch from 1a86c3b to 3e77e84 Compare June 19, 2026 05:47
@pascalpp pascalpp force-pushed the codex/231-primary-grouped-ruler-ui branch from 1c7e507 to 4b03917 Compare June 19, 2026 05:47

pascalpp commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

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.

1 participant