Skip to content

优化手机端首屏提交路径 - #59

Merged
zj1123581321 merged 4 commits into
mainfrom
card/mobile-submit-ux
Aug 12, 2026
Merged

优化手机端首屏提交路径#59
zj1123581321 merged 4 commits into
mainfrom
card/mobile-submit-ux

Conversation

@zj1123581321

Copy link
Copy Markdown
Collaborator

范围

  • 重排首页表单:分享输入、URL 识别、转录选项摘要、令牌提示、CTA、低频设置。
  • 已保存令牌时保持高级设置折叠;缺失令牌时展示“尚未配置访问令牌/去设置”并聚焦令牌输入。
  • 空输入隐藏 URL 预览,非法输入和提交失败提供输入框旁行内反馈;保留多 URL 选择、原 API payload、鉴权存储、PWA share-target、安装/主题事件和桌面布局语义。
  • 添加移动端结构契约测试与触控/ARIA 检查。

测试

  • npm run test:web:13 files / 168 tests passed;既有 SW cache 与 JSDOM navigation warning。
  • pytest -q --confcutdir=tests/unit/web tests/unit/web/test_frontend_auth.py tests/unit/web/test_frontend_nav.py tests/unit/web/test_frontend_mobile_submit.py:53 passed。
  • pytest -q tests/unit/test_api_routes.py:被仓库既有 config/config.jsonc JSONC 注释导入错误阻塞。
  • Chromium 392x637 本地渲染:/tmp/mobile-submit-after.png;无横向滚动、CTA 首屏可见、控制台/page error 为 0。

配置触点

无配置、依赖或后端 API 变更。

@github-actions

Copy link
Copy Markdown

Required Gate v2 — aggregate verdict

Result: skipped
No action needed — the primary review is intentionally skipped while the PR is a draft; the full primary review will run once you mark the PR ready for review.
Terminal state: classification=expected_skip, reason_code=review_not_expected, gate_result=skipped

Accepted:

  • quality: success
  • primary: skipped and accepted (draft=True, review_expected=False)

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

⚙️ Review ledger state(机器状态记录,非评审结论)

这是 review ledger 的机器状态记录,不代表评审结论,通常无需任何操作。

机器状态明细
  • Commit: 6763d4427e683d65bb4b902f9d0221f8e25e463e
  • Round: 5
  • Status / findings: pass / 0
  • Reviewer: codex-sub
  • Comparison: same_head_rerun; stable/missing/appeared = 0/0/0

完整数据保存在 codex-review-ledger-v2 artifact;此 sticky comment 仅保存 v2 epoch 的跨 rerun 连续游标。

@github-actions

Copy link
Copy Markdown

Required Gate v2 — aggregate verdict

Result: skipped
No action needed — the primary review is intentionally skipped while the PR is a draft; the full primary review will run once you mark the PR ready for review.
Terminal state: classification=expected_skip, reason_code=review_not_expected, gate_result=skipped

Accepted:

  • quality: success
  • primary: skipped and accepted (draft=True, review_expected=False)

@github-actions

Copy link
Copy Markdown

Required Gate v2 — aggregate verdict

Result: skipped
No action needed — the primary review is intentionally skipped while the PR is a draft; the full primary review will run once you mark the PR ready for review.
Terminal state: classification=expected_skip, reason_code=review_not_expected, gate_result=skipped

Accepted:

  • quality: success
  • primary: skipped and accepted (draft=True, review_expected=False)

@zj1123581321
zj1123581321 marked this pull request as ready for review August 12, 2026 10:01
@github-actions

Copy link
Copy Markdown

Required Gate v2 — aggregate verdict

Result: skipped
No action needed — the primary review is intentionally skipped while the PR is a draft; the full primary review will run once you mark the PR ready for review.
Terminal state: classification=expected_skip, reason_code=review_not_expected, gate_result=skipped

Accepted:

  • quality: success
  • primary: skipped and accepted (draft=True, review_expected=False)

@github-actions

Copy link
Copy Markdown

Required Gate v2 — aggregate verdict

Result: pass
No action needed — quality passed and the primary reviewer approved this change; the gate is green.
Terminal state: classification=code_pass, reason_code=primary_pass, gate_result=pass

Accepted:

  • quality: success
  • primary audit source run_attempt=1 (current run_attempt=1)
  • primary: pass

@github-actions

Copy link
Copy Markdown

OCR 复核未完成

已实锤 8 条 findings 已发布;2 条未复核/无法复核,未进入上方 findings。

要不要管:不阻塞合并;需要完整复核时请重跑本 PR 或查看事件 artifact/job 日志。未复核项不能视为已检查。

ocr_verify_status=partial · 实锤=8 · 未复核=2

Advisory review

OCR 复核:原始 11 条 → 实锤 8 / 驳回 1 / 未定 2(另有 0 条超预算未复核) OpenCodeReview advisory 补充评审:11 条评论,4 个文件

Findings

  • ocr-001 (minor, correctness) — src/web/static/css/styles.css:233: The `.advanced-settings` element has a base rule `max-height: 0; overflow: hidden;` (defined elsewhere) that animates open via the `.expanded` class. However, the newly added `[hidden]` selector sets `display: none`. When the JS (`toggleAdvancedSettings`) both adds `.expanded` AND removes the `hidden` attribute, the element will have `display: flex/none` toggling alongside `max-height` animation — these two mechanisms conflict. Setting `display: none` bypasses the `max-height` transition entirely (no smooth animation when showing), and the `max-height: 0` base rule means the content is invisible even after `hidden` is removed if `.expanded` isn't applied. Consider choosing ONE mechanism for show/hide: either rely solely on the `hidden` attribute (and remove the `max-height` animation logic) or keep the animation and avoid toggling the `hidden` attribute.
    • Source: OpenCodeReview 判定:category=bug(advisory 补充评审,不参与门禁判定)
    • Evidence: src/web/static/css/styles.css:237-238 以 hidden 强制 display:none;src/web/static/css/styles.css:445-453 同一元素又通过 max-height transition/expanded 动画;src/web/static/js/app.js:754-761 展开时同步添加 expanded 并移除 hidden,收起时同步移除 expanded 并设置 hidden。
    • Suggested fix: /* Use only the [hidden] approach; remove .advanced-settings max-height animation, OR exclude .advanced-settings from this rule and keep max-height animation only. */ .url-preview[hidden], .input-feedback[hidden], .auth-missing-prompt[hidden], .transcription-options-panel[hidden] { display: none; }
  • ocr-003 (minor, design) — src/web/static/css/styles.css:882: Truncating `.transcription-options-summary` and `.advanced-summary` with `text-overflow: ellipsis` on mobile may hide meaningful information (e.g., the speaker recognition status). Since the summary text is dynamically set and can vary in length, consider adding a `title` attribute in the JS that sets the summary text, so users can still see the full text via a native tooltip on hover/long-press.
    • Source: OpenCodeReview 判定:category=maintainability(advisory 补充评审,不参与门禁判定)
    • Evidence: src/web/static/css/styles.css:882-886 在 480px 以下对两个摘要设置单行 ellipsis;src/web/static/js/app.js:1128-1136 动态更新说话人识别摘要但未同步设置 title;src/web/static/index.html:69,105 两个摘要元素也均无 title。
    • Suggested fix: 人工复核后自行决定是否采纳
  • ocr-006 (minor, correctness) — src/web/static/js/app.js:1081: The newly referenced DOM elements `transcription-options-toggle` and `auth-settings-link` have event listeners attached without null checks (`transcriptionOptionsToggle.addEventListener(...)` and `authSettingsLink.addEventListener(...)`). If either element is missing from the HTML, this will throw a `TypeError: Cannot read properties of null (reading 'addEventListener')` and break the entire `initializePage` function. This is inconsistent with the defensive null-checking pattern applied to other elements in the same diff (e.g., `speakerSummary`, `tokenInput`, `icon`). Add null guards before attaching listeners.
    • Source: OpenCodeReview 判定:category=bug(advisory 补充评审,不参与门禁判定)
    • Evidence: src/web/static/js/app.js:1081-1085 两次 getElementById 的结果均未经 null 检查即调用 addEventListener;src/web/static/index.html:65、88 当前页面包含对应元素。
    • Suggested fix: const transcriptionOptionsToggle = document.getElementById('transcription-options-toggle'); if (transcriptionOptionsToggle) { transcriptionOptionsToggle.addEventListener('click', UIManager.toggleTranscriptionOptions); } const authSettingsLink = document.getElementById('auth-settings-link'); if (authSettingsLink) { authSettingsLink.addEventListener('click', () => { if (!isAdvancedSettingsExpanded) UIManager.toggleAdvancedSettings(); const tokenInput = document.getElementById('bearer-token'); if (tokenInput) tokenInput.focus(); }); }
  • ocr-007 (minor, correctness) — src/web/static/js/app.js:824: `previewContainer` is obtained via `document.getElementById('url-preview')` and its `.hidden` property is immediately accessed (line 826) without a null check. While `inputFeedback` is properly null-checked on the next line, `previewContainer` is not. This is inconsistent and could throw a `TypeError` if the element is absent. Apply the same defensive null-check pattern.
    • Source: OpenCodeReview 判定:category=bug(advisory 补充评审,不参与门禁判定)
    • Evidence: src/web/static/js/app.js:824-826 获取 previewContainer 后直接访问 .hidden;src/web/static/js/app.js:830-833 对 inputFeedback 则先做非空检查。
    • Suggested fix: const previewContainer = document.getElementById('url-preview'); const inputFeedback = document.getElementById('input-feedback'); if (previewContainer) previewContainer.hidden = urlResults.length === 0;
  • ocr-008 (minor, correctness) — src/web/static/js/app.js:996: After successful form submission, `previewContainer` is accessed without a null check before setting `.innerHTML` and `.hidden`. This is inconsistent with the null-check applied to `inputFeedback` just two lines below. If `url-preview` is not in the DOM, this will throw a `TypeError`.
    • Source: OpenCodeReview 判定:category=bug(advisory 补充评审,不参与门禁判定)
    • Evidence: src/web/static/js/app.js:997 获取 `url-preview` 后,998-999 行直接访问 `previewContainer.innerHTML` 和 `.hidden`;src/web/static/js/app.js:1001 对 `inputFeedback` 则进行了空值检查。
    • Suggested fix: document.getElementById('share-content').value = ''; const previewContainer = document.getElementById('url-preview'); if (previewContainer) { previewContainer.innerHTML = ''; previewContainer.hidden = true; }
  • ocr-009 (minor, design) — tests/unit/web/test\_frontend\_mobile\_submit.py:16: Using str.index() raises ValueError ("substring not found") instead of a clear assertion failure when an element ID is missing from index.html. Since these are contract tests whose entire purpose is to detect when a required element disappears, the error message should be self-describing. Use a helper like `assert 'id="share-content"' in html` or wrap each lookup so the failure message identifies the missing contract.
    • Source: OpenCodeReview 判定:category=maintainability(advisory 补充评审,不参与门禁判定)
    • Evidence: tests/unit/web/test_frontend_mobile_submit.py:16-23 directly calls html.index(...) for each required element ID without prior presence assertions; a missing ID therefore raises ValueError before the assertion at line 24.
    • Suggested fix: ids = [ 'id="share-content"', 'id="url-preview"', 'id="transcription-options-toggle"', 'id="auth-missing-prompt"', 'id="submit-btn"', 'id="advanced-settings"', ] for element_id in ids: assert element_id in html, f"Missing required element: {element_id}" positions = [html.index(element_id) for element_id in ids] assert positions == sorted(positions)
  • ocr-010 (minor, design) — tests/unit/web/test\_frontend\_mobile\_submit.py:24: The extra assertion `html.index('id="advanced-settings"') > html.index('id="submit-btn"')` is fully redundant: 'submit-btn' precedes 'advanced-settings' in the positions list, and `positions == sorted(positions)` already guarantees that ordering. Remove it to reduce noise.
    • Source: OpenCodeReview 判定:category=maintainability(advisory 补充评审,不参与门禁判定)
    • Evidence: tests/unit/web/test_frontend_mobile_submit.py:21-25 positions 依次包含 submit-btn 和 advanced-settings,随后已断言 positions == sorted(positions)。
    • Suggested fix: assert positions == sorted(positions)
  • ocr-011 (minor, design) — tests/unit/web/test\_frontend\_mobile\_submit.py:36: split(...)[1] raises an IndexError if '@media (max-width: 480px)' is absent from styles.css. This produces an opaque failure instead of a meaningful assertion. Prefer an explicit membership check so the failure message identifies the missing media query.
    • Source: OpenCodeReview 判定:category=maintainability(advisory 补充评审,不参与门禁判定)
    • Evidence: tests/unit/web/test_frontend_mobile_submit.py:36 对 split("@media (max-width: 480px)", 1) 的结果直接取 [1],此前没有显式存在性断言。
    • Suggested fix: css = STYLES_CSS.read_text(encoding="utf-8") assert "@media (max-width: 480px)" in css, "Missing @media (max-width: 480px) breakpoint" mobile_css = css.split("@media (max-width: 480px)", 1)[1] assert '.site-nav-link[href="/"]' in mobile_css assert 'display: none' in mobile_css

@zj1123581321
zj1123581321 merged commit b5a315f into main Aug 12, 2026
20 checks passed
@zj1123581321
zj1123581321 deleted the card/mobile-submit-ux branch August 12, 2026 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant