feat: add figure reference links, panel jumps, and crop flow#17
Open
enu3379 wants to merge 9 commits into
Open
feat: add figure reference links, panel jumps, and crop flow#17enu3379 wants to merge 9 commits into
enu3379 wants to merge 9 commits into
Conversation
📝 WalkthroughWalkthrough피규어 스캔·멘션 링크·PDF 목적지 이동·카드 UI·수동 크롭 기능을 새 구조로 통합했습니다. 캡션 앵커 및 수동 영역 병합, 세대 기반 스캔 가드, 접근성 동작과 관련 테스트·문서도 갱신됐습니다. ChangesFigure UX 통합
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant PDF
participant PdfHost
participant Viewer
participant FiguresTab
participant CropMode
PDF->>PdfHost: 내부 destination 전달
PdfHost->>Viewer: resolved destination 전달
Viewer->>FiguresTab: figure 패널 열기 및 활성 상태 갱신
FiguresTab->>Viewer: figure 또는 mention 점프 요청
Viewer->>CropMode: 크롭 모드 시작
CropMode->>Viewer: 선택한 PDF region 전달
Viewer->>FiguresTab: crop preview 및 저장 결과 반영
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Static + runtime review of G1-G8 passed (arXiv test paper: scan, ref click panel flow, origin chip return, region/band flashes, crop enter/Esc, no console errors). Card-click now keeps the panel open by spec (v1.1) so the return chip stays reachable. Follow-ups recorded: origin chip for hyperref-mediated opens, render-region page LRU. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The text layer is a transparent copy of the canvas glyphs; giving mgn-ref links a text color made that copy visible and it rendered as smeared double text over the real glyphs (and the opaque hover/active background hid them). Keep link text transparent and mark references with a dotted underline plus translucent tint only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The scan text concatenates pdf.js items with no separator, so page text
can lose inter-word spaces ("threereviewers" on the test paper's p.15).
Caption matching then failed, captionAnchor stayed undefined, and the
figure's own caption label was counted as a body mention (Figure 2
showed 2 mentions instead of 1). Match captions ignoring whitespace
entirely, mapping back to original offsets; add a regression test with
the measured page text. Remaining word-boundary limitation recorded as
FG-R4 in docs/figure-ux.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
enu3379
force-pushed
the
feature/figure-ux
branch
from
July 13, 2026 13:25
554e8b7 to
5a30249
Compare
Capture the clicked annotation link's page+y in a capture-phase listener, match it to the nearest same-page mention of the resolved figure, and pass it as originRefKey so hyperref-opened panels get the return chip too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
enu3379
force-pushed
the
feature/figure-ux
branch
from
July 13, 2026 13:35
5a30249 to
6fb1b66
Compare
Real-device QA: with the panel unpinned, clicking a card closed the panel and took the return chip with it - violating the amended v1.1 pin rule (card jumps keep the panel; mention-chip return still closes when unpinned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/progress.md (1)
74-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value"실기기 QA" 잔여 표현이 다소 부정확함.
figure-ux.md§7.1(같은 날짜 2026-07-13 갱신)은 이미 QA 항목 1·3·4·5·6을 통과하고 F1 버그도 수정 완료했다고 기록합니다. 실제 잔여는 §7의 2·7(샘플 미확보)뿐인데, 이 줄의 "남은 것: PR 머지, 실기기 QA"라는 표현은 실기기 QA가 전혀 진행되지 않은 것처럼 읽힐 수 있습니다. "§7 2·7(샘플 미확보) 잔여"처럼 구체화하면 더 명확할 것 같습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/progress.md` around lines 74 - 75, Update the “피규어 UX 개정” entry in docs/progress.md to replace the broad “실기기 QA” remaining-work wording with a precise reference to figure-ux.md §7 items 2 and 7, which remain pending because samples are unavailable. Preserve the existing PR merge note and links.test/tab-figures.test.ts (1)
148-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueconsole.error spy가 복원되지 않음.
vi.spyOn(console, 'error').mockImplementation(...)이 이 테스트 안에서mockRestore()되지 않습니다. 전역afterEach에vi.restoreAllMocks()설정이 없다면 같은 파일의 이후 테스트로 spy가 새어 나갈 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/tab-figures.test.ts` at line 148, Restore the console.error spy created in the test around the `vi.spyOn(console, 'error')` call by invoking `mockRestore()` during cleanup, or use the file’s existing cleanup mechanism if one is present. Ensure the spy is restored before subsequent tests run, without changing the test’s assertions.src/viewer/panel/tab-figures.ts (1)
200-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
role="button"요소 안에 네이티브<button>중첩.
preview(role=button, tabindex=0) 내부에 크롭<button>을 넣어 인터랙티브 요소가 중첩됩니다. 이는 HTML/ARIA상 유효하지 않은 중첩으로, 스크린리더에서 크롭 버튼 인식/포커스 처리가 불안정할 수 있습니다. 크롭 버튼을preview의 형제 요소로 분리하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/viewer/panel/tab-figures.ts` around lines 200 - 217, In the figure preview construction, separate the crop button from the interactive preview element created in the surrounding render logic. Keep preview as the jump target and make the crop button its sibling rather than appending it inside preview, preserving the existing crop action, figure identifier, label, and button accessibility attributes.src/core/render-region.ts (1)
19-31: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift페이지 전체 렌더 대신 영역 렌더로 메모리 피크를 줄이세요
pageCanvas를 페이지 전체 크기로 만든 뒤drawImage로 잘라내고 있어, 작은 figure도 고배율 렌더 시 큰 캔버스 메모리를 불필요하게 씁니다.pdf.js의 offset/transform viewport로 대상 영역만 렌더하면 crop 크기로 캔버스를 제한할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/render-region.ts` around lines 19 - 31, Update the rendering flow around pageCanvas and page.render to render only the normalized target region using pdf.js offset/transform viewport options, rather than allocating a full-page canvas and cropping afterward. Size the canvas directly from the target crop dimensions while preserving the existing normalized rectangle orientation and output behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/implementation-plan.md`:
- Around line 265-267: 상단 상호작용 규칙 R8의 크롭 진입 설명을 그림·표 탭의 ‘영역 지정’ 버튼이 아니라 카드 이미지
우상단 호버 크롭 아이콘(⌗)을 사용하는 방식으로 수정하세요. 상태 머신과 §8에 반영된 진입 동작과 일치시키고, R8의 나머지 규칙은
유지하세요.
In `@src/viewer/viewer.css`:
- Line 373: Rename the mgnJumpFlash keyframe to a kebab-case name and update
every corresponding animation reference, including the declarations near the
highlighted lines, so the `@keyframes` definition and all usages remain consistent
and pass stylelint.
---
Nitpick comments:
In `@docs/progress.md`:
- Around line 74-75: Update the “피규어 UX 개정” entry in docs/progress.md to replace
the broad “실기기 QA” remaining-work wording with a precise reference to
figure-ux.md §7 items 2 and 7, which remain pending because samples are
unavailable. Preserve the existing PR merge note and links.
In `@src/core/render-region.ts`:
- Around line 19-31: Update the rendering flow around pageCanvas and page.render
to render only the normalized target region using pdf.js offset/transform
viewport options, rather than allocating a full-page canvas and cropping
afterward. Size the canvas directly from the target crop dimensions while
preserving the existing normalized rectangle orientation and output behavior.
In `@src/viewer/panel/tab-figures.ts`:
- Around line 200-217: In the figure preview construction, separate the crop
button from the interactive preview element created in the surrounding render
logic. Keep preview as the jump target and make the crop button its sibling
rather than appending it inside preview, preserving the existing crop action,
figure identifier, label, and button accessibility attributes.
In `@test/tab-figures.test.ts`:
- Line 148: Restore the console.error spy created in the test around the
`vi.spyOn(console, 'error')` call by invoking `mockRestore()` during cleanup, or
use the file’s existing cleanup mechanism if one is present. Ensure the spy is
restored before subsequent tests run, without changing the test’s assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16973b91-cd24-4672-adf3-bac4e27266de
📒 Files selected for processing (17)
docs/figure-ux.mddocs/implementation-plan.mddocs/progress.mdsrc/core/figures.tssrc/core/mentions.tssrc/core/render-region.tssrc/core/types.tssrc/viewer/crop-mode.tssrc/viewer/jump.tssrc/viewer/main.tssrc/viewer/panel/tab-figures.tssrc/viewer/pdf-host.tssrc/viewer/viewer.csstest/figures.test.tstest/jump.test.tstest/mentions.test.tstest/tab-figures.test.ts
Comment on lines
265
to
+267
| 상태 머신: `idle → armed(figId) → dragging → preview → idle` | ||
|
|
||
| - 진입: 그림·표 탭 상세의 버튼 — region 있으면 "영역 다시 지정", 없으면 "영역 지정". 진입 시 대상 페이지로 스크롤(기존 region 또는 캡션 위치). | ||
| - 진입: 그림·표 카드 이미지 우상단 호버 크롭 아이콘(⌗). 진입 시 대상 페이지로 스크롤(기존 region 또는 캡션 위치). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
R8(27행대) 규칙이 이번에 갱신된 크롭 진입 방식과 불일치.
이 구간에서 크롭 진입을 "카드 이미지 우상단 호버 크롭 아이콘"으로 명시했고 289행의 §8도 같은 방식으로 갱신됐지만, 상단의 상호작용 규칙 R8(변경되지 않은 27-41행 구간)은 여전히 "그림·표 탭의 '영역 지정' 버튼으로 진입"이라고 서술되어 있습니다. figure-ux.md도 "§8의 '영역 지정/다시 지정' 텍스트 버튼은 아이콘으로 대체... 계획서 §8 서술 개정 필요(G8)"라고 명시했는데, §8은 갱신됐으나 R8은 빠진 것으로 보입니다. R8 문구도 아이콘 진입 방식으로 맞춰주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/implementation-plan.md` around lines 265 - 267, 상단 상호작용 규칙 R8의 크롭 진입 설명을
그림·표 탭의 ‘영역 지정’ 버튼이 아니라 카드 이미지 우상단 호버 크롭 아이콘(⌗)을 사용하는 방식으로 수정하세요. 상태 머신과 §8에 반영된
진입 동작과 일치시키고, R8의 나머지 규칙은 유지하세요.
| position: absolute; | ||
| z-index: 5; | ||
| pointer-events: none; | ||
| animation: mgnJumpFlash 1.2s ease-out forwards; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
stylelint 실패 방지: keyframe 이름을 kebab-case로 변경하세요.
mgnJumpFlash는 keyframes-name-pattern 규칙 위반으로 stylelint 에러입니다. @keyframes 이름과 참조를 함께 변경하세요.
🔧 제안 수정
- animation: mgnJumpFlash 1.2s ease-out forwards;
+ animation: mgn-jump-flash 1.2s ease-out forwards;-@keyframes mgnJumpFlash {
+@keyframes mgn-jump-flash {
0% { opacity: 1; }
100% { opacity: 0; }
}Also applies to: 389-389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/viewer/viewer.css` at line 373, Rename the mgnJumpFlash keyframe to a
kebab-case name and update every corresponding animation reference, including
the declarations near the highlighted lines, so the `@keyframes` definition and
all usages remain consistent and pass stylelint.
Source: Linters/SAST tools
This was referenced Jul 15, 2026
onetwothr1
added a commit
that referenced
this pull request
Jul 15, 2026
## Summary figure-preview-test 엔진 **v2.5.1**을 byte-identical 벤더링. PDFViewer#12 메모리 대응의 **엔진 측** 작업을 land. ## Changes - `src/core/fig-extract.js` — v2.5.1 byte-identical 복사 (엔진 repo에서만 수정, 여기선 복사) - `figure.canvas`(페이지 전체 렌더 보관) 폐지 → `figure.cropCanvas`(그림 영역만 즉시 크롭). figure마다 ~9.4MB scale-2.2 페이지 캔버스를 물던 상주 구조 제거 → 스캔 중 페이지 캔버스 동시 상주 **최대 1장** - 페이지 스캔 상한 기본값(60) 제거 — 미지정 시 전체 페이지 스캔, `opts.maxPages`는 선택적 상한으로 유지 - `opts.signal` abort 시 진행 중 페이지 렌더도 `RenderTask.cancel()`로 즉시 중단 (페이지 경계 대기 제거) - `src/core/fig-engine.ts` — `EngineFigure.canvas`→`cropCanvas`, `maxPages`·`signal` 주석 동기화 - `docs/fig-extract-integration.md` — 크롭 캔버스 수명·취소 가이드 갱신 (호스트가 문서 교체 시 signal abort 필수임을 명시) - `test/*` — mock figure의 `canvas`→`cropCanvas` ## #12 acceptance criteria - [x] Full-page canvases are not retained for every figure-bearing page after previews are produced — `cropCanvas`로 대체, 페이지 캔버스 미보관 - [x] Engine changes are made upstream first, then vendored byte-identically with a version bump — 엔진 repo v2.5.1 후 byte-identical 벤더링 - [x] `npm run typecheck` · `npm test`(25) · `npm run build` 통과 - [ ] Peak retained page render surfaces bounded (LRU) — 엔진 피크는 1장으로 bound. Margin `render-region.ts` LRU 3장은 후속 - [ ] Document replacement cancels the previous extraction — 엔진 메커니즘 완비(signal + in-flight 렌더 취소). **`main.ts`가 `AbortController`로 signal 전달·abort하는 wiring은 후속** (#17 머지 후) - [ ] Preview Blob/Object URLs revoked — Margin 측 (현행 dataURL 캐시 `setDocument` clear로 충족 검토) - [ ] Long figure-heavy PDF memory QA documented — 후속 ## Test plan - [x] `npm run typecheck` - [x] `npm test` (6 files, 25 tests) - [x] `npm run build` - [ ] 런타임 스캔 검증 — 벤더 엔진은 탐지·bbox 로직 무변경(엔진 repo에서 patch로 판정, 물리 diff 0 기대). Margin 통합 런타임은 후속 wiring PR에서 ## Notes 후속 Margin wiring(`main.ts` signal 전달, `render-region.ts` LRU, 메모리 QA)은 **#17(figure-ux) 머지 후** 별도 PR. 이 PR은 엔진 vendoring + 타입/문서 동기화만. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **개선 사항** * 문서 전체 페이지를 기본으로 스캔하도록 `maxPages` 동작을 개선했습니다. * 작업 취소 시 진행 중인 페이지 렌더링이 즉시 중단됩니다. * 그림별 크롭 캔버스를 사용해 미리보기 생성과 메모리 관리 효율을 높였습니다. * 중복 그림을 더 일찍 제거하고, 불필요한 페이지 전체 캔버스 보관을 줄였습니다. * **문서** * 취소 처리와 캔버스 수명 관리 규칙을 최신 동작에 맞게 업데이트했습니다. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
본문 피규어 참조(주입 링크·PDF 내장 링크·캡션 라벨)를 패널 경유 탐색으로 연결하고(DC-F1-B), 점프 정렬 규칙(텍스트 상단 1/8·피규어 세로 중앙)과 플래시, 수동 크롭 진입, 원점 칩(↩) 복귀 UX를 구현합니다. 설계 정본은 docs/figure-ux.md, 구현 리뷰는 §6.1, 실기기 QA 결과는 §7.1.
Changes
jump.ts), 멘션 스캔·주입(mentions.ts), 내장 링크 인터셉트(MarginLinkService), 그림·표 탭 개편(언급 칩·원점 칩·크롭 아이콘), 수동 크롭 모드, 캡션 라벨 링크, 계획서 개정nearestFigureMention으로 같은 페이지의 가장 가까운 언급을 원점 칩으로 지정 — arXiv처럼 참조가 hyperref인 문서에서도 복귀 UX 동작 (3초 TTL 일회성 소비)ensureScanned, 스캔 세대 가드(탭), 문서 전환 시 엉뚱한 버킷 저장 방지 가드(scanFigures)replaceChildren+textContent화, 실패 상태 렌더 가드.tab-figures.test.ts를 새 클래스(콜백 주입) 기준으로 재작성(3개 의도 유지),requireFigExtract·fig-engine.test.ts수용jumpToFigure의closePanel()제거 (§3 v1.1 규칙 준수; 언급 칩 복귀의 핀 OFF 닫힘은 유지)Test plan
npm run typecheck·npm test·npm run buildChecklist
feat: …— squash 커밋 메시지가 됨)npm run typecheck·npm test통과🤖 Generated with Claude Code