Improve usage detail layout#2182
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcb293fca1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| (current.detailLeftText == nil) == (candidate.detailLeftText == nil) && | ||
| (current.detailRightText == nil) == (candidate.detailRightText == nil) && | ||
| (current.detailRightSecondaryText == nil) == (candidate.detailRightSecondaryText == nil) && |
There was a problem hiding this comment.
Re-measure when metric detail text changes rows
With the new MetricDetailRow, rows without detailRightSecondaryText can change height based on the actual detail strings because ViewThatFits falls back from one row to two rows when the text no longer fits. This compatibility check still only compares whether the detail fields are nil, so an open live card can accept a refresh that changes detailLeftText/detailRightText from short to long as the same layout; the existing NSMenuItem height is then not rebuilt/re-measured and the new second row can be clipped until the menu is reopened. Reject compatibility or compare enough text shape to cover the one-row/two-row transition.
Useful? React with 👍 / 👎.
Split usage pace ETA and run-out risk across two rows. Show all reset-credit expiries with consistent compact dates. Keep Preferences, accessibility, and height fingerprints aligned.
fcb293f to
6aadf0d
Compare
Reject live metric updates when adaptive detail text can change row count. Add regression coverage for short-to-long detail transitions.
Normalize numeric detail changes to stable text shapes so refreshes do not retain stale data. Preserve remeasurement for wording changes and cover compatible text shapes with a regression test. Focused tests and make check pass. Full make test remains blocked by existing Claude credentials test failures.
|
Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 2:35 AM ET / 06:35 UTC. Summary Reproducibility: not applicable. This PR is a presentation improvement rather than a report of broken established behavior. The supplied screenshot and layout tests provide a concrete way to inspect the proposed UI result. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Merge the shared layout treatment only after a maintainer confirms the menu and Preferences presentation remains desirable at normal and constrained menu widths. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR is a presentation improvement rather than a report of broken established behavior. The supplied screenshot and layout tests provide a concrete way to inspect the proposed UI result. Is this the best way to solve the issue? Yes. Centralizing the adaptive detail behavior in AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f8636cb37eb0. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
…-layout # Conflicts: # Sources/CodexBar/MenuCardHeightFingerprint.swift # Sources/CodexBar/MenuCardView.swift
Summary
Tests
Screenshots