refactor(client): extract clipboard helpers + useSwipeNav hook#261
Merged
Conversation
Replace 10 inline navigator.clipboard call sites with copyToClipboard / writeClipboardSilently / readClipboard in client/src/lib/clipboard.js, centralizing the insecure-context guard and the "Copy failed" toast. Pull MediaLightbox's touchStart/touchEnd swipe nav into a reusable hooks/useSwipeNav.js. Fix a pre-existing optimistic-checkmark bug in NextActionBanner: setCopied(true) now gates on the actual write result.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes a client-side refactor to deduplicate clipboard interactions into a shared helper module and to extract MediaLightbox swipe navigation into a reusable hook, reducing repeated logic across multiple UI surfaces.
Changes:
- Added
client/src/lib/clipboard.jswithcopyToClipboard,writeClipboardSilently, andreadClipboard, and migrated multiple call sites to use them. - Added
client/src/hooks/useSwipeNav.jsand updatedMediaLightboxto use the extracted swipe navigation handlers. - Updated several UI components/pages to use the new helpers, including a correctness fix in
NextActionBanner(copied indicator now gates on write success).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| PLAN.md | Removes the completed dedup/refactor checklist item from the plan. |
| client/src/pages/Shell.jsx | Uses readClipboard() to centralize paste behavior and fallback to manual input. |
| client/src/pages/RunsHistoryPage.jsx | Uses writeClipboardSilently() for “copy execution ID”. |
| client/src/pages/RapidReader.jsx | Uses readClipboard() for “paste from clipboard”. |
| client/src/pages/JiraReports.jsx | Uses writeClipboardSilently() for report copy action. |
| client/src/lib/clipboard.js | New centralized clipboard helper functions with consistent guard/toast behavior. |
| client/src/hooks/useSwipeNav.js | New hook encapsulating swipe thresholds and button-touch guarding. |
| client/src/components/media/MediaLightbox.jsx | Migrates swipe logic to useSwipeNav and copy logic to copyToClipboard. |
| client/src/components/digital-twin/tabs/ExportTab.jsx | Uses copyToClipboard() and gates copied UI state on success. |
| client/src/components/digital-twin/NextActionBanner.jsx | Uses writeClipboardSilently() and only shows “copied” after a successful write. |
| client/src/components/cos/tabs/AgentCard.jsx | Replaces inline clipboard+toast logic with copyToClipboard(). |
| client/src/components/apps/EditAppModal.jsx | Replaces inline clipboard+toast logic with copyToClipboard(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ton gate - JiraReports.handleCopy now bails with a 'Copy failed' toast when writeClipboardSilently returns false instead of unconditionally flashing the 'Copied' checkmark (mirrors the NextActionBanner fix). - useSwipeNav: isButtonTouch now optional-chains on .closest in case the touch target isn't an Element with closest available. - useSwipeNav.onTouchEnd no longer bails when the touch *ends* on a button. Origin gating is already handled by onTouchStart clearing the ref, so a swipe that starts on the surface and lifts over an inline control still registers — matching the documented behavior.
- copyToClipboard: accept successMessage=null to keep failure/insecure
toasts but suppress the success toast (callers with their own
"Copied" UI indicator).
- JiraReports.handleCopy: switch from writeClipboardSilently +
hand-rolled toast.error('Copy failed') to copyToClipboard(plain, null);
gains the insecure-context distinction the shared helper provides.
- clipboard.test.js: 14 unit tests covering all three helpers across
success / throw / unavailable / empty paths, plus the null-message
suppression path.
CI on PR #261 hit a flake where buildSeason's two separate `nowIso()` calls landed on different milliseconds, breaking the `createdAt === updatedAt` invariant the test asserts. Call once and assign both fields to the same timestamp. Unrelated to the JiraReports/clipboard changes in this PR, but the flake blocked CI on the most recent push.
A bare `navigator` reference throws ReferenceError in environments where `navigator` is fully absent (non-browser unit-test runners without the globalThis.navigator polyfill, SSR/prerender, some webviews) — optional-chaining can't catch it because the lookup fails before the chain runs. Read clipboard via `globalThis.navigator?.clipboard` and short-circuit the same way as when only `clipboard` is missing. Adds three test cases that stub `navigator` itself to undefined.
The pre-extraction MediaLightbox handler bailed in both onTouchStart AND onTouchEnd when the target was an inline button. The extracted hook only kept the origin-based gate, which lets a swipe that releases over a button double-fire as nav + a synthetic button click (e.g. swipe-end on the fullscreen toggle would also toggle fullscreen). Restore the end-target guard for parity with the prior UX.
clipboard.js: 'Clipboard unavailable on insecure context' is now only shown when globalThis.isSecureContext === false. Secure-origin failures (unsupported browser, Permissions Policy disabled, etc.) fall back to a generic 'Clipboard unavailable' so the message doesn't mislead. useSwipeNav.js: revert the touch-end isButtonTouch check added in the prior round and rewrite the comment to be honest about Touch.target semantics. Per spec, `e.target` on touchend is the *start* element, not where the finger released — so the second check just re-checked the start element and was redundant with the touchstart gate. Documented why we don't need a true end-target check (the SWIPE_MIN_PX threshold is well past tap-slop, so a real swipe won't synthesize a click on whatever button the finger releases over). Also adds two new clipboard test cases (insecure vs secure context error messages) and trims the now-incorrect insecure-on-undefined test.
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
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
Completes the Extract
useSwipeNavhook +lib/clipboard.jsdedup item from PLAN.md's Code quality section.client/src/lib/clipboard.jsexports three helpers —copyToClipboard(text, successMessage)(toasts),writeClipboardSilently(text)(returns boolean),readClipboard()(returns string or null). The insecure-origin guard (navigator.clipboard?.writeText) and the "Copy failed" toast now live in one place instead of being copy-pasted across 10 sites.client/src/hooks/useSwipeNav.jsencapsulatesMediaLightbox's horizontal swipe nav (touchStartref, button-touch guard, 50px / 1.2× horizontal-bias threshold).MediaLightbox,AgentCard,EditAppModal,NextActionBanner(×2),ExportTab,RunsHistoryPage,RapidReader,JiraReports,Shell.NextActionBanner: the copy-prompt buttons previously calledsetCopied(true)unconditionally — the green checkmark would lie if the write failed. Both sites now gate on the actual write result.Test plan
MediaLightboxon mobile, swipe left/right to nav between imagesShellpaste withclipboard.readTextavailable, denied, and unavailable — manual paste input should appear in the last two casesNextActionBannercopy-prompt buttons: checkmark only appears after a successful writeExportTabCopy button still works (local function renamed tohandleCopyExport)