feat: make download buttons real links (copy-link-address works)#409
feat: make download buttons real links (copy-link-address works)#409
Conversation
Wraps the Markdown and PDF download buttons in anchor tags via `Button asChild`, so right-click "Copy Link Address" and modifier-click (cmd/ctrl/shift/alt) or middle-click work natively. Left-click still uses the fetch+blob path to force the short-hash filename and download. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdded an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DownloadModal/DownloadModal.tsx`:
- Around line 60-69: handleLinkClick currently relies on the isProcessing React
state which can race on rapid clicks; add a synchronous ref latch (e.g.,
useRef<boolean>(false)) and check/ref-set it inside handleLinkClick before
calling fetchAndSave to prevent duplicate starts, then clear the latch in a
finally block after fetchAndSave completes (or on error); keep the existing
isProcessing state updates if needed but ensure the ref is the first guard so
two quick clicks cannot both proceed — reference handleLinkClick, isProcessing,
and fetchAndSave when making the change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c269d0dc-1392-4cf8-bf6e-d716d091084a
📒 Files selected for processing (1)
src/components/DownloadModal/DownloadModal.tsx
Visual Regression Test Report ✅ PassedGithub run id: 24569172358 🔗 Artifacts: Download |
`isProcessing` is React state, so two rapid clicks within the same render cycle could both pass the guard and start duplicate downloads. Use a ref as the synchronous gate inside both `handleLinkClick` and `fetchAndSave`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DownloadModal/DownloadModal.tsx`:
- Line 13: Both async paths must use the same processing latch: ensure the
shared ref isProcessingRef is checked and set at the start of every entry point
(fetchAndSave, the PDF generation handler—e.g.,
generatePdf/handleGeneratePdf—and the anchor click/download handler) to prevent
concurrent starts; return early if isProcessingRef.current is true, set
isProcessingRef.current = true and setIsProcessing(true) at the start, wrap the
async work in try/finally, and in finally set isProcessingRef.current = false
and setIsProcessing(false) so the modal state is only toggled when the guarded
work completes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c11168d-de2c-48fa-8d47-cd74ef34f11c
⛔ Files ignored due to path filters (7)
tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-options-menu-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-options-menu-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-closed-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-view-opened-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-view-via-url-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-view-via-url-light-mode-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/components/DownloadModal/DownloadModal.tsx
The ref guard previously only covered `fetchAndSave`, so clicking Generate PDF didn't block a concurrent download click and whichever finished first flipped `isProcessing` off mid-flight. Mirror the same ref check/set in `handleGeneratePdf` so both async paths share the latch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DownloadModal/DownloadModal.tsx`:
- Around line 71-79: The click handler handleLinkClick currently checks for
modifier keys before checking isProcessingRef, so modified clicks will still
navigate while the component is busy; change the logic to return immediately if
isProcessingRef.current is true (i.e., gate on the busy state first) and only
perform the modifier-key early-return when not processing, and apply the same
fix to the other download link click handlers in this file that call
fetchAndSave so modifier-clicks are inert while processing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41d1f158-fed1-4944-807a-589d116f4b7f
⛔ Files ignored due to path filters (2)
tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-view-opened-light-mode-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/components/DownloadModal/DownloadModal.tsx
Summary
DownloadModalare now rendered as real<a>tags (viaButton asChild), so right-click → Copy Link Address / Open in New Tab works natively.onClickstill callspreventDefault()and uses the existingfetch+Blobpath so the file is downloaded with the short-hash filename, same as before.Test plan
graypaper-<shortHash>.{md,pdf}filename.https://gp.fluffylabs.dev/graypaper-<fullHash>.{md,pdf}URL.🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Bug Fixes