fix(models): normalize mobile tab spacing and stabilize Combos layout - #3374
fix(models): normalize mobile tab spacing and stabilize Combos layout#3374lidge-jun wants to merge 4 commits into
Conversation
Carried from #3333 onto current dev. Independent of the other carried fixes, so it ships as its own PR rather than a stack layer. Co-authored-by: hajune <june@smartix.co.kr>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
Warning Review limit reachedNext included review available in 9 seconds. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes stabilize Models and Combos layout sizing, spacing, alignment, and fallback states across desktop and mobile viewports. They also adjust log detail controls and table-cell overflow behavior, with CSS regression coverage for Models tab layouts. ChangesModels and Combos layout
Log content layout
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change stabilizes Models and Combos sizing, alignment, and fallback presentation without any identified current-head merge risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The PR satisfies the coding objectives in issue Full details: Out of Scope Changes checkExplanation Most changes are within scope, but gui/src/styles.css also changes log styling: Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (3 skipped: 3 unsupported.) Full details: Title checkExplanation The title accurately describes the mobile tab-spacing updates and Combos layout stabilization. It does not mention the Models panel width fixes, but it clearly identifies major changes in the pull request. ✨ Finishing Touches 💡 1📝 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa130a6563
ℹ️ 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".
| zh-TW's 檢視詳細資料 would lose its tail in the 8% status column. */ | ||
| white-space: normal; | ||
| text-align: left; | ||
| white-space: nowrap; |
There was a problem hiding this comment.
When the fixed-layout Logs table contains a long cost, model, reasoning value, or translated details label, this hunk removes the body-cell clipping and reasoning wrapping rules while forcing the button back to nowrap, so content can paint over adjacent columns. The existing gui/tests/logs-table-overflow.test.ts regression test now fails both containment assertions; restore the removed overflow and wrapping declarations.
AGENTS.md reference: gui/AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
| .main-inner.main-inner--combos > .page-sub { margin-bottom: 10px; } | ||
|
|
||
| /* Keep the Models tab strip border aligned with its tab buttons in the full-bleed layout. */ | ||
| .main-inner.main-inner--combos > .page-tabs { |
There was a problem hiding this comment.
When gui/tests/dashboard-tabs.test.ts runs, it locates the first literal .page-tabs { block in this stylesheet; placing this scoped selector before the base .page-tabs rule makes the test inspect this margin-only block and fail because flex-wrap: wrap is absent. Move this scoped declaration below the base rule or update the source-contract test to match the exact selector so the required GUI test gate passes.
AGENTS.md reference: gui/AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
Rendered against the branch stylesheet; the measured widths are read from getBoundingClientRect, not drawn.
리뷰 · 우선순위 58 / 80이 PR은 Models / Combos 탭의 가로 폭이 탭마다, 로딩 상태마다 들쭉날쭉하던 문제를 고치려 합니다. 지금 고치는 방향의 핵심은 좋습니다. 다만 이 브랜치의 그 외에는 types/config 분할과 무관하고, GUI 레이아웃 범위도 본문 설명과 대체로 맞습니다. CSS 소스 계약 테스트라 브라우저 미니파이 결과를 직접 검증하지는 않지만, 선택자 문자열을 잠그는 용도로는 충분합니다. Combos 셸 없는 상태의 모바일 패딩 오버라이드도 같이 들어 있습니다.
원본 #3333 - 머지 후 자동 close 또는 landed-via 정리가 필요합니다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Attribution correction on this carry PR, noted during a backlog review. The The trailer in this description now reads: Before squashing this PR, make sure the merge commit carries that exact trailer rather than the Original work by @blackjune67 in #3333. |
CI caught this: the carried Models/Combos styling adds a scoped
`.main-inner--combos > .page-tabs` rule above the base one, and the dashboard
guard located its target with a bare indexOf(".page-tabs {") — which matches any
rule whose selector merely ENDS in that string. It read the new scoped block,
found no flex-wrap, and failed on a declaration nothing had touched.
Anchoring the search to a line-start .page-tabs rule fixes the guard without
weakening it: removing flex-wrap from the real base rule still fails the test.
CI was right and my earlier read of it was wrong. #3333 predates #3367, so taking gui/src/styles.css wholesale from the PR head silently reverted the newer Logs fix: .logs-table tbody td { overflow: hidden }, the .log-reasoning-cell wrap rule, and the .log-detail-btn wrapping that #3367 added on top. tests/logs-table-overflow.test.ts caught exactly that. This is the hazard in carrying a whole file rather than a diff, and it is why the guard test earned its keep.
CI caught a real regression I introduced while rebuilding this branch: I took the nine i18n catalogs wholesale from the old branch, which silently reverted every key dev had added since — sidebar.preferences among them — and broke the GUI build's TKey union. The catalogs are now restored from dev with only the two device keys added on top. bun run build:gui passes, as do locale parity and fr-localization. Same root cause as the Logs styles revert on #3374: carrying a whole file instead of a diff takes the file's history with it.
…d an account (#3385) * fix(codex,cli,gui): surface the device login so a headless hub can add an account The deviceauth grant landed in #3369 but nothing could reach it: openai/codex/ chatgpt route through the Codex-auth API, which dropped deviceCode from the start DTO and handed every non-empty URL to a local browser. - POST /api/codex-auth/login accepts device: true, returns deviceCode, and skips the browser open when one is present -- the verification page belongs on the user's other machine, and on a headless host the spawn fails anyway. - ocx account login openai --device prints the URL, the code, and the flow id, and keeps them under --no-wait --json. kimi/nous/github-copilot accept the flag as a no-op because their only login is already a device flow. - The Codex modal gets its own "Device code login" row, and a reauth (which skips the pick step) can switch to it from the waiting step. Both poll budgets go to 480 attempts. The grant lives 15 minutes and the point is that the operator walks to another device; the old 5-minute budget would have killed the flow while its code was still valid, and the extra 30 attempts cover the token exchange after the final poll. Rebuilt on current dev after #3369 squash-merged. Refs #3366 * fix(gui): re-add the device strings without reverting newer locale keys CI caught a real regression I introduced while rebuilding this branch: I took the nine i18n catalogs wholesale from the old branch, which silently reverted every key dev had added since — sidebar.preferences among them — and broke the GUI build's TKey union. The catalogs are now restored from dev with only the two device keys added on top. bun run build:gui passes, as do locale parity and fr-localization. Same root cause as the Logs styles revert on #3374: carrying a whole file instead of a diff takes the file's history with it. --------- Co-authored-by: jun <jun@lidge.dev>
lidge-jun
added
the
landed-via-maintainer
1 participant
Summary
Carried from #3333 by @blackjune67 onto current
dev, with the author'sCo-authored-bytrailer on the commit.The Models page keeps its panels mounted and toggles
hidden, so width rules written against panel contents behaved differently depending on what had loaded and what had been visited. Two concrete consequences:.models-workspace-shell, which is absent during the loading skeleton and after a cold failure. The tab rendered at 980px and snapped to 1200px once data arrived.Scoping to
#models-panel-catalog:not([hidden])and#models-panel-routing:not([hidden])holds the width across load, empty, and error, and cannot leak to another tab.Widths in that capture are read from
getBoundingClientRect()against this branch's stylesheet, not annotations.Separately, the Combos page renders its loading and error states without the workspace shell, so full-bleed styling applied to them produced the wrong flow; that is now limited to the actual workspace, with boxed fallback restored.
Closes #3333
Verification
RED without the patch: the asserted selectors do not exist on current
dev. Worth stating plainly for the reviewer — this is a CSS-source contract test, so it locks the declarations rather than the rendered pixels; the capture above is the rendered evidence.Checklist
Co-authored-by: blackjune67 46661504+blackjune67@users.noreply.github.com