Conversation
Replace navigator.clipboard.writeText() with a textarea + execCommand approach that works reliably on Safari/iPad OS. The Clipboard API silently resolves without writing to the clipboard on Safari iPad OS. Also fix description text extraction to use innerText instead of textContent to preserve line breaks from <br> tags. Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
…flex') selector Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes clipboard copy on the LinkedIn resume utility page, targeting Safari/iPadOS behavior where navigator.clipboard.writeText() can “succeed” without actually populating the clipboard.
Changes:
- Replace Clipboard API usage with a textarea +
document.execCommand('copy')copy path. - Preserve line breaks when copying descriptions by switching from
textContenttoinnerText. - Make DOM lookup for the description field less Tailwind-class-dependent by using
parentElementscoping.
You can also share your feedback on Copilot code review. Take the survey.
src/pages/resume/linkedin.astro
Outdated
| // iOS/iPad Safari requires both selection methods | ||
| textarea.select(); | ||
| textarea.setSelectionRange(0, textarea.value.length); | ||
|
|
There was a problem hiding this comment.
textarea.select() will move focus from the button to the temporary textarea; once the textarea is removed, keyboard users may lose focus context. Consider capturing the currently focused element before selecting and restoring focus afterward (or explicitly re-focusing the clicked copy button).
src/pages/resume/linkedin.astro
Outdated
| try { | ||
| await navigator.clipboard.writeText(textToCopy); | ||
| const success = copyToClipboard(textToCopy); |
There was a problem hiding this comment.
Copy behavior has been substantially changed (new copyToClipboard() path and innerText extraction), but the existing Playwright coverage for /resume/linkedin/ doesn’t currently assert that clicking a copy button updates the clipboard contents. Adding an E2E assertion around a representative copy button would help prevent regressions (especially in WebKit/Safari).
| const success = copyToClipboard(textToCopy); | |
| const success = copyToClipboard(textToCopy); | |
| (window as any).__lastCopiedText__ = textToCopy; |
src/pages/resume/linkedin.astro
Outdated
| function copyToClipboard(text: string): boolean { | ||
| const textarea = document.createElement('textarea'); |
There was a problem hiding this comment.
The inline <script> now contains TypeScript type annotations (e.g., text: string, : boolean). Unless this script is explicitly compiled as TypeScript (e.g., via a lang="ts" attribute or moving the logic into a .ts module), this will be invalid JavaScript and will break the build in browsers/bundling.
|
@copilot implement code review feedback. |
…clipboard and focus - Restore focus to the clicked button after textarea is removed to prevent keyboard users from losing focus context - Add E2E test verifying clipboard content matches data-copy attribute - Add E2E test verifying focus is restored to the copy button after click - TypeScript in inline <script> is valid: Astro processes scripts through its bundler which strips type annotations at build time Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Addressed all three review items in 0b31d17:
|
|
@copilot implement code review feedback. |
Extract inline TypeScript from linkedin.astro into src/scripts/linkedin-copy.ts, addressing the code review concern about TypeScript type annotations in inline <script> tags. Follows the existing src/scripts/navToggle.ts pattern. Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com> Agent-Logs-Url: https://github.com/benbalter/benbalter.github.com/sessions/7144846b-b2c5-4881-a786-3b74dfee13fa
Moved the clipboard TypeScript logic from the inline |
|
@copilot fix failing tests |
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com> Agent-Logs-Url: https://github.com/benbalter/benbalter.github.com/sessions/735cc370-3cc9-4263-98b5-030f6179504b
|
@copilot merge in |
# Conflicts: # src/pages/404.astro
navigator.clipboard.writeText()silently resolves on Safari iPad OS without actually writing to the clipboard — the checkmark shows but the clipboard is blank.Changes
execCommand('copy')via a temporary off-screen<textarea>. The modern API can't be used with a catch fallback since Safari resolves successfully with empty content.textarea.select()andsetSelectionRange()— iPad Safari requires both. Textarea is setreadonlyto suppress the keyboard.textContentstrips<br>tags, so bullet points ran together. Switched toinnerTextwhich preserves them..closest('.flex')(Tailwind utility class) with.parentElement.linkedin.astrointosrc/scripts/linkedin-copy.ts, following the existingsrc/scripts/navToggle.tspattern for proper TypeScript compilation.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.