Skip to content

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

Closed
blackjune67 wants to merge 6 commits into
lidge-jun:devfrom
blackjune67:hajune/fix-combos-tab-underline
Closed

fix(models): normalize mobile tab spacing and stabilize Combos layout#3333
blackjune67 wants to merge 6 commits into
lidge-jun:devfrom
blackjune67:hajune/fix-combos-tab-underline

Conversation

@blackjune67

@blackjune67 blackjune67 commented Sep 3, 2026

Copy link
Copy Markdown

Summary

  • Fix inconsistent spacing and alignment across the dashboard Models category tabs.
  • Align the Combos tab underline with the other Models tabs.
  • Keep the Catalog, Routing, Compatibility, and Combos panels at a consistent width.
  • Normalize mobile spacing and vertical positioning when switching between Models and Combos.
  • Prevent Combos loading, empty, and error states from incorrectly using a full-bleed layout.
  • Add declaration-level regression coverage for desktop and mobile layout rules.

Verification

  • bun x tsc --noEmit passed.
  • cd gui && bun run lint passed.
  • cd gui && bun test tests/models-tab-layout.test.ts passed with 18 assertions.
  • Manually verified the Models and Combos tabs on a mobile viewport.
  • Verified that repeatedly switching between Models and Combos keeps the heading, refresh button, tab strip, and panel spacing aligned.
  • The full test suite was attempted locally; unrelated Windows ACL, file-lock, and timeout failures occurred.

Before

Before correction

After

After correction

Checklist

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

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes

    • Improved layout consistency across catalog, routing, and compatibility panels, including loading and error states.
    • Fixed tab-strip alignment and workspace spacing for the Combos view on desktop and mobile.
    • Prevented panel widths and styling from carrying over incorrectly between tabs.
  • Tests

    • Added coverage for workspace widths, spacing, alignment, and fallback layouts.

@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: 5d2a2c45-fbdb-4bd3-8dfd-ce8a43170b4a

📥 Commits

Reviewing files that changed from the base of the PR and between 38f8a81 and 5ce40a1.

📒 Files selected for processing (4)
  • gui/src/styles-compatibility-matrix.css
  • gui/src/styles-models-workspace.css
  • gui/src/styles.css
  • gui/tests/models-tab-layout.test.ts

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


📝 Walkthrough

Walkthrough

The change updates Models workspace CSS selectors and fallback rules. Catalog, routing, and compatibility tabs retain 1200px widths in visible and shell-free states. Combos receives desktop and mobile spacing rules. A Bun test validates the resulting layout declarations.

Changes

Models workspace layout

Layer / File(s) Summary
Visible Models panel widths
gui/src/styles-models-workspace.css lines 11-33, gui/src/styles-compatibility-matrix.css lines 6-15
Catalog and compatibility width rules now target visible panel IDs. Routing now uses the same 1200px maximum width.
Combos layout and fallback states
gui/src/styles.css lines 453-492, 2368-2389
Combos tab spacing now uses 36px desktop and 18px mobile insets. Shell-free loading and error states use boxed container, panel, and chrome spacing.
Layout assertions
gui/tests/models-tab-layout.test.ts lines 1-62
The Bun test validates tab alignment, shell-free fallback spacing, and consistent widths across Models tabs.

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

Merge Risk: ⚪ Minimal · up to 5ce40

This change standardizes Models-tab widths and Combos fallback spacing across desktop and mobile layouts. No current merge-blocking risk remains.

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 1 files. (3 skipped: 3 … 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 summarizes the main changes: it identifies the Models tab scope, mobile spacing normalization, and Combos layout stabilization. It is concise and specific enough for the change se…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title accurately summarizes the main changes: it identifies the Models tab scope, mobile spacing normalization, and Combos layout stabilization. It is concise and specific enough for the change set.

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@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
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 06:27
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 54 / 80

이 PR은 Models 화면에서 탭을 왔다 갔다 할 때 가로 폭과 Combos 탭의 여백이 흔들리던 GUI 버그를 고친다. 지금 dev HEAD는 38f8a8164(#3330 Cursor 피커에서 unbranded 행의 cursor/ 슬러그 유지)이고, Combos는 이미 풀블리드 레이아웃이다. gui/src/App.tsx가 Combos 탭일 때만 main-inner--combos를 붙이고, gui/src/styles.css가 그 클래스에서 padding/max-width를 없애 그리드를 화면 끝까지 민다. 문제는 두 가지다. 첫째, Catalog는 보이는 패널 안에 .models-workspace-shell이 있을 때만 1200px로 넓어지고, Compatibility는 :has(.lab-page)라서 로딩(스켈레톤) 중에는 980px로 줄었다가 데이터가 오면 1200px로 튀며, 한 번 연 뒤에는 패널이 마운트된 채로 남아 다른 탭에도 폭이 새어 나간다. Routing에는 1200px 규칙 자체가 없어서 탭만 바꿔도 페이지가 넓어졌다 좁아졌다 한다. 둘째, Combos 로딩·에러는 gui/src/pages/Combos.tsxcombos-workspace-shell 없이 스켈레톤/에러만 돌려주는데, main-inner--combos는 탭 선택만으로 붙으므로 안내 문구와 재시도 버튼이 좌우로 끝까지 늘어난다. 이 PR은 Catalog/Routing/Compatibility를 모두 #models-panel-*:not([hidden])로 묶어 1200px을 고정하고, shell이 없을 때는 풀블리드를 끄며, 탭 스트립 underline이 버튼과 맞도록 margin/padding을 맞춘다. CSS 선언 단위 회귀 테스트도 새로 넣었다. types/config 분할과 무관한 순수 GUI 수정이라 리베이스 부담은 작다.

라인 styles-compatibility-matrix.css - :has(.lab-page)를 패널 id로 바꾼 방향은 맞다. 다만 Compatibility 로딩 UI가 나중에 #models-panel-compatibility 밖·다른 id로 바뀌면 폭 규칙이 다시 깨지니, 패널 id 계약을 짧게 주석/테스트로 고정해 두는 편이 안전하다.
라인 styles.css(모바일 shell-free .page-tabs) - 같은 선택자에 padding-inline: 0margin-inline: 0이 두 블록으로 나뉘어 있다. 동작은 같지만 한 규칙으로 합치면 읽기 쉽다.
경로 gui/tests/models-tab-layout.test.ts - 선언/캐스케이드 검증은 좋다. 실제 Chromium에서 탭 전환·로딩→성공 스냅이 흔들리지 않는지는 픽셀/시각 테스트가 아니라서, 리뷰어가 모바일 뷰포트에서 한 번만 훑어보면 충분하다.
경로 Vite Rolldown CSS - 콤마로 묶인 :not(:has(...))가 깨진다는 주석은 가치가 크다. 이후 Combos 레이아웃을 손볼 때 콤마 리스트를 다시 넣지 말라는 경고로 남기기 좋다.

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

  • review-ready이고 범위가 CSS+선언 테스트뿐이라, CI만 초록이면 바로 머지할지, 아니면 Combos 로딩/에러 화면을 모바일에서 한 번 더 눈으로 확인한 뒤 머지할지.
  • Catalog/Routing/Compatibility를 전부 1200px로 맞춘 선택이 앞으로 Models 하위 탭의 기본 폭인지, 아니면 Routing만 다시 980px로 둘지.

너의 추천
CI가 초록이면 머지해도 된다. 머지 전에 Combos 탭에서 로딩·에러(또는 네트워크 끊고 새로고침)일 때 안내 문구가 다른 Models 탭과 같은 폭으로 보이는지만 한 번 확인하면 충분하다. types.ts/config.ts 분할과 겹치지 않으니 닫을 이유는 없다.

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

lidge-jun pushed a commit that referenced this pull request Sep 3, 2026
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.
lidge-jun pushed a commit that referenced this pull request Sep 3, 2026
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 added a commit that referenced this pull request Sep 3, 2026
…#3386)

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: jun <jun@lidge.dev>
Co-authored-by: hajune <june@smartix.co.kr>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on as #3386 (), carried onto current with your trailer. Note for anyone carrying this next time: the first attempt copied wholesale and silently reverted #3367 and #3382 — the merged version applies your diff instead.

@lidge-jun lidge-jun closed this Sep 3, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as #3386 (a333811), carried onto current dev with your Co-authored-by trailer. Note for anyone carrying this next time: the first attempt copied gui/src/styles.css wholesale and silently reverted the Logs clipping fix (#3367) and the sidebar footer rework (#3382). The merged version applies your diff instead.

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants