0681e056fd
ย้าย logic ของแถบ tool ออกจาก PDFViewer.tsx ไป pure module ใหม่ components/pdf/toolbar-tools.ts (behavior-preserving) ตามแผน TDD docs/PLAN-TOOLBAR.md ขั้น 1+2 รวม findings #1-#6: - ขั้น 1: ย้าย type Tool/TOOLS/visibleToolsFor/canSwitchTo/ toolButtonFace/menuIntentAfterClick ออกจาก PDFViewer.tsx + rewire + ตัด icon imports ว่าง 9 ตัว + spec test tests/components/toolbar-tools.test.ts - ขั้น 2 (findings #2/#5): TOOL_MENU_KEYS + type MenuTool + TOOL_MENU_TABLE (Record บังคับครบทุก key — เพิ่มเมนูลืมแถว = compile fail) + closeToolMenus()/closePagePopups() ใช้ร่วมครบทุก กลุ่มจุดเดิม รวม 2 จุดที่เดิมไม่เคลียร์เลย (scannedOnly + สลับไฟล์ ภาพ) — popup bookmark/composer/popover ของ pin/แผง selection/editor ข้อความไม่ค้างเปิดซ้อนกันครบทุกทางเข้า - findings #1: คีย์ลัดจับคู่ด้วย e.code แทน e.key (TOOL_SHORTCUTS KeyH/KeyD/KeyC + ขยายครอบ Ctrl+Z undo/redo) — layout ไทยให้ e.key เป็นอักษรไทยจนคีย์ลัด H/D/C/B และ undo ใช้ไม่ได้ - findings #3/#4/#6: ปรับให้ปุ่มใช้ตัวแปร active เดียวกันทั้งชุด (aria-pressed) ผ่าน tsc + vitest 207/207 + e2e Playwright ตามแผน (เมนูค้าง/สลับไฟล์ ภาพ/popup bookmark ลำดับปิด-เปิด/คีย์ลัด synthetic layout ไทย) Co-Authored-By: Claude Code <noreply@anthropic.com>
155 lines
20 KiB
Markdown
155 lines
20 KiB
Markdown
# PLAN-TOOLBAR — Test + แก้ไข logic แถบ tool ของหน้า annotate
|
||
|
||
> สร้าง 2026-09-29 จาก TDD session — ขั้นตอนไหนจะทำเมื่อไรให้รอคำสั่ง
|
||
> (อ้างอิงบรรทัดจาก HEAD 8571ef9)
|
||
>
|
||
> **ความคืบหน้า:** ขั้น 1 + ขั้น 2 (รวม findings #2/#5) + findings #1/#3/#4/#6
|
||
> เสร็จแล้ว 2026-09-29 (ยังไม่ commit) — เหลือเฉพาะขั้น 4 (รออาการ bug เฉพาะ)
|
||
> และรายการ "จดไว้" #7–#10, #13–#14
|
||
|
||
## สถานะปัจจุบัน
|
||
|
||
- **ขั้น 1 ทำเสร็จ 2026-09-29:** spec test ฉบับแรกถูก revert ไปพร้อมโค้ดรอบก่อน
|
||
(git มีแต่แผนนี้) จึงเขียนใหม่ตามเกณฑ์เดิม (literal จาก rules + 8571ef9
|
||
ไม่คัดลอกสูตร — กัน tautological) → ยืนยันแดงที่ import ก่อน implement →
|
||
สร้าง `components/pdf/toolbar-tools.ts` + rewire PDFViewer + ตัด icon imports
|
||
ว่าง 9 ตัว — vitest **204/204** + `tsc --noEmit` สะอาด + code-reviewer ผ่าน
|
||
(ไม่มี findings ต้องแก้/ควรแก้ — จุดจดไว้อยู่ท้ายตารางขั้น 3 #11–#12)
|
||
- **ขั้น 2 + findings #2/#5 ทำเสร็จ 2026-09-29:** ทางเลือก (ก) — `TOOL_MENU_KEYS`
|
||
+ `type MenuTool` ใน toolbar-tools.ts, `TOOL_MENU_TABLE` (Record บังคับครบทุก
|
||
key = เพิ่มเมนูลืมแถวก็ compile fail) + `closeToolMenus()`/`closePagePopups()`
|
||
ใน PDFViewer ใช้ร่วมครบ 6 กลุ่มจุดเดิม (switchTool/outside-click/เลื่อนแถบ·
|
||
resize/auto-select/openBookmarkComposer/reset tool ตรง 2 จุดที่เดิมไม่เคลียร์
|
||
เลย — scannedOnly + สลับไฟล์ภาพ) — finding #2 แก้: `openBookmarkComposer`
|
||
เรียก `closePagePopups()` ก่อนเปิด popup bookmark — test เพิ่ม TOOL_MENU_KEYS
|
||
cross-check + truth table เคสที่ 4 (**205/205**) + e2e Playwright 8/8
|
||
(กดแก้ไข comment → กด B = popover/editor ปิดก่อน popup bookmark โผล่ + focus
|
||
ย้ายเข้าช่องชื่อ; เมนู highlight ค้าง → สลับไฟล์ภาพ = ปุ่มหาย เมนูไม่ค้าง) +
|
||
sync rules/pdf-viewer.md 2 จุด
|
||
- **findings #1 แก้เสร็จ 2026-09-29:** คีย์ลัดจับคู่ด้วย `e.code` แทน `e.key` —
|
||
`TOOL_SHORTCUTS` (KeyH/KeyD/KeyC → tool) ใน toolbar-tools.ts, handler ใช้
|
||
`e.code` ครบ H/D/C/B **+ Ctrl+Z undo/redo** (ขยายครอบตามเหตุผลเดียวกัน —
|
||
layout ไทยให้ e.key "ฎ" ทำให้ undo ตายเหมือนกัน), delete/backspace ใช้
|
||
canonical `e.key` ต่อ (เหมือนทุก layout) — unit 207/207 (เพิ่ม 2 เคส
|
||
TOOL_SHORTCUTS + cross-check คู่ label `(H)/(D)/(C)` กัน drift) + e2e
|
||
synthetic Thai keydown (e.key อักษรไทย + e.code ตำแหน่งปุ่ม) **6/6** +
|
||
rules sync (bullet คีย์ลัด)
|
||
- **แก้ตาม code review ขั้น 2 (รอบเดียวกัน):** ไม่มี bug — แก้ 3 จุด: (a) ⚠️
|
||
`handleAnnotationBookmark` (ปุ่ม bookmark ในแผง selection N2 — ทางเปิด popup
|
||
bookmark ทางเดียวที่ยังไม่ผ่านชุดเคลียร์) เพิ่ม `closeToolMenus()` +
|
||
`closePagePopups()` ต้น function ให้จบ finding #2 ครบทุกทาง (b) ตัด unused
|
||
import `TOOLS` ที่ขั้น 1 ตกค้าง (c) ก้อน Esc ของคีย์ลัด (7 setter) แทนด้วย
|
||
`closePagePopups()` — reviewer เทียบ 1:1 เป๊ะ behavior-preserving — จุด
|
||
enumerate setter ด้วยมือฝั่ง popup หมดสิ้น — tsc + 205/205 + e2e rerun เคส
|
||
A (B/Esc path) **5/5**
|
||
- ขั้น 3 ขึ้นไป: codebase ยังไม่ถูกแตะ รอคำสั่ง
|
||
- reviewer 2 ตัว (code-reviewer + plan-reviewer) ตรวจพื้นที่ toolbar แล้ว —
|
||
สรุป findings ในขั้น 3
|
||
|
||
## ขั้น 1 — ทำ spec แดงให้เขียว: สร้าง `components/pdf/toolbar-tools.ts`
|
||
|
||
pure module (ไม่มี React/state) export 5 อย่างตาม spec:
|
||
|
||
| API | เนื้อที่ย้ายจาก PDFViewer.tsx (behavior-preserving 100%) |
|
||
|---|---|
|
||
| `type Tool` + `ToolEntry` | 156-166 |
|
||
| `TOOLS` + comment เต็ม | 379-398 |
|
||
| `visibleToolsFor(hide)` | 3161 |
|
||
| `canSwitchTo(t, hide)` | guard 3114 |
|
||
| `toolButtonFace(active, {label,text}, value?)` | pattern เดียวกัน 3391-3397 / 3487-3489 / 3597-3600 / 3686-3691 |
|
||
| `menuIntentAfterClick(cur, key, open)` | `reopen` 3414 / 3504 / 3615 / 3713 / 3941 |
|
||
|
||
Rewire ใน PDFViewer: import แทน definition เดิม + แทน inline ternary 4 ปุ่ม
|
||
dropdown ด้วย `toolButtonFace` + แทน `reopen` 5 จุด + **ตัด icon imports ที่ว่าง
|
||
9 ตัว** (Eraser/EyeOff/Hand/Highlighter/MessageSquarePlus/MousePointer2/Shapes/
|
||
Type/Stamp — เหลือ Pencil, LucideIcon ที่ยังใช้อื่น)
|
||
|
||
**Verify:** test 11 เคสเขียว + ทั้งชุด 204/204 + `tsc --noEmit` สะอาด
|
||
(เคย implement ผ่านเต็มมาแล้วรอบหนึ่งก่อนคืนตามคำสั่ง — ขั้นตอนนี้ทำซ้ำได้เลย)
|
||
|
||
## ขั้น 2 — switchTool cascade: รวมชุด "ปิดเมนู/popup" ที่เขียนซ้ำ 6 แห่ง
|
||
|
||
**ปัญหา:** ชุด `set*MenuOpen(false)` ×5 enumerate ด้วยมือซ้ำกัน 6 จุด —
|
||
3118-3126 (switchTool), 1968-1971 (outside-click), 1538-1544 (เลื่อนแถบ/resize),
|
||
2442-2446 (openBookmarkComposer), 2146-2151 (auto-select), + setTool ตรง 2 จุด
|
||
(1617-1619 scannedOnly, 1691 สลับไฟล์ภาพ) ที่**ไม่เคลียร์อะไรเลย** → เพิ่มเมนู
|
||
ใหม่/แก้พฤติกรรมแล้วลืมแห่งเดียว = drift เงียบ (เคยเกิดจริง: effect เลื่อนปุ่ม
|
||
active ใช้ title match เสียเงียบตั้งแต่ 2026-09-23 — commit 8571ef9)
|
||
|
||
**ทางเลือก (เลือกตอน implement):**
|
||
- **(ก) เบา — แนะนำ:** helper เดียวใน toolbar-tools.ts เช่น `TOOL_MENU_KEYS`
|
||
+ `resetMenus()`/`resetPagePopups()` ใช้ร่วมทั้ง 6 จุด; test ยืนยัน helper ครบ
|
||
ทุกเมนูที่มี dropdown (cross-check กับ TOOLS — กันลืมตัวใหม่)
|
||
- **(ข) หนัก:** ย้ายก้อน state menu/popup (~10 ตัว) ไป `useReducer` เดียว —
|
||
test ครบทั้ง transition แต่ diff ใหญ่ในไฟล์ ~4,000 บรรทัด เสี่ยงเกินผล
|
||
|
||
**แก้พวงในขั้นเดียวกัน (ต้องแตะจุดเดียวกัน):**
|
||
- `openBookmarkComposer` (2438-2458) ปิดเมนูแต่**ไม่เคลียร์ popup บนหน้า**
|
||
(`editingCommentId`/`openPinId`/`selectedId`/`composer`/`textboxComposer`/
|
||
`editingTextboxId`) → popup bookmark ซ้อน popover/editor ที่ค้างเปิด +
|
||
autoFocus แย่ง focus — ผิดหลัก "popup เปิดได้ทีละชุด" (rules ข้อ 30)
|
||
- setTool ตรง 2 จุด (1617-1619, 1691) ให้ผ่านชุดเคลียร์เดียวกัน — edge:
|
||
เมนู highlight ค้างเปิด → ไฟล์จริงเป็นสแกนล้วน → ปุ่มหายแต่เมนู portal
|
||
ยังลอยไร้ปุ่มยึด
|
||
- จุด `anyMenuOpen` อ่านจาก closure ของ render (3118) — ปัจจุบันเวิร์กเพราะ
|
||
outside-click flush ก่อนเสมอ แต่ไม่ enforced ที่จุดเดียว; ใส่ comment
|
||
กำกับ ordering นี้ตอนรวม helper
|
||
|
||
**Verify:** test helper ครบทุกเมนู + เดินมือ/Playwright: กด "แก้ไข" comment →
|
||
กด B = popover ปิดก่อน popup bookmark โผล่; เปิดเมนู highlight → เปิดไฟล์ภาพ
|
||
(สลับ dynamic param) = เมนูไม่ค้าง
|
||
|
||
## ขั้น 3 — findings จาก reviewer แปลงเป็นรายการแก้ (จัดลำดับ)
|
||
|
||
| # | ประเด็น | ที่ | แนวทาง | ลำดับ |
|
||
|---|---|---|---|---|
|
||
| 1 | ~~**คีย์ลัด H/D/C/B ใช้ไม่ได้บน layout ภาษาไทย**~~ **แก้เสร็จ 2026-09-29** — `TOOL_SHORTCUTS` (map `e.code`→tool) ใน toolbar-tools.ts + handler ใช้ `e.code` ทั้ง H/D/C/B และ **Ctrl+Z undo/redo ด้วย** (ขยายครอบตามเหตุผลเดียวกัน — e.key บน layout ไทยเป็น "ฎ" ทำให้ undo ตายเหมือนกัน) + test cross-check คู่ label `(H)/(D)/(C)` + e2e synthetic Thai keydown 6/6 | เดิม 2056-2070 | — | **เสร็จ** |
|
||
| 2 | popup bookmark ซ้อน editor (รายละเอียดขั้น 2) | 2438-2458 | รวมกับขั้น 2 | สูง (ทำพร้อมขั้น 2) |
|
||
| 3 | ~~โควตา coarse hint 3 ครั้ง นับเกิน~~ **แก้เสร็จ 2026-09-29** — นับเฉพาะ `enteringHighlight` (เพิ่งสลับเข้าจาก tool อื่น — กดปุ่ม dropdown เปิด/พับเมนูตอน active อยู่แล้ว และกด H ซ้ำ ไม่กินโควตา) | เดิม 3136-3138 | — | **เสร็จ** |
|
||
| 4 | ~~กด H บนไฟล์ภาพ/สแกนล้วน = no-op เงียบสนิท~~ **แก้เสร็จ 2026-09-29 (ทางเบา)** — เก็บ tool เดิมไว้ แต่ผ่านชุดเคลียร์เมนู+popup+selection เหมือนสลับ tool จริง (toast บอกเหตุผลเป็นทางเลือกที่**ยังไม่ทำ** — จิตวิทยา minimal ของผู้ใช้) | เดิม 3114 | — | **เสร็จ** |
|
||
| 5 | โครงซ้ำ 6 แห่ง (ขั้น 2) | ดูขั้น 2 | ขั้น 2 | สูง (พร้อมขั้น 2) |
|
||
| 6 | ~~`aria-pressed={tool === "textbox"}` ไม่ใช้ตัวแปร `active`~~ **แก้เสร็จ 2026-09-29** — เปลี่ยนเป็น `active` พร้อมชุด #3/#4 | เดิม 3934 | — | **เสร็จ** |
|
||
| 7 | `aria-pressed` คู่ `aria-haspopup="menu"` ชนกันตาม WAI-ARIA (toggle vs menu button) ทั้ง 5 ปุ่ม | 3405-3407 ฯลฯ | พิจารณาคง `aria-expanded` เดี่ยว — **เป็น behavior change ต้องขออนุมัติแยก** | ต่ำ/เลือกได้ |
|
||
| 8 | `liveMark`/`selectPreview` เคลียร์ผ่าน side effect ของ `removeAllRanges()` → `selectionchange` → effect ไม่ใช่ state ตรง — เวิร์กแต่ coupling ซ่อนอยู่ | 2681-2735, 3134 | ไม่แก้ — บันทึกความเสี่ยงไว้ใน comment ใกล้ switchTool | จดไว้ |
|
||
| 9 | ทางทฤษฎี: ความกว้าง `main` เปลี่ยนโดย window คงเดิม ไม่ปิดเมนู (ทางเข้าสลับ sidebar ผ่าน outside-click ก่อนเสมอจึงไม่เจอจริง) | 1546-1548 | ไม่แก้ — จดไว้ | จดไว้ |
|
||
| 10 | เคสจิ๋ว: แอนิเมชันเลื่อนปุ่ม active (guard 600ms) กลืน scroll ของผู้ใช้ → เมนูเพี้ยนจากปุ่ม | 1552, 1586 | ต้อง track จบจริง (`scrollend`) — แพงเกินปัญหา | จดไว้ |
|
||
| 11 | ช่องว่าง exhaustiveness ของ BTNS arrays: ถ้าขยาย `HighlightStyle`/`ShapeKind` ใน schema โดยไม่เพิ่มแถว `HIGHLIGHT_STYLE_BTNS`/`SHAPE_KIND_BTNS` (array ธรรมดา ไม่มี type บังคับครอบ union) ปุ่ม active จะแสดง `text` + title `label` เฉย ๆ (เดิม: title `"label — text"`) — ทางแก้ยาว: BTNS เป็น `Record<HighlightStyle, …>` หรือ `satisfies` เต็ม union | ToolControls.tsx:280, PDFViewer (SHAPE_KIND_BTNS) | ไม่แก้ตอนนี้ (ขัด behavior-preserving ของขั้น 1) — แก้เมื่อขั้นไหนแตะ BTNS | จดไว้ (code review ขั้น 1) |
|
||
| 12 | ~~test `menuIntentAfterClick` ยังไม่ครอบ truth table เคสที่ 4~~ **แก้แล้ว 2026-09-29 ระหว่างขั้น 2** | tests/components/toolbar-tools.test.ts | — | เสร็จ |
|
||
| 13 | deps ของ outside-click effect เป็น spread array `[...TOOL_MENU_KEYS.map(...)]` — runtime ถูกต้อง (ความยาว/ลำดับคงที่ ค่า primitive) แต่ react-hooks/exhaustive-deps lint มองไม่เห็นค่าใน spread — โปรเจกต์ไม่มี lint script จึงไร้ผลตอนนี้ | PDFViewer outside-click effect deps | จดไว้กันงงถ้าอนาคตเปิด eslint | จดไว้ (code review ขั้น 2) |
|
||
| 14 | `closeToolMenus`/`TOOL_MENU_TABLE` นิยามหลังจุดใช้เมื่อไล่อ่านจากบน (เรียกครั้งแรกที่ effect resize ~1538 นิยามที่ ~1962) — ไม่ใช่ bug (hoisted + effect รัน post-render + setter เสถียร) แค่อ่านยาก | PDFViewer | ย้ายขึ้กลุ่ม state เมนูเมื่อแตะจุดนั้นครั้งถัดไป | จดไว้ (code review ขั้น 2) |
|
||
| 15 | Ctrl+Alt+Z / Cmd+Option+Z ยิง undo เพิ่มจาก HEAD ในบาง layout (Windows จำลอง AltGr เป็น Ctrl+Alt — HEAD เช็ค e.key ที่ AltGr+Z แผ่อักขระพิเศษจึงไม่ตรง แต่ e.code ยิงเสมอ) — แทบไม่เกิดกับ layout ไทย/อังกฤษ ไม่ทำลายอะไร | PDFViewer ctrl branch | ถ้าต้องกัน: `!e.altKey` ในเงื่อนไข KeyZ บรรทัดเดียว | จดไว้ (code review #1/#3/#4/#6) |
|
||
| 16 | `TOOL_SHORTCUTS[e.code]` type เป็น `Tool` ไม่ใช่ `Tool \| undefined` (ไม่มี `noUncheckedIndexedAccess`) — ปลอดภัยเพราะมี guard runtime แต่ถอด guard ทิ้ง tsc ไม่ช่วยจับ | toolbar-tools.ts | เปลี่ยนเป็น `Partial<Record<string, Tool>>` ถ้าอยากให้ compiler บังคับ | จดไว้ (code review #1/#3/#4/#6) |
|
||
| 17 | `e.code` บนคีย์บอร์ดจอ (software keyboard) ของ iPad เคยมีรายงานว่าเป็นค่าว่างใน WebKit บางเวอร์ชัน — e2e ยืนยันด้วย synthetic event บน desktop เท่านั้น (hardware keyboard ปลอดภัยแน่นอน) | PDFViewer keydown | ถ้าได้รายงาน "คีย์ลัดตายบนคีย์บอร์ดจอ iPad" เช็คจุดนี้อันดับแรก + ทดสอบบนเครื่องจริง | จดไว้ (code review #1/#3/#4/#6) |
|
||
|
||
**ช่องว่าง test เพิ่ม (ตาม plan-reviewer):** ค่า face ของ stamp (`"กำหนดเอง"` เมื่อ
|
||
`stampPreset` ไม่ตรง preset), การ reset ค่ากลับ preset ตอนเลือก pen (3541-3546),
|
||
ยืนยัน textbox ไม่มีค่าย่อย — เพิ่มใน suite เดียวกันตอนขั้น 1/2; DOM-level
|
||
(toggle เมนู / outside-click + tap 300ms / Escape สองชั้น / `data-tool` ครบ 10 ปุ่ม)
|
||
ต้อง jsdom/e2e — แยกเป็นรอบหลัง
|
||
|
||
## ขั้น 4 — อาการเฉพาะจากผู้ใช้
|
||
|
||
**รอระบุ** — ผู้ใช้เลือกไว้ว่ามีอาการ bug เฉพาะที่เจอเอง แต่ยังไม่ได้บอกรายละเอียด
|
||
เมื่อได้อาการแล้ว: เพิ่มเป็นหัวข้อ "เคส F" พร้อม test reproduce ก่อนแก้ตามลูป TDD
|
||
|
||
## Carve-outs ที่ห้าม "แก้ให้ตรงกติกา" (จาก plan-reviewer — ทำลายได้ถ้ารวมเช่นเดียวกัน)
|
||
|
||
1. แถวเดี่ยวเลือกจบ 4 เมนู (3451-3455, 3541-3549, 3650-3654, 3869-3873) +
|
||
`finishCustom` (3695-3698) **ปิดเมนูไม่ผ่าน `markMenuClosed` โดยเจตนา** —
|
||
tap ถัดไปต้องวางรายการได้ทันที (กลืน tap 300ms จะฆ่า flow วางตรายาง)
|
||
2. คีย์ B ปิดเมนูไม่ mark — คีย์บอร์ดไม่มี click หลงเหลือ
|
||
3. Escape สองชั้น (1973-1979 มี stopPropagation) — ครั้งแรกปิดเมนูอย่างเดียว
|
||
4. ลำดับ guard ของ `handlePageClick` (2916-2988): isCollapsed → 300ms → ปิด
|
||
bookmark/pin → deselect → branch ตาม tool — ห้ามสลับ
|
||
5. `menuOpenWRef` ตั้ง 2 ทาง (openToolMenu + positionBookmarkPopup) — resize
|
||
เฉพาะความกว้างครอบ popup bookmark ด้วย
|
||
6. `selfScrollUntilRef` ต้องไม่ trigger การปิดเมนู (1529, 1586)
|
||
7. `stampCustomMode` reset เฉพาะตอนเปิดเมนูใหม่ผ่านปุ่ม (3717)
|
||
8. `scannedOnly` เป็น latch ของ `pageSpanCount` — ไม่ลบตอน virtualization unmount
|
||
|
||
## ลำดับการทำเมื่ออนุมัติ
|
||
|
||
1. ขั้น 1 (test เขียว) → 2. ขั้น 2 + findings #2/#5 → 3. findings #1 (คีย์ลัด
|
||
ไทย — แจ้ง behavior change ก่อน) → #3 → #4 → #6 → 4. ขั้น 4 เมื่อได้อาการ
|
||
ทุกขั้นจบด้วย: `tsc --noEmit` สะอาด + vitest ทั้งชุดผ่าน + sync
|
||
`.claude/rules/pdf-viewer.md` ถ้าพฤติกรรมเปลี่ยน + code review
|