Repository navigation
feat(menubar): finish opt-in provider colors - #4321
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. Reviewed October 7, 2026, 10:42 PM ET / October 8, 2026, 02:42 UTC (Revision 4). ClawSweeper reviewWhat this changesAdds a default-off Color by provider preference across existing menu-bar styles, with monochrome fallbacks for tracking, stale data, and accessibility states. Example: Enable Color by provider for Claude
Review scores
ProductKind: Feature · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 2 items remain Keep open; this PR needs real behavior proof before merge. Earlier functional findings are addressed and completed regression coverage is now recorded, but the supplied screenshot still demonstrates only offscreen rendering. Priority: P3 Before merge
FindingsNone. Provenance
Tests
Agent review detailsHow this fits togetherCodexBar displays provider usage in native macOS menu-bar items. Saved preferences, provider accents, and usage snapshots determine the icons, percentages, and spoken accessibility labels. flowchart TD
A[Saved display preferences] --> C[Menu-bar controller]
B[Provider usage and accent] --> C
C --> D[Appearance and tracking checks]
D --> E{Provider color allowed?}
E -->|Yes| F[Colored native content]
E -->|No| G[Monochrome native content]
Technical reviewBest possible solution: Retain the shared optional tint path and establish its settings, native menu tracking, and accessibility behavior in a freshly built application. Do we have a high-confidence way to reproduce the issue? Not applicable to a new appearance capability; the inspected screenshot establishes offscreen colored output, while native application behavior remains unproven. Is this the best way to solve the issue? Yes at the implementation level: the shared renderer and existing accent palette provide one color source while preserving layouts and defaults. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 844b0e19bbbb. Provenance checked
TestingProof path: in-process harness. Added test files: 45. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (3 earlier review cycles)
|
Keep Color by provider as one default-off preference across the existing styles and stacked provider rows. Share the tint path and preserve pace colors and spoken labels. Repair menu tracking recovery, system contrast handling, and contrast checks for light, dark, and vibrant appearances. Retain the contributor history, add pixel and lifecycle regressions, localize the single toggle, and document its conservative fallback policy. Refs steipete#4321. Co-authored-by: David Aronchick <aronchick@gmail.com>
|
Thanks @aronchick! Merged in fe95bfd with your commits preserved. Color by provider ships as a single opt-in toggle on the menu bar layout (default off), tinting each provider slot with its brand colour on top of the active layout and icon style; the maintainer follow-ups made VoiceOver labels independent of colour, repaired the menu-close fallback, and fixed the contrast failure with rendered-colour regressions for both appearances. Ships in the next release. |
Summary
Add a single Color by provider toggle to the existing menu bar settings. It defaults off and overlays the selected Critters, Meter bars, or Icon & percent style, including each provider's own stacked row. Saved layouts, the existing style picker, and Usage & Spend's Brand/Monochrome artwork remain unchanged.
Provider colors fall back to monochrome during menu tracking, stale data, system Increase Contrast, and the inactive-display contrast preference. Menu close restores color immediately. Accents must pass a 2:1 threshold against conservative reference backgrounds: 25% sRGB gray in dark appearance and 85% in light appearance. This does not sample wallpaper or claim WCAG text conformance. Pace colors keep their separate meaning, and VoiceOver labels do not depend on color.
The maintainer follow-up preserves @aronchick's commits, simplifies the duplicated tint logic, covers standard and vibrant AppKit appearances, adds rendered-pixel and lifecycle regressions, and localizes the one toggle label in every catalog. Thanks @aronchick!
Verification
All Swift runs use
source Scripts/test_environment.shand the native backend.swift test --build-system native --jobs 4 -Xswiftc -gnone --no-parallel --filter 'MenuBarProviderColorTests|MenuBarLayoutRendererTests|MergedIconPresentationTests|MenuBarLayoutDisplayOptionsTests|MenuBarPaceColorSettingsTests|SettingsStoreMergeIconStackedTests|StatusMenuAppearanceTests|StatusItemAnimationSignatureTests|PreferencesDocumentTests|LocalizationLanguageCatalogTests|ProviderIconResourcesTests': 192 tests in 12 suites passed.vibrantLightand actual accessibility appearance names before the classifier repair.make check: 0 violations, 0 serious, 2,844 Swift files, with repository checks passing. An unchanged process-cleanup fixture timed out on the shared host during an earlier attempt; the retry passed.swift test --build-system native --jobs 4 -Xswiftc -gnone --no-parallel --filter ProviderArchitectureGatekeeperTests: 48 tests passed. The extracted Warp blink helper retains the same behavior; its exact gatekeeper anchor is updated.CODEXBAR_TEST_SUITE_TIMEOUT=600 ./Scripts/test.sh --swift-command <native-wrapper> --direct-workers 4: full inventory of 1,592 selections / 144 groups, with 14,080 discovered methods verified against the direct runtime. The native wrapper forwardstestandbuildto Swift with--build-system native --jobs 4 -Xswiftc -gnone.swift test --skip-build --build-system native --jobs 4 -Xswiftc -gnone --no-parallel --filter KiroStatusProbeTeststhen passed 57 tests.Scripts/direct_swift_test_groups.pyafter revalidating the same complete runtime inventory: 49 groups / 546 selections passed, with 0 failures, 0 retries, 0 timeouts. All original selections are covered on the final head, with 0 unresolved failures. This is aggregate full-suite proof, not a claim that the initial fail-fast command exited successfully.Synthetic rendered proof
Real renderer and AppKit drawing with fixture values, including pixel assertions in light/dark and fallback cases. This is offscreen proof, not a desktop capture or a live-account/app relaunch.
Refs #3533
Refs #3628
Refs #4294