Fix reset timestamp truncation in menu card metric rows - #2846
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 5:36 PM ET / 21:36 UTC. ClawSweeper reviewWhat this changesThe PR makes usage-card metric headers keep a single row when the primary metric and reset text fit, then stack them when a localized reset timestamp would crowd the metric. Merge readinessThe final rebased head resolves the prior title-width concern with a fit-based stacked fallback; no actionable correctness or security defect remains. This PR is still necessary because current main retains the truncation-prone single row, and the owner has explicitly requested review of the exact head. Priority: P2 Review scores
Verification
How this fits togetherCodexBar turns provider usage data into menu-bar cards. A metric row renders the primary quota label, optional reset time, and progress bar that users use to understand account limits. flowchart LR
A[Provider usage data] --> B[Usage card model]
B --> C[Metric header]
C --> D{Both labels fit?}
D -->|Yes| E[Single-line header]
D -->|No| F[Stacked compact header]
E --> G[Menu card and progress bar]
F --> G
Before merge
Agent review detailsSecurityNone. Review metricsNone. Technical reviewBest possible solution: Land the responsive header and its standard-width regression coverage once the required checks complete, preserving the existing compact one-row appearance for short content. Do we have a high-confidence way to reproduce the issue? Yes—current main's residual-width reset label can be exercised at the standard card width with a long localized reset string; the PR also supplies signed before/after captures and a focused layout test. The review did not execute tests because this is a read-only checkout review. Is this the best way to solve the issue? Yes—the fit-based one-row-or-stacked layout is a narrow maintainable fix that preserves short-content behavior and avoids language-specific width heuristics. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b43ba3a02c4e. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
a3296d1 to
dce6bfb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dce6bfb8b6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .foregroundStyle(MenuHighlightStyle.secondary(self.isHighlighted)) | ||
| .lineLimit(2) | ||
| .multilineTextAlignment(.trailing) | ||
| .layoutPriority(1) |
There was a problem hiding this comment.
Reserve width for the metric title
When the reset label is long—such as an absolute date in German/Spanish or a provider-supplied reset description—giving it the sole higher layout priority lets it claim nearly the entire standard 296-point card row before presentation.titleText is measured. The metric name and percentage can consequently collapse to an ellipsis, replacing the original reset-label truncation with loss of the primary usage value; use a balanced layout or reserve a minimum width for the title while still allowing the reset text to wrap.
Useful? React with 👍 / 👎.
dce6bfb to
4728b2f
Compare
|
@clawsweeper re-review The reset-only priority finding is addressed at head |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
4728b2f to
46ebfbe
Compare
|
@clawsweeper re-review Rebased onto current |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Merged as Proof:
Signed visual proof used the actual The original reset-only priority approach was replaced after review showed it could truncate the primary metric. The final fit-based single-row/stacked fallback preserves both values without hardcoded localization widths. All temporary proof scaffolding was removed before publication. |
Summary
Note