Enhance AddSignature component with per-page positioning and fix drag interactions#44
Enhance AddSignature component with per-page positioning and fix drag interactions#44sumitsahoo merged 2 commits intomainfrom
Conversation
…and refactor position handling
… drag interactions
There was a problem hiding this comment.
Pull request overview
Enhances the PDF signing tool to support per-page signature placement (or a shared placement across all pages), improves the preview drag positioning behavior, and updates PDF embedding logic to handle per-page coordinates.
Changes:
- Add per-page signature position tracking in the
AddSignaturetool while preserving an “apply to all pages” option. - Extend
addSignaturePDF operation to accept either a single position or a per-page position map. - Prevent Android Chrome long-press context menu interference during sortable touch interactions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/tools/AddSignature.tsx |
Tracks signature position per page and updates drag + apply logic to generate per-page coordinates. |
src/utils/pdf-operations.ts |
Extends addSignature to support per-page coordinates via Map<number, Position>. |
src/hooks/useSortableDrag.ts |
Adds onContextMenu prevention to reduce Android long-press interference during drag. |
| // Single shared position | ||
| const page = pdfDoc.getPage(0); | ||
| const { width: pageWidth, height: pageHeight } = page.getSize(); | ||
| const x = (position.xPercent / 100) * pageWidth - sigSize.width / 2; | ||
| const y = (position.yPercent / 100) * pageHeight - sigSize.height / 2; |
There was a problem hiding this comment.
When applying to all pages, the PDF coordinates are computed using page 0’s dimensions and then reused for every page. If the PDF contains pages with different sizes, the signature won’t stay at the same percentage position (and may end up off-page). Consider computing coordinates per page (e.g., build a per-page position map even for apply-to-all) or moving the percent->points conversion into addSignature so it can use each page’s size.
| export async function addSignature( | ||
| file: File, | ||
| signatureDataUrl: string, | ||
| pageIndices: number[], | ||
| position: Position, | ||
| position: Position | Map<number, Position>, |
There was a problem hiding this comment.
The JSDoc for addSignature still describes position as a single { x, y, width, height } applied to every page, but the function now accepts a Map<number, Position> with per-page coordinates. Please update the doc (and param description) to reflect the new union type and behavior.
| for (const idx of pageIndices) { | ||
| const fallback = isMap ? position.values().next().value : position; | ||
| const pos = isMap ? (position.get(idx) ?? fallback) : position; | ||
| if (!pos) continue; |
There was a problem hiding this comment.
For the Map case, missing entries fall back to the first Map value, and an empty Map silently results in no signatures being drawn (pos becomes undefined and the loop continues). This fallback behavior is fragile/implicit for API consumers—consider validating that the map contains positions for all pageIndices (or providing an explicit default position) and handling the empty-map case with a clear error.
| for (const idx of pageIndices) { | |
| const fallback = isMap ? position.values().next().value : position; | |
| const pos = isMap ? (position.get(idx) ?? fallback) : position; | |
| if (!pos) continue; | |
| if (isMap) { | |
| if (position.size === 0) { | |
| throw new Error("Signature position map must not be empty."); | |
| } | |
| const missingPageIndices = pageIndices.filter((idx) => !position.has(idx)); | |
| if (missingPageIndices.length > 0) { | |
| throw new Error( | |
| `Signature position map is missing positions for page indices: ${missingPageIndices.join(", ")}`, | |
| ); | |
| } | |
| } | |
| for (const idx of pageIndices) { | |
| const pos = isMap ? position.get(idx)! : position; |
| // Prevent Android Chrome long-press context menu ("Open in new tab" etc.). | ||
| onContextMenu(e: React.MouseEvent) { | ||
| e.preventDefault(); | ||
| }, |
There was a problem hiding this comment.
onContextMenu is unconditionally prevented for all draggable items, which also disables the right-click context menu on desktop (not just Android long-press). Consider gating preventDefault() to touch-driven interactions (e.g., only after a touchstart/drag is detected) so standard desktop context menus remain available.
This pull request enhances the signature placement workflow in the PDF signing tool by introducing per-page signature positioning. Users can now set a different signature position for each selected page, or use a single shared position for all pages. The changes also improve the drag-and-drop experience and update the PDF generation logic to support per-page positions.
Signature placement and user experience improvements:
AddSignature.tsx, allowing users to specify a unique signature position for each page when not applying to all pages. The UI and drag logic now use apagePositionsmap to track positions per page, and the help text reflects this new behavior. [1] [2] [3] [4] [5] [6] [7] [8] [9]PDF generation logic:
addSignaturefunction inpdf-operations.tsto accept either a single position or a map of positions, and apply the correct coordinates for each page during PDF generation. [1] [2]Cross-platform drag-and-drop fixes:
onContextMenuevent inuseSortableDrag.