[#214] react-pdf a태그 부모 요소 크기 설정#215
Conversation
✅ Changeset detectedLatest commit: 7149f69
If no version change is needed, please add The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
|
NPM Packages📦 @naverpay/react-pdfTotal Sizes: 22 kB Total Changes: +24 B (0%)
ℹ️ View Unchanged Files
powered by: naverpay size-action |
|
/canary-publish |
1 similar comment
|
/canary-publish |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
The annotation layer size fix is correct in intent, but the inline comment flags a nit: Math.floor() should be applied to match Canvas.tsx and Text.tsx patterns.
Extended reasoning...
Overview
This single-file PR adds explicit viewport-based width/height to the annotation layer's first child element (lines 79-80) and adds cursor: pointer to <a> tags inside the annotation layer (line 91). It fixes issue #214 where the parent element of <a> tags lacked explicit dimensions, causing hit-area misalignment.
Security Risks
No security risks. The changes are purely CSS style assignments on DOM elements rendered from PDF annotations.
Level of Scrutiny
Low-to-medium. The fix is straightforward and addresses a real rendering issue. The only concern is the consistency nit already flagged by the inline comment: Canvas.tsx (line 37-38) and Text.tsx (line 99-100) both apply Math.floor() to viewport dimensions before converting to CSS pixels, but the new code does not. This is a minor nit — the sub-pixel difference is visually invisible — but fixing it would keep the codebase consistent.
Other Factors
The cursor: pointer addition is clearly correct. The parent-size fix is logically sound. No test changes are included, which is expected for a layout/style fix. The overall change is small and self-contained.
|
/canary-publish |
There was a problem hiding this comment.
The Math.floor() fix was applied, but the concern about setting viewport dimensions on children[0] (the first annotation's section element) instead of the layer container was marked resolved without a code change — the structural issue is still present.
Extended reasoning...
Overview
This PR modifies Annotation.tsx to add explicit width/height sizing to an annotation DOM element, plus a cursor:pointer style on anchor tags. The changeset file is a routine patch entry.
Prior Review History
Two earlier passes from this bot raised concerns. The first flagged missing Math.floor() which was correctly addressed in commit d90d6cb. A third-pass inline comment at line 83 raised a structural issue: in pdfjs-dist v4, AnnotationLayer.render() appends individual section elements directly to the container div with no intermediate wrapper. children[0] is therefore the first annotation's section, not a parent container. The author marked that inline comment as resolved, but the code remains unchanged.
Security Risks
No auth, crypto, or permissions code is touched. The risk is functional correctness — link hit areas and cursor styling — not a security concern.
Level of Scrutiny
The fix is in the PDF annotation layer rendering path. Setting full-viewport dimensions on the first annotation's section can expand its click target to cover the entire page, and subsequent annotations' anchor tags never receive cursor:pointer or positional overrides. This warrants a human review before merging.
Other Factors
The inline comment was resolved without a corresponding code change. Since resolved comments are collapsed in the GitHub UI, the concern may not be visible to other reviewers. A human reviewer should verify whether the firstChildren assumption is valid for the pdfjs-dist version in use, or whether the fix should target element directly.
|
/canary-publish |
This comment was marked as outdated.
This comment was marked as outdated.
부모 요소(element)에 viewport 크기를 설정하여 모든 annotation의 a 태그가 올바른 클릭 영역을 가지도록 개선했습니다. 변경사항: - annotation layer 컨테이너에 viewport 기반 크기 설정 - 첫 번째 annotation만이 아닌 모든 a 태그에 스타일 적용 - cursor: pointer 스타일 추가로 링크 UX 개선 - unused variable 제거 및 타입 캐스팅 간소화 - 코드 의도를 명확히 하는 주석 추가 이전 코드는 children[0](첫 번째 annotation section)에만 크기를 설정했지만, 이제는 element 자체에 설정하여 모든 annotation의 a 태그가 올바르게 처리됩니다.
|
/canary-publish |
This comment was marked as outdated.
This comment was marked as outdated.
annotation layer 컨테이너에 position: absolute를 적용하여 TextLayer와 동일한 패턴으로 normal flow에서 제거. 명시적 width/height 설정 시 페이지 높이가 2배로 늘어나는 문제 방지. CSS Module 스코핑으로 동작하지 않는 cursor: pointer 규칙 제거.
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
viewport 크기 동기화 방식을 TextLayer와 동일하게 inline style로 변경하여 클릭 영역 정확성을 보장하고, 불필요한 스타일을 제거하여 안정성을 높였습니다. 변경사항: - annotation layer 크기 설정을 return 문 inline style로 이동 - SCSS에서 width/height 100% 제거 (동적 viewport 동기화 불가) - a 태그 스타일 간소화 (cursor: pointer만 유지) - annotation layer 구조 개선 (position: relative 명시) 이전 방식은 CSS 100%로 인한 viewport 불일치와 스케일 변경 시 클릭 영역 버그가 있었으나, 이제 TextLayer와 동일한 패턴으로 안정적인 동작을 보장합니다.
SCSS와 inline style 간 충돌을 제거하고 스타일을 정리하여 코드 일관성과 유지보수성을 개선했습니다. 변경사항: - SCSS에서 width/height 100% 제거 (inline style과 충돌 방지) - cursor: pointer를 SCSS로 통합 (스타일 일관성) - 불필요한 a 태그 처리 코드 제거 - error handling 주석 개선 이제 annotation layer는 TextLayer와 동일한 패턴으로 viewport 크기를 동기화하고, 스타일은 SCSS에서 일관되게 관리됩니다.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
4 participants
Related Issue
Describe your changes
실제로 a 태그의 부모 요소 즉, firstChildren 에 width 와 height 가 설정되지 않아서 a 태그의 width: 100% 가 제대로 적용되지 않는 문제
Request