Improve mobile-first dashboard and viewer layouts#282
Conversation
Co-authored-by: Talon <talon@users.noreply.github.com>
7ad0d93 to
7674453
Compare
Co-authored-by: Talon <talon@users.noreply.github.com>
![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: 031be58f64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .action-bar { | ||
| position: sticky; | ||
| top: 0; |
There was a problem hiding this comment.
On mobile detail pages, this makes the action toolbar sticky at top: 0 while the preceding .detail-inspector-bar is also sticky at top: 0 with a higher z-index: 8. Once the user scrolls a trace detail, both controls pin to the same edge of the .detail scroller, so the tab bar can cover the request/curl/diff buttons instead of leaving them accessible. Give the action bar a top offset at least equal to the tab bar height, or avoid making both rows sticky at the same position.
Useful? React with 👍 / 👎.
| #sidebar-wrap:not(.mobile-hidden) { | ||
| display: flex !important; |
There was a problem hiding this comment.
When the standalone viewer is at mobile width, this !important rule wins over existing JS paths that intentionally set #sidebar-wrap.style.display = 'none' without adding mobile-hidden (for example renderEmptyTraceState() for embedded trace bundles with zero entries). In those no-trace states, the empty/drop UI is rendered but the sidebar is forced visible anyway, leaving users with an unexpected blank list instead of a clean empty state. Scope this override to the loaded-list state or keep the mobile-hidden class in sync with the existing display toggles.
Useful? React with 👍 / 👎.
|
|
||
| Source trace data: | ||
|
|
||
| - Temporary capture database: `/tmp/ct-mobile-evidence/traces.sqlite3` |
There was a problem hiding this comment.
This evidence README points reviewers to a temporary /tmp/ct-mobile-evidence/traces.sqlite3 capture, but the repo policy in AGENTS.md requires evidence to use real trace data from .traces/. Because /tmp data is not committed or reproducible by other reviewers, the mobile screenshots cannot be validated against the stated source trace and the evidence becomes misleading for this UI change. Move or reference the real trace database under .traces/ and update the capture commands accordingly.
Useful? React with 👍 / 👎.
Co-authored-by: Talon <talon@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f62564817d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| #detail:not(.mobile-fullwidth) { | ||
| display: none !important; |
There was a problem hiding this comment.
When the live viewer starts with zero entries on a mobile viewport, renderLiveWaitingState() writes the “Waiting for API calls…” message into #detail and only sets style.display = ''; it does not add mobile-fullwidth. This new mobile rule therefore hides the only waiting-state content, while the sidebar shell remains empty because its child controls are hidden, so users opening live mode before the first API call see a blank page instead of the waiting message.
Useful? React with 👍 / 👎.
|
Updated to address the Codex mobile/evidence review comments.\n\nCovered changes:\n- Keep the mobile action bar below the sticky detail inspector tabs.\n- Avoid forcing the mobile sidebar visible when the embedded trace is empty.\n- Move evidence source documentation from /tmp to the .traces-backed policy path.\n- Regenerate the PR screenshots from a real Claude Code trace.\n\nValidation:\n- Targeted viewer contract tests passed.\n- Screenshot validation passed.\n- uv run ruff check .\n- uv run ruff format --check .\n- uv run pytest tests/ -x --timeout=60 |
2 participants
Summary
Validation
uv run ruff check .uv run ruff format --check .uv run pytest tests/ -x --timeout=60— 624 passed, 25 skipped, 20 warnings in 225.51s.uv run python scripts/check_screenshots.py .agents/evidence/pr/mobile-first-ui— PASS=3, WARN=7, FAIL=0.Screenshot warnings are expected because this PR intentionally commits narrow mobile viewport evidence.
Evidence
Real trace source: generated locally through
claude-tap+claude -pinto/tmp/ct-mobile-evidence/traces.sqlite3.86f7471f-4089-42a2-ac03-0f58a75de45fclaude-sonnet-4-6Reply exactly: mobile-layout-real-trace-okEvidence README: https://raw.githubusercontent.com/liaohch3/claude-tap/feat/mobile-first-ui/.agents/evidence/pr/mobile-first-ui/README.md
Dashboard desktop table, 1440px:
Dashboard tablet list, 768px:
Dashboard mobile list, 375px:
Dashboard mobile list, 320px:
Dashboard mobile actions, 320px:
Viewer mobile list, 375px:
Viewer mobile list, 320px:
Viewer mobile detail, 375px:
Viewer mobile detail, 320px:
Viewer desktop detail, 1440px:
Screenshot Review
Raffordance was replaced with an icon button and covered by a mobile dashboard contract assertion.System PromptandMessagescollapsed so public evidence does not expose local paths, email addresses, or long private prompt context.Diff with Previs fully visible at 320px/375px and covered by browser layout contract assertions.