fix(ui): uniform control heights across settings clusters#106898
Conversation
Inputs (41px), buttons (30-38px), and segmented switches (33px) sat at mixed heights inside the same settings rows and toolbars. All controls in settings clusters now share --settings-control-height (32px), and the eight remaining native selects (cron filter popover, model-provider default/fallback pickers, usage session sort) adopt .settings-select. Row control clusters wrap only at mobile widths where they have a definite width (Chromium collapses wrappable shrink-to-fit flex containers to their widest item).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fabca8342d
ℹ️ 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".
| <label class="field"> | ||
| <span>${t("modelProviders.add.provider")}</span> | ||
| <select | ||
| class="settings-select" |
There was a problem hiding this comment.
Keep add-provider fields at one height
When the Add provider form is open, this select sits in .model-providers__add-form beside the API-key <input> and Save button, but the new settings-select rule forces the select to --settings-control-height (32px) while the neighboring .field input still keeps the generic 38px height from components.css. That leaves this grid row with the same mixed-height controls this change is trying to eliminate; either size the adjacent input/button through the settings-control rules too or leave this select on the generic field baseline.
Useful? React with 👍 / 👎.
|
Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 7:39 PM ET / 23:39 UTC. Summary PR surface: Source +60. Total +60 across 6 files. Reproducibility: not applicable. as a functional bug reproduction: the uneven heights are directly demonstrated by the supplied before images and DOM audit measurements rather than a broken runtime workflow. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the single-token, class-only styling follow-up after the final merge-base refresh and UI checks confirm the representative desktop and mobile settings clusters remain aligned and usable. Do we have a high-confidence way to reproduce the issue? Not applicable as a functional bug reproduction: the uneven heights are directly demonstrated by the supplied before images and DOM audit measurements rather than a broken runtime workflow. Is this the best way to solve the issue? Yes. Centralizing the height in the existing settings design-language stylesheet and applying the existing select class is narrower and less drift-prone than page-specific sizing overrides. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9e2a6b126900. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +60. Total +60 across 6 files. View PR surface stats
What 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
|
…aw#106898) Inputs (41px), buttons (30-38px), and segmented switches (33px) sat at mixed heights inside the same settings rows and toolbars. All controls in settings clusters now share --settings-control-height (32px), and the eight remaining native selects (cron filter popover, model-provider default/fallback pickers, usage session sort) adopt .settings-select. Row control clusters wrap only at mobile widths where they have a definite width (Chromium collapses wrappable shrink-to-fit flex containers to their widest item).
…aw#106898) Inputs (41px), buttons (30-38px), and segmented switches (33px) sat at mixed heights inside the same settings rows and toolbars. All controls in settings clusters now share --settings-control-height (32px), and the eight remaining native selects (cron filter popover, model-provider default/fallback pickers, usage session sort) adopt .settings-select. Row control clusters wrap only at mobile widths where they have a definite width (Chromium collapses wrappable shrink-to-fit flex containers to their widest item).
What Problem This Solves
Second polish pass on the settings design language (#106470): controls sitting in the same row or toolbar had mixed heights — text inputs rendered 41px, default buttons 38px, small buttons 33px, icon buttons 30–36px, segmented switches 33px — and eight selects (cron filter popover, model-provider default/fallback pickers, usage session sort) still rendered with the generic
.fieldbaseline instead of the design-language control style.Why This Change Was Made
A DOM audit script measured every control cluster across all 22 settings routes and flagged each cluster whose controls differed by >2px, plus every native-styled form control. Fixes, all at the design-language layer:
--settings-control-height: 32pxtoken; buttons, inputs, selects, and segmented switches inside.settings-row__control,.settings-section__actions, and the cron toolbar all share it. Icon buttons become 32px squares.min-heightso many-option enums can still wrap)..settings-select.User Impact
Every settings row and toolbar now shows one flush control height; the cron filter popover, model-provider pickers, and usage sort match the rest of the design language. CSS + class-only changes, no behavior changes.
Evidence
node scripts/run-vitest.mjson touched pages (cron, model-providers, usage): 13 files / 129 tests green.gpt-5.6-sol, xhigh) on the branch diff: clean, no findings.