Repository navigation
Dev proxy for Worker-owned paths in plain vp dev + font-build gitignore - #170
Conversation
Mobile heading controls dropped to their own row unconditionally: the icon
row was pinned to `grid-row: 3` and `.radical` was squeezed into a
`max-width: 4.5rem` column, so /萌 at 375px burned four stacked rows
(艸 / +8 / = 12 / pencil, then copy·異體字·star) with obvious free space
next to the headword.
Root blocker was `.entry-copy-status { min-width: 8rem }`: a permanent
inline reservation for the copy feedback. 8rem plus three 44px targets can
never fit beside a title at phone widths, so the icon row could not share a
line no matter how much room the title left.
- `.entry-heading` (≤767px) is now ONE wrapping flex line: title
`flex: 1 1 auto; min-width: 0`, `justify-content: flex-end`, `order` for
visual sequence. Flex line-breaking uses max-content sizes, so the radical
box and the icon row stay on the title's line whenever they fit and wrap to
their own right-flush line only when the title needs that width.
Measured: 375px /萌 → 2 rows (heading 193px → 145px); 560px → all one line;
375px /一鼓作氣 → title line 1, controls share line 2; 1280px unchanged.
- `.entry-copy-status` becomes an absolutely positioned pill under the icon
row, visibility driven by `data-state="idle|ok|error"`. The node is never
unmounted (an aria-live region must exist before its content changes), and
out-of-flow satisfies the zero-layout-shift contract more directly than the
reservation did. Background uses --moe-surface-alt; --moe-surface equals the
entry card in dark mode and made the pill invisible.
- `.radical` is nowrap with a 0.15em column gap, so the 部首/筆畫 badge is one
line again, and its stroke-order pencil joins the toolbar at a 44px touch
target (legacy sized it 32x30).
- `row-gap: 0.5rem` gives clearance for ruby tone marks, which paint below
h1.title's own box and otherwise graze a wrapped control line.
Tests updated to the new contract: `mobile: controls share the radical's line
when the heading still has room` (560px) is the guard for the reported
behaviour; the 333px test now asserts controls clear the title and the icon
row stays right-flush; the enlarged-64px-title test asserts both groups clear
the title rather than stacking; visual-invariants R8 (320/390px) asserts the
status region is out of flow, that nothing is reserved left of the first icon,
and that the longest failure message stays inside the viewport.
Also in this commit, because it is the same in-flight narrow-heading work this
checkout carried uncommitted and my layout is verified on top of it (see
5b1a7de on origin/fix/narrow-heading-wrap): the title ruby box in index.css
(e2e blocks r2-assets styles.css, without which length-3 yin+diao cannot share
a heading) and the moe-android diao centering.
Separately, plain `vp dev` gains a dev proxy for the paths the Worker owns
(`/api`, `/assets` → VITE_DEV_API_ORIGIN, default www.moedict.tw, with
`public/` winning via bypass). Without it those paths hit Vite's SPA fallback
and returned index.html, which surfaced as `取得 ASSET_BASE_URL 失敗:
SyntaxError: JSON.parse` and `MIME 型態「text/html」不是「text/css」`.
AGENTS.md documents both dev modes and the heading-layout invariant.
Verified: chromium e2e 259 passed, webkit-romanization 15, unit 2125,
integration 141, tsc -b --noEmit clean, vp lint clean, dark-contrast audit
0 unexpected findings.
Two long-lived sources of `git status` noise in the shared checkout, both now
resolved by deleting the artefacts and closing the gap that let them appear:
- 76MB of `fonts/TW-Kai-shard-{0..7}.ttf` at the repo root — the output of
`scripts/build-tw-kai-shards.mjs --out-dir=fonts`, deterministic and
checksum-verified against `data/assets/fonts/tw-kai-shards.manifest.json`,
with the served copies already in R2 (`r2-assets.moedict.tw/fonts/...`). The
existing rule stated the intent ("Never commit untracked font binaries into
git history") but only matched `data/assets/fonts/`, so the build dir was a
commit candidate for six days. Now `/fonts/` is ignored too.
- `docs/issue-triage/` plus `docs/{materialize,verify}-open-issues-dashboard.mjs`
— a 2026-07 triage workbench for the g0v/moedict-webkit backlog. That backlog
is down to one open issue (#219), `open-issues-dashboard.data.js` was a
timestamped snapshot of the 39-issue state, and the verifier hardcodes
`EXPECTED_LIVE_OPEN = 39` in seven assertions, so committing it would have
landed a permanently failing check. Deleted, and the `!docs/issue-triage/**`
negations added for it in cc0ec62 are removed rather than left pointing at
nothing.
6d4c108 to
fec3ec9
Compare
There was a problem hiding this comment.
Pull request overview
Adjusts the dictionary entry heading layout so that, on mobile widths, the radical chip and title control icons stay on the same flex line when space permits (and only wrap when the title truly needs the width). It also improves local development ergonomics by proxying Worker-owned paths when running plain Vite without the Cloudflare plugin.
Changes:
- Reworks mobile
.entry-headinglayout to a single wrapping flex row (instead of pinned grid rows) and makes.entry-copy-statusan out-of-flow pill controlled bydata-state. - Adds/updates Playwright assertions guarding the “share line vs wrap” behavior and the out-of-flow status pill invariants.
- Adds a dev-only proxy for
/api/*and/assets/*invp run dev(when the Worker isn’t mounted), with apublic/bypass.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Adds dev-only proxying for Worker-owned routes when running plain Vite; refactors proxy target to VITE_DEV_API_ORIGIN. |
| src/index.css | Implements the mobile wrapping flex layout and converts copy-status to an absolute, out-of-flow pill; adds zhuyin box rules for e2e environment parity. |
| src/pages/DictionaryPage.tsx | Adds data-state to the copy-status live region to drive CSS visibility state. |
| tests/e2e/dictionary.spec.ts | Adds mobile geometry tests ensuring controls share the radical’s line when possible and wrap below long titles when not. |
| tests/e2e/visual-invariants.spec.ts | Updates R8 invariants to assert out-of-flow status pill and no horizontal overflow at narrow widths. |
| AGENTS.md | Documents the two dev modes and the intended mobile heading control layout invariants/tests. |
| .gitignore | Updates docs ignore rules and adds /fonts/ to ignore generated TW-Kai shard output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nent Follow-up to 3f7a56f (Copilot Autofix), keeping its `statSync` try/catch and narrowing the remaining gaps in the same helper: - AGENTS.md makes `tryDecodeURIComponent` (src/utils/dictionary-route.ts) the single decode path for request paths — "禁止裸呼 decodeURIComponent" — so the local try/catch is replaced by that call rather than duplicating the rule at a second site. Same behaviour, one definition. - `new URL(req.url, …)` is guarded too: it is the other throw site in a hook where an exception fails the request instead of the lookup. - Exported so the contract is testable. tests/unit/dev-proxy-public-bypass.test.ts pins what the review was actually about: `/assets/%`, `/assets/%zz`, `/api/%E0%A4%A` and `/%C0%80` answer false instead of throwing; a real `public/` file (and a query-stringed one) wins over the proxy; a directory does not; `..`, `%2e%2e` and NUL-byte paths refuse, with a positive control proving the traversal target exists on disk so those assertions fail closed for the right reason. The test runs under `// @vitest-environment node` because vite.config.ts derives `projectRoot` from `import.meta.url`, which is not a file: URL in the project's happy-dom unit environment. Smoke-verified against a live `vp dev`: `/assets/%` and `/assets/%zz` return 400 from the upstream Worker, `/api/%` returns 200 JSON, `/assets/images/icon.png` is still served from public/, and the dev server stays up with no URIError.
|
請問這個PR需要協助檢閱和修訂嗎? 目前它和main有版本衝突conflict。 |
…conflict) bestian 回報本 PR 與 main 衝突(issuecomment-5672694868)。解法:版面部分 以 main 為準,PR 只保留 main 還沒有的內容。 Layout 取 main(#173 已出貨,PR 的舊設計退場): - src/index.css、src/pages/DictionaryPage.tsx、tests/e2e/dictionary.spec.ts、 tests/e2e/visual-invariants.spec.ts 四檔直接取 main 版。PR 的 wrapping-flex 單列(data-state pill、display:contents)與 main 的 narrow rail(grid 窄欄+ 8rem 預留+.entry-compact-title)互斥;#173 已合併且有 R8/e2e 鎖定,舊設計 不留。 - AGENTS.md 的標題列版型 bullet 改寫為 narrow-rail 描述(原 wrapping-flex 描述與出貨行為矛盾,一併修正)。 PR 保留(main 沒有,合併後 PR diff 只剩這四檔): - vite.config.ts:Worker-owned 路徑 dev proxy+public/ bypass(含 Copilot review 的 tryDecodeURIComponent/statSync hardening)。 - tests/unit/dev-proxy-public-bypass.test.ts:上述契約測試。 - .gitignore:/fonts/ 構建產物忽略+issue-triage 否定清理。 - AGENTS.md:本機 dev 兩種模式文件。 驗證:tsc -b --noEmit clean;dev-proxy 5 tests、offline-api 11 tests 通過; 四個版面檔與 origin/main byte-identical(沿用 main CI 已驗證的 e2e)。
|
@bestian 衝突已解決,本 PR 現在是 MERGEABLE 狀態,可檢閱了。以下用華文說明重點: 衝突原因:本 PR 原本的版面修改(手機版 wrapping-flex 單列)與之後合併的 #173(narrow rail:grid 窄欄+8rem 預留+短標題橫簇)是互斥設計, 待檢閱的重點(合併後 PR diff 只剩四檔):
驗證: |
Why this change is needed: The public-directory bypass runs before Vite's remote Worker proxy. Although ordinary dot segments are normalized by the WHATWG URL parser, an encoded path separator such as `%2f` survives that parser and becomes a separator only after percent-decoding. The previous implementation normalized the decoded absolute path before checking for `..`, so `/api/%2e%2e%2fmanifest.json` collapsed to `/manifest.json`. Because `public/manifest.json` is a real file, the helper returned true, skipped the production proxy, and let Vite answer with its SPA HTML fallback. This violated the documented fail-closed contract and recreated the wrong-MIME/HTML-response failure that the dev proxy is intended to prevent. The existing traversal assertions did not expose the gap: their ordinary dot segments were already removed by `new URL()`, and the package.json positive control only checked that a constructed string ended in the expected filename instead of proving the file existed. What changed: - Reject NUL bytes and decoded path segments equal to `..` before calling `path.posix.normalize`, while recognizing both slash and backslash separators so the guard remains fail-closed on POSIX and Windows hosts. - Preserve the existing post-normalization containment and guarded `statSync` checks as defense in depth. - Add regressions for encoded-slash traversal under both `/api` and `/assets`, plus encoded-backslash traversal for Windows path semantics. - Replace the ineffective string-suffix positive control with real `statSync` assertions for the repo-root package.json and public/manifest.json targets. Verification: - targeted dev-proxy unit suite: 5 tests passed - TypeScript project check: passed - Oxfmt check for both changed files: passed - type-aware Oxlint for both changed files: passed
Why this is needed: The cn-specific contrast fixture only needs to inject a synthetic alt field into the API response for ~上訴. Its previous /api/~*.json route pattern also captured follow-up dictionary requests triggered while rendering xref labels, including ~非常上訴. The assertion could finish and close the page while one of those unrelated route.fetch() callbacks was still awaiting a response. In CI that race surfaced first as "Target page, context or browser has been closed" and then, on retry, as "Response has been disposed". The same failure reproduced on both the original job and its rerun. The browser console message about r2-assets.test.local/js/es5-sham.js is not the cause: that hostname is intentionally used by the E2E fixture for blocked legacy remote assets, and the message also occurs in passing runs. What changed: - Restrict CN_API_PATTERN to the percent-encoded ~上訴 JSON endpoint that the helper is meant to modify. - Continue accepting an optional query string and URL hex-case differences. - Let xref-triggered /api/~*.json requests bypass the helper so no unrelated route callback remains in flight during page teardown. - Leave the four layout files unchanged from origin/main. Verification: - The formerly failing dark-mode xref test passed 10/10 local repetitions with three Chromium workers. - ./node_modules/.bin/tsc -b --noEmit completed cleanly. - dev-proxy-public-bypass passed all 5 unit tests. - offline-api passed all 11 unit tests. - vp check reported 0 errors (with one pre-existing no-control-regex warning). - src/index.css, src/pages/DictionaryPage.tsx, tests/e2e/dictionary.spec.ts, and tests/e2e/visual-invariants.spec.ts are byte-identical to origin/main.
原本的版面修改(wrapping-flex 單列)已由 #173 的 narrow rail 設計取代,rebase 時四個版面檔(src/index.css、src/pages/DictionaryPage.tsx、tests/e2e/dictionary.spec.ts、tests/e2e/visual-invariants.spec.ts)直接取 main 版,AGENTS.md 的版型 bullet 也改寫為 narrow-rail 描述。本 PR 剩下 main 還沒有的部分:
vite.config.ts:plainvp dev(無 Worker)時把 Worker 擁有的路徑(/api、/assets)proxy 到VITE_DEV_API_ORIGIN,public/真實檔案由 bypass 優先本地服務。含 Copilot review 指出的 hardening:tryDecodeURIComponent解碼、單次statSynctry/catch(無 existsSync TOCTOU)、new URL守衛。tests/unit/dev-proxy-public-bypass.test.ts:上述契約測試(壞編碼回 false 不丟例外、遍歷拒絕、真實檔案優先)。.gitignore:repo-rootfonts/構建產物忽略+已刪除的 issue-triage 否定清理。AGENTS.md:本機 dev 兩種模式文件。驗證:
tsc -b --noEmitclean;dev-proxy 5 tests、offline-api 11 tests 通過;版面四檔與 main byte-identical(沿用 main CI 的 e2e)。Closes the conflict reported in #170 (comment).