Add customizable browser omnibar font size - #5407
austinywang wants to merge 3 commits into
Conversation
The browser omnibar (address bar) had a hardcoded 12pt URL font and fixed button sizes with no way to enlarge it. Add a browser.omnibarFontSize setting (default 12, range 10-24) that scales the address bar as a unit: the URL field font, the back/forward/reload and toolbar buttons, the secure badge, and the bar height. The tab strip is unaffected. Wired through the markdown.fontSize precedent: live runtime scaling in BrowserPanelView via @AppStorage, a Settings > Browser stepper, cmux.json parsing with range validation, template plus supported paths, the package catalog key, search index/aliases/curated entries, the config schema, and en/ja localization. Two parse tests cover a valid value and out-of-range rejection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds a configurable omnibar font size setting to the browser. Users can adjust the address-bar text and icon sizes between 10–24 points via a stepper control in settings, with proportional scaling applied to related UI metrics throughout the address bar. ChangesOmnibar font size configuration
Sequence Diagram (high-level): sequenceDiagram
participant SettingsUI
participant BrowserPanelView
participant BrowserOmnibarFontSizeSettings
participant OmnibarTextFieldRepresentable
participant NSTextField
SettingsUI->>BrowserPanelView: user adjusts omnibar font size (Stepper)
BrowserPanelView->>BrowserOmnibarFontSizeSettings: setDefault(pointSize)
BrowserOmnibarFontSizeSettings-->>BrowserPanelView: persisted pointSize / scale(forPointSize:)
BrowserPanelView->>OmnibarTextFieldRepresentable: pass fontSize
OmnibarTextFieldRepresentable->>NSTextField: initialize/update font
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs:
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3c13ac7. Configure here.
Greptile SummaryAdds a
Confidence Score: 5/5Safe to merge. Default 12 pt preserves the existing hardcoded layout; all new code paths are gated on a user-configured value in the validated 10–24 pt range. The change is a well-scoped display preference that follows an established pattern end-to-end. The parser validates and rejects out-of-range values before they reach UserDefaults; @AppStorage provides reactive SwiftUI updates; and the live field-editor update in updateNSView is correct AppKit practice. Localization is complete for all supported locales (en and ja). No actor isolation, concurrency, or state mutation issues were introduced. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Register omnibar-font-size row path in S..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Stepper( | ||
| value: Binding(get: { omnibarFontSize.current }, set: { omnibarFontSize.set($0) }), | ||
| in: 10...24 | ||
| ) { |
There was a problem hiding this comment.
10...24 rather than derived from BrowserOmnibarFontSizeSettings constants. The markdown analog (MarkdownTypographyControl) can reference MarkdownFontSizeSettings.minimumPointSize...MarkdownFontSizeSettings.maximumPointSize directly because both files live in the app target. Here the package boundary prevents that, so if the min/max constants change in BrowserOmnibarFontSizeSettings the stepper silently drifts — the UI would accept values the runtime rejects (or vice versa). Consider mirroring the constants on the BrowserCatalogSection DefaultsKey (or a small extension) so the package owns the authoritative bounds that both this stepper and the schema reference.
| Stepper( | |
| value: Binding(get: { omnibarFontSize.current }, set: { omnibarFontSize.set($0) }), | |
| in: 10...24 | |
| ) { | |
| Stepper( | |
| value: Binding(get: { omnibarFontSize.current }, set: { omnibarFontSize.set($0) }), | |
| in: 10...24 // Must stay in sync with BrowserOmnibarFontSizeSettings.minimumPointSize/maximumPointSize | |
| ) { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/CmuxSettings/Sources/CmuxSettings/Keys/BrowserCatalogSection.swift`:
- Around line 29-36: The DefaultsKey declaration omnibarFontSize currently uses
a dotted userDefaultsKey matching its id; change the userDefaultsKey value to
the camelCase form "browserOmnibarFontSize" to match the naming pattern used by
other keys in this file (e.g., browserSearchEngine,
browserCustomSearchEngineName) and update any runtime readers (such as
BrowserOmnibarFontSizeSettings or browser panel consumers) to read the camelCase
key if they are still expecting the dotted form.
In `@web/data/cmux.schema.json`:
- Around line 949-955: Add a localized descriptionKey for the omnibarFontSize
schema entry: in the omnibarFontSize object (key "omnibarFontSize") add a
"descriptionKey" property (for example "browser.omnibarFontSize.description")
instead of inline English, and then add that key and its localized text to the
web messages JSON for all supported locales so the schema description is served
via localization rather than hardcoded English.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 111df509-3542-41bc-a2d8-4e39022a1ad0
📒 Files selected for processing (13)
Packages/CmuxSettings/Sources/CmuxSettings/Keys/BrowserCatalogSection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swiftResources/Localizable.xcstringsSources/CmuxSettingsJSONPathSupport.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftcmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swiftweb/data/cmux.schema.json
| /// Font size, in points, for the omnibar (address bar) URL field, | ||
| /// navigation buttons, and toolbar icons. The tab strip is unaffected. | ||
| /// Mirrors the app-target `BrowserOmnibarFontSizeSettings` (default 12). | ||
| public let omnibarFontSize = DefaultsKey<Int>( | ||
| id: "browser.omnibarFontSize", | ||
| defaultValue: 12, | ||
| userDefaultsKey: "browser.omnibarFontSize" | ||
| ) |
There was a problem hiding this comment.
Inconsistent userDefaultsKey naming pattern.
All other browser settings in this file use camelCase userDefaultsKey values without the browser. prefix:
- Line 8:
userDefaultsKey: "browserSearchEngine"(forid: "browser.defaultSearchEngine") - Line 14:
userDefaultsKey: "browserCustomSearchEngineName" - Line 26:
userDefaultsKey: "browserSearchSuggestionsEnabled"
The new omnibarFontSize setting uses userDefaultsKey: "browser.omnibarFontSize" (dotted, same as the id), breaking the established pattern. To maintain consistency with the rest of this file, the key should be "browserOmnibarFontSize" (camelCase, no prefix).
Verify that BrowserOmnibarFontSizeSettings and other runtime readers in the browser panel layer expect the camelCase key.
Proposed fix
public let omnibarFontSize = DefaultsKey<Int>(
id: "browser.omnibarFontSize",
defaultValue: 12,
- userDefaultsKey: "browser.omnibarFontSize"
+ userDefaultsKey: "browserOmnibarFontSize"
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/CmuxSettings/Sources/CmuxSettings/Keys/BrowserCatalogSection.swift`
around lines 29 - 36, The DefaultsKey declaration omnibarFontSize currently uses
a dotted userDefaultsKey matching its id; change the userDefaultsKey value to
the camelCase form "browserOmnibarFontSize" to match the naming pattern used by
other keys in this file (e.g., browserSearchEngine,
browserCustomSearchEngineName) and update any runtime readers (such as
BrowserOmnibarFontSizeSettings or browser panel consumers) to read the camelCase
key if they are still expecting the dotted form.
| "omnibarFontSize": { | ||
| "type": "integer", | ||
| "minimum": 10, | ||
| "maximum": 24, | ||
| "default": 12, | ||
| "description": "Font size, in points, for the browser omnibar (address bar): the URL field, navigation buttons, and toolbar icons. The tab strip is unaffected." | ||
| }, |
There was a problem hiding this comment.
Add a localized descriptionKey for browser.omnibarFontSize.
This introduces new user-facing schema copy as inline English only. For web/data/**/*.json, new schema descriptions should be keyed so all supported web locales can translate them consistently.
Suggested patch
"omnibarFontSize": {
"type": "integer",
"minimum": 10,
"maximum": 24,
"default": 12,
- "description": "Font size, in points, for the browser omnibar (address bar): the URL field, navigation buttons, and toolbar icons. The tab strip is unaffected."
+ "descriptionKey": "schemaDescriptions.browser.omnibarFontSize",
+ "description": "Font size, in points, for the browser omnibar (address bar): the URL field, navigation buttons, and toolbar icons. The tab strip is unaffected."
},As per coding guidelines, web/data/**/*.json user-facing schema text must have locale coverage for all supported locales. Based on learnings, descriptionKey in schema is web-doc metadata and should be localized via web messages, not xcstrings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/data/cmux.schema.json` around lines 949 - 955, Add a localized
descriptionKey for the omnibarFontSize schema entry: in the omnibarFontSize
object (key "omnibarFontSize") add a "descriptionKey" property (for example
"browser.omnibarFontSize.description") instead of inline English, and then add
that key and its localized text to the web messages JSON for all supported
locales so the schema description is served via localization rather than
hardcoded English.
Sources: Coding guidelines, Learnings
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 13 files
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Panels/BrowserPanelView.swift">
<violation number="1" location="Sources/Panels/BrowserPanelView.swift:501">
P2: The in-progress downloading label in the address bar still uses a fixed font size and doesn't scale with `omnibarScale`, unlike all other omnibar elements (nav icons, secure badge, URL field). At larger omnibar font sizes the download label will appear disproportionately small.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -436,6 +436,7 @@ struct BrowserPanelView: View { | |||
| @AppStorage(BrowserImportHintSettings.variantKey) private var browserImportHintVariantRaw = BrowserImportHintSettings.defaultVariant.rawValue | |||
There was a problem hiding this comment.
P2: The in-progress downloading label in the address bar still uses a fixed font size and doesn't scale with omnibarScale, unlike all other omnibar elements (nav icons, secure badge, URL field). At larger omnibar font sizes the download label will appear disproportionately small.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Panels/BrowserPanelView.swift, line 501:
<comment>The in-progress downloading label in the address bar still uses a fixed font size and doesn't scale with `omnibarScale`, unlike all other omnibar elements (nav icons, secure badge, URL field). At larger omnibar font sizes the download label will appear disproportionately small.</comment>
<file context>
@@ -473,10 +474,31 @@ struct BrowserPanelView: View {
+ private var omnibarFieldHeight: CGFloat { (18 * omnibarScale).rounded() }
+
+ /// Font size for the back/forward/reload chevrons, scaled from 12pt.
+ private var omnibarNavIconSize: CGFloat { 12 * omnibarScale }
init(
</file context>
The new required fontSize parameter broke the test target's direct construction of OmnibarTextFieldRepresentable (reload.sh only builds the cmux scheme, so the cmuxTests compile error only surfaced in CI). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
everyCuratedSettingEntryIsReachable() requires each curated setting entry to be scroll/highlight reachable via a row whose configurationReview path resolves to it. The new browser.omnibarFontSize row needs its path in the test's rowConfigPaths mirror. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
The cmux browser omnibar (address bar) had a hardcoded 12pt URL font and fixed-size buttons, with no way to make it bigger. This adds a
browser.omnibarFontSizesetting that scales the address bar as a unit: the URL field font, the back/forward/reload and toolbar buttons, the secure badge, and the bar height. The tab strip above it is unchanged.Where to set it:
cmux.json:browser.omnibarFontSize(validated; out-of-range values are rejected)Default is 12, matching the previous hardcoded size, so the omnibar looks identical until you raise it. Changes apply live to open browser panels, including the active field editor while the omnibar is focused.
Implementation mirrors the existing
markdown.fontSizesetting end to end (catalog key, Settings stepper,cmux.jsonparse, template, supported paths, search index/aliases, schema, en/ja localization). Two parse tests inKeyboardShortcutSettingsFileStoreStartupTestscover a valid value and out-of-range rejection.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
UI and preference plumbing only; default 12 preserves prior layout, with validated config parsing and focused tests.
Overview
Adds
browser.omnibarFontSize(10–24 pt, default 12) so users can scale the embedded browser address bar as one unit: URL text, back/forward/reload icons, secure badge, toolbar/dev-tools sizing, and bar height. The tab strip is unchanged.Settings & config: New catalog key, Browser settings stepper, search/anchor wiring, en/ja strings,
cmux.jsontemplate + parser (clamp/round; invalid/out-of-range rejected), supported JSON paths, andcmux.schema.json.Runtime:
BrowserOmnibarFontSizeSettingsplusBrowserPanelView@AppStorageand proportionalomnibarScalemetrics;OmnibarTextFieldRepresentabletakesfontSizeand updates the field editor live while focused.Tests: Settings-file apply/reject tests; omnibar test coordinator updated with
fontSize: 12.Reviewed by Cursor Bugbot for commit 56895af. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Add a customizable
browser.omnibarFontSize(10–24 pt, default 12) that scales the browser address bar for better readability. The URL font, nav/reload and toolbar icons, secure badge, and bar height scale together; the tab strip is unchanged.New Features
cmux.jsonviabrowser.omnibarFontSize.@AppStorage, including while the omnibar is focused.cmux.jsonparsing validates range and rounds; out-of-range values are rejected.Bug Fixes
fontSizetoOmnibarTextFieldRepresentableinOmnibarAndToolsTests, and registered thebrowser.omnibarFontSizerow path inSettingsRowAnchorResolutionTestsso the curated entry is reachable.Written for commit 56895af. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization
Settings & Search
Tests