Skip to content

fix(models): normalize mobile tab spacing and stabilize Combos layout - #3386

Merged
lidge-jun merged 1 commit into
devfrom
codex/carry-3333-v2
Sep 3, 2026
Merged

fix(models): normalize mobile tab spacing and stabilize Combos layout#3386
lidge-jun merged 1 commit into
devfrom
codex/carry-3333-v2

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Summary

Carried from #3333 by @blackjune67 onto current dev, with the author's Co-authored-by trailer 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:

  • The catalog column gated on .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.
  • Routing had no rule of its own and fell back to 980px — a visible jump on every hop, which only disappeared once the compatibility tab had leaked its own 1200px in.

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.

Models tab width stability before and after

Widths in that capture are read from getBoundingClientRect() against this branch's stylesheet, not annotations.

Combos separately renders its loading and error states without the workspace shell, so full-bleed styling stretched a lone subtitle or retry notice edge to edge while every sibling tab stayed boxed. That is now limited to the actual workspace.

One test change, and it is load-bearing. tests/dashboard-tabs.test.ts located its target with indexOf(".page-tabs {"), which matches any rule whose selector merely ends in that string — including the scoped .main-inner--combos > .page-tabs rule this adds above the base one. It read the wrong block and failed on a declaration nothing had touched. The search is now anchored to a line-start rule; removing flex-wrap from the real base rule still fails it.

Replaces #3374, which was rebuilt after CI caught that copying gui/src/styles.css wholesale silently reverted the Logs clipping fix (#3367) and the sidebar footer rework (#3382). This branch applies the PR's own diff instead.

Closes #3333

Verification

bun run build:gui                                     exit 0
bun run lint:gui                                      clean

cd gui && bun test <22 files that read the touched stylesheets>
  112 pass, 0 fail

That file set is every GUI test that reads styles.css, styles-models-workspace.css, or styles-compatibility-matrix.css — chosen because a stylesheet change's blast radius is not visible from the diff, which is exactly how the first attempt's reverts slipped through.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (Styling only.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Co-authored-by: hajune june@smartix.co.kr

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Models workspace layouts changing width while catalog, routing, compatibility, or combos panels load or display errors.
    • Kept the active Models tab consistently aligned at the expected width when switching between tabs.
    • Improved tab-strip borders and spacing in full-width layouts.
    • Added responsive behavior for tab navigation and fallback loading/error states on smaller screens.
  • Tests

    • Added coverage for Models tab layouts, panel spacing, loading/error states, and responsive width consistency.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 3, 2026 17:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-03T17:56:18.997424Z 0586f92 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 3, 2026
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 6c36b810-ea03-4d0d-8461-00ec97b5b053

📥 Commits

Reviewing files that changed from the base of the PR and between 0586f92 and 5354655.

📒 Files selected for processing (1)
  • gui/src/styles.css

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The Models workspace now keeps consistent widths and spacing across catalog, routing, compatibility, and Combos tabs. Loading and error states use constrained fallback layouts. Desktop and mobile tab alignment have regression coverage.

Changes

Models layout normalization

Layer / File(s) Summary
Panel width routing
gui/src/styles-models-workspace.css, gui/src/styles-compatibility-matrix.css
Catalog, routing, and compatibility panels use the visible panel state to apply a 1200px maximum width during active, loading, and error states.
Combos layout and mobile fallbacks
gui/src/styles.css
The Combos tab strip aligns with its buttons. Shell-free loading and error states use constrained desktop geometry and normalized mobile spacing.
CSS layout regression coverage
gui/tests/models-tab-layout.test.ts
Tests verify tab-strip alignment, fallback containers, panel spacing, and consistent workspace widths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 53546

Models tab panels now maintain consistent desktop and mobile layout across loaded and fallback states, with no current merge-blocking risk identified.

Suggested reviewers: wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: normalized Models tab spacing and stabilized Combos layout. It is concise, specific, and matches the linked issue and stylesheet updates.
Linked Issues check ✅ Passed The changes satisfy the coding objectives in [#3333]. The CSS updates normalize tab-strip spacing and alignment, stabilize Catalog, Routing, and Compatibility widths across panel states, constrain Com…
Out of Scope Changes check ✅ Passed The changed files remain within the scope of [#3333]. The PR modifies Models and Combos layout CSS and adds focused regression tests. No unrelated feature, API, data, or cleanup changes are described.
Full details: Linked Issues check

Explanation

The changes satisfy the coding objectives in [#3333]. The CSS updates normalize tab-strip spacing and alignment, stabilize Catalog, Routing, and Compatibility widths across panel states, constrain Combos full-bleed styling to rendered workspace shells, add mobile fallback rules, and add desktop and mobile layout regression coverage in gui/tests/models-tab-layout.test.ts.

Full details: Docstring Coverage

Explanation

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 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/carry-3333-v2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 66 / 80

이 PR은 Models 페이지의 가로 폭이 탭·로딩 상태에 따라 들쭉날쭉하던 버그를 고칩니다. 원본은 @blackjune67 의 #3333이고, 지금 dev 위에 올린 carry입니다. 닫힌 #3374가 같은 작업을 gui/src/styles.css 통째 복사로 올려서 #3367 Logs 잘림 수정과 #3382 사이드바 오브 작업을 조용히 되돌렸습니다. 이번 브랜치는 그 파일의 자기 diff만 적용합니다.

지금 dev HEAD(e71386434)의 동작을 짧게 말하면 이렇습니다. Models 탭 패널은 한 번 열린 뒤에도 DOM에 남아 있고, 안 보이는 탭만 hidden을 답니다. gui/src/pages/models-tab.tsmodelsPanelDomIdmodels-panel-catalog / models-panel-routing / models-panel-compatibility id를 줍니다. 카탈로그의 넓은 칸 규칙은 gui/src/styles-models-workspace.css에서 .main-inner:has(#models-panel-catalog:not([hidden]) .models-workspace-shell)입니다. 그런데 gui/src/pages/Models.tsx를 보면 찬 로딩은 DataSurfaceSkeleton이고, 찬 실패는 안내문+재시도입니다. 둘 다 .models-workspace-shell이 없습니다. 그래서 카탈로그 탭이 먼저 980px로 보이다가 데이터가 오면 1200px로 튀습니다. 라우팅 탭은 자기 규칙이 없어서 기본 .main-inner 980px로 떨어지고, 호환성 탭은 gui/src/styles-compatibility-matrix.css.main-inner:has(.lab-page)가 한 번 열린 뒤 다른 탭으로 폭을 흘립니다. 패널이 마운트된 채로 남기 때문입니다.

이 PR은 세 탭 모두 보이는 패널 id만 봅니다. #models-panel-catalog:not([hidden]), #models-panel-routing:not([hidden]), #models-panel-compatibility:not([hidden]). 셸이 있든 없든 1200px이 유지되고, 다른 탭으로 새지 않습니다. Combos는 다른 구멍입니다. gui/src/pages/Combos.tsx의 로딩 스켈레톤과 찬 실패는 .combos-workspace-shell 없이 안내문만 그립니다. 그런데 gui/src/App.tsx는 콤보 탭만 골라도 main-inner--combos를 붙여 패딩을 0으로 만들고 칸을 화면 끝까지 늘립니다. 그래서 콤보만 로딩/에러일 때 안내문이 가로로 쭉 늘어집니다. 이 PR은 셸이 없을 때 풀블리드를 끄고 1200px 상자로 되돌립니다. :not(:has(.combos-workspace-shell))를 쉼표로 묶지 않고 선택자를 하나씩 쓴 이유는 Vite Rolldown CSS 압축기가 그런 목록을 깨뜨리기 때문입니다.

테스트도 실무적입니다. gui/tests/dashboard-tabs.test.ts.page-tabs {indexOf로 찾아 왔습니다. 이 PR이 그 문자열로 끝나는 Combos 전용 규칙을 위에 넣으면, 가드가 엉뚱한 블록을 읽고 flex-wrap이 없다고 실패합니다. 검색을 줄 시작 규칙으로 고정한 것은 이 변경이 없으면 깨지는 잠금입니다. 새 gui/tests/models-tab-layout.test.ts는 카탈로그/라우팅/호환성 1200px과 셸 없는 Combos 상자를 잠급니다. types/config 분할과는 무관하고, #3382 대시보드 미니멀 방향과도 겹치지 않습니다. 폭만 고칩니다.

원본 #3333은 아직 열려 있습니다. 머지 후 Landed via #3386landed-via-maintainer로 닫아야 합니다. #3374는 이미 닫혀 있습니다. CI 일부(macos, test shards)는 이 리뷰를 쓰는 동안 아직 대기였습니다. 레이아웃 회귀는 스타일시트 테스트로 많이 잡히지만, 실제 브라우저에서 탭을 오가며 폭이 안 튀는지는 스크린샷이 보여 주는 범위입니다.

gui/src/styles-models-workspace.css (현재 dev) - 카탈로그 폭이 .models-workspace-shell에 묶여 있어 스켈레톤/찬 실패에서 980px로 떨어집니다. 이 PR이 보이는 패널 id만 봅니다.

gui/src/styles-compatibility-matrix.css (현재 dev) - :has(.lab-page)는 로딩 중 폭이 줄고, 한 번 열리면 다른 탭으로 1200px이 샙니다. 이 PR이 #models-panel-compatibility:not([hidden])로 바꿉니다.

gui/src/pages/Combos.tsx 318~336행 - 로딩/찬 실패에 .combos-workspace-shell이 없습니다. 현재 devmain-inner--combos 풀블리드가 그 안내문을 화면 끝까지 늘립니다. 이 PR이 셸 없는 상태를 상자로 되돌립니다.

gui/tests/dashboard-tabs.test.ts - indexOf(".page-tabs {")는 선택자 끝이 같은 규칙을 집어 갑니다. 줄 시작 앵커가 이 PR의 Combos 규칙과 같이 있어야 가드가 살아 있습니다.

원본 #3333 - 머지 후 Landed via #3386 + landed-via-maintainer로 닫아야 합니다. 같은 제목의 열린 PR을 하나 더 두면 기여자가 헷갈립니다.

메인테이너의 판단이 필요한 지점

  • Combos 셸 없는 상태를 1200px(Models 탭과 맞춤)로 둘지, 기본 980px로 둘지. 이 PR은 1200입니다.
  • 원본 #3333을 머지와 동시에 닫을지, 랜딩 코멘트만 남기고 조금 기다릴지
  • GUI 레이아웃이라 스크린샷 한 장이 있으면 충분한지, 실제 대시보드에서 탭 이동을 한 번 더 볼지

너의 추천
CI가 초록이면 merge해도 됩니다. #3374가 되돌렸던 Logs/사이드바 수정을 이번엔 건드리지 않고, 폭 계약만 고칩니다. 머지 커밋 SHA가 나오면 #3333에 Landed via #3386 at <commit>을 남기고 닫으세요. 스타일시트 말고 로직을 더 고칠 구멍은 보이지 않습니다.

이 댓글은 grok-bot이 작성했습니다

Carried from #3333 onto current dev. Independent of the other carried fixes,
so it ships as its own PR rather than a stack layer.

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: the catalog column gated on .models-workspace-shell,
which is absent during the loading skeleton and after a cold failure, so the tab
rendered at 980px and snapped to 1200px once data arrived. Routing had no rule
at all and fell back to 980px. Scoping to the visible panel id fixes both and
cannot leak to another tab.

Combos separately renders its loading and error states without the workspace
shell, so full-bleed styling stretched a lone notice edge to edge. That is now
limited to the actual workspace.

The dashboard page-tabs guard located its target with a bare
indexOf(".page-tabs {"), which matches any rule whose selector merely ENDS in
that string — including the scoped Combos rule this adds above it. Anchoring the
search to a line-start rule fixes the guard without weakening it.

Rebuilt from the PR's own diff rather than its files: the first attempt copied
gui/src/styles.css wholesale and silently reverted both the Logs clipping fix
(#3367) and the sidebar footer rework (#3382).

Co-authored-by: hajune <june@smartix.co.kr>
@lidge-jun

Copy link
Copy Markdown
Owner Author

Rebased onto current dev. One conflict, resolved by dropping my edit: gui/tests/dashboard-tabs.test.ts was deleted on dev by #3387, so the page-tabs guard fix is no longer needed — the remaining diff is the three stylesheets plus the new layout test. Re-verified on the new base: bun run build:gui exit 0, lint:gui clean, 44 focused GUI tests pass, and the diff has no deletions outside the two rules it intentionally replaces.

@lidge-jun

Copy link
Copy Markdown
Owner Author

test 4/4 failed on update stops the running proxy before replacing files > npm launcher restarts the stopped runtime after a staged update failure (91.6s, timing-sensitive). It is unrelated to this change: tests/update-stop-first.test.ts reads nothing from gui/, and this PR only touches three stylesheets plus one new GUI test. It passes locally (15 pass / 0 fail). Re-running the failed jobs.

@lidge-jun
lidge-jun merged commit a333811 into dev Sep 3, 2026
43 of 46 checks passed
@lidge-jun
lidge-jun deleted the codex/carry-3333-v2 branch September 3, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant