fix(ui): clipboard copy for http://0.0.0.0:3000 and unify implementations#2487
fix(ui): clipboard copy for http://0.0.0.0:3000 and unify implementations#2487CommanderStorm merged 22 commits intomainfrom
Conversation
…host contexts Fixes #1673: Copy to clipboard does not work for http://0.0.0.0:3000 - Created unified copyToClipboard utility in lib/utils.ts with fallback - Uses navigator.clipboard.writeText when available (secure contexts) - Falls back to document.execCommand('copy') for non-HTTPS/non-localhost - Added comprehensive tests for the new utility - Refactored all 5 clipboard implementations to use unified utility: - tooltip-copy-text.tsx - copy-link-button.tsx - SpriteCanvas.tsx - sprite-download.tsx - style-integration-guide.tsx Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
- Position textarea off-screen with left/top instead of opacity - Add aria-hidden attribute for better accessibility - Use setSelectionRange for more reliable selection - Add focus() before selection for better compatibility Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
|
- Replace Object.defineProperty with vi.stubGlobal for navigator mocking - Use vi.unstubAllGlobals and vi.restoreAllMocks for proper cleanup - Add execCommand directly to document object (jsdom doesn't have it) - Avoid stubbing entire document object to prevent unexpected behavior Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
Addressed in c552e5c:
Clipboard implementations are now unified to a single |
|
@copilot |
1 similar comment
|
@copilot |
- Create useCopyToClipboard hook that encapsulates: - Calling copyToClipboard utility - Managing copied/copiedText state with auto-reset - Showing success/error toasts with customizable messages - Proper timeout cleanup to prevent memory leaks - Refactor all 5 components to use the new hook: - tooltip-copy-text.tsx - copy-link-button.tsx - SpriteCanvas.tsx - sprite-download.tsx - style-integration-guide.tsx - Add comprehensive tests for the new hook - Fix TypeScript error in utils.test.ts Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
Addressed in 5d8b3d8. Created a const { copy, copied, copiedText } = useCopyToClipboard({
successMessage: 'URL copied!',
errorMessage: 'Copy failed',
resetDelay: 2000,
});The hook handles:
All 5 components now use this single hook instead of duplicating the try/catch/toast pattern. |
Addressed in 5d8b3d8. Created a two-layer clipboard abstraction:
All 5 components now use the hook instead of duplicating clipboard handling logic:
|
- Add useEffect cleanup to clear timeout on unmount - Remove unused id parameter from CodeBlock component - Remove unused useId import - Fix object property sorting to satisfy biome linter Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
CommanderStorm
left a comment
There was a problem hiding this comment.
- handleCopy seems uneessesary and can be inlined
- martin/martin-ui/src/hooks/use-copy-to-clipboard.ts
- needs a pass to clean up the code. It is currently written very amateur-ish with way to many comments. reduce the size of the comments to a best practice level
- can reduce its options, for example always showing the error/success toast, making resetDelay standardised and hard coding the errroMessage. Only successMessage seems like a resonable option for this.
- Remove verbose JSDoc comments, keep single line description - Hardcode ERROR_MESSAGE and RESET_DELAY constants (no longer configurable) - Remove showSuccessToast, showErrorToast, errorMessage, resetDelay options - Keep only successMessage as configurable option - Remove unused reset() method from hook - Inline handleCopy in copy-link-button.tsx - Remove handleCopyUrl wrapper in sprite-download.tsx - Update tests to match simplified API Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
Addressed in 250384e:
|
Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
|
@copilot the sprite download page shares one Make sure this is not true on other pages as well. |
Add copiedValue to useCopyToClipboard hook to track which specific value was copied. This fixes the bug where copying one URL would show the green "Copied" icon on all items in the sprite-download page. Co-authored-by: CommanderStorm <26258709+CommanderStorm@users.noreply.github.com>
... Fixed in 9698664. Added The other components are fine:
|
|
do we actually need an indicator that the value was copied, if we also have a toast popping up? |
|
Well.. it looks nice 😉 |
|
sounds like a good reason :) |
There was a problem hiding this comment.
Pull request overview
This PR fixes clipboard copy functionality for non-secure contexts (http://0.0.0.0:3000) and unifies multiple near-duplicate clipboard implementations across the UI codebase by introducing a centralized utility and hook.
Key changes:
- Added
copyToClipboardutility with modern Clipboard API and legacyexecCommandfallback - Created unified
useCopyToClipboardhook with toast notifications and auto-reset state - Refactored 6 components to use the new unified implementation
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
martin/martin-ui/src/lib/utils.ts |
Adds centralized copyToClipboard function with fallback for non-secure contexts |
martin/martin-ui/src/hooks/use-copy-to-clipboard.ts |
New hook providing consistent clipboard API with toast notifications and state management |
martin/martin-ui/src/components/ui/tooltip-copy-text.tsx |
Refactored to use unified hook, removing duplicate clipboard logic |
martin/martin-ui/src/components/ui/copy-link-button.tsx |
Migrated to new hook with visual feedback (icon switching) |
martin/martin-ui/src/components/sprite/SpriteCanvas.tsx |
Simplified clipboard handling using new hook |
martin/martin-ui/src/components/error/error-state.tsx |
Updated styling to use gap utility for consistency |
martin/martin-ui/src/components/dialogs/style-integration-guide.tsx |
Refactored CodeBlock to use hook, removed per-block ID tracking |
martin/martin-ui/src/components/dialogs/sprite-download.tsx |
Migrated to hook with per-item copy state using copiedValue |
martin/martin-ui/__tests__/lib/utils.test.ts |
Added comprehensive tests for copyToClipboard including fallback scenarios |
martin/martin-ui/__tests__/hooks/use-copy-to-clipboard.test.tsx |
New test suite for the hook covering success, custom messages, and error cases |
martin/martin-ui/package-lock.json |
Version bump to 1.2.0 and automatic dependency resolution updates |
Files not reviewed (1)
- martin/martin-ui/package-lock.json: Language not supported
| variant: 'destructive', | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for the copiedValue state tracking. The hook exposes copiedValue which is critical for per-item copy state in sprite-download.tsx (checking if copiedValue === format.url), but there are no tests verifying this functionality. Tests should verify that copiedValue is set correctly and reset after the timeout.
| textarea.focus(); | ||
| textarea.setSelectionRange(0, text.length); |
There was a problem hiding this comment.
The textarea.focus() call in the fallback clipboard implementation could cause visual focus shifts for keyboard users. Since the textarea is aria-hidden and positioned off-screen, calling focus() might still trigger unexpected focus events or screen reader announcements. Consider using textarea.select() without an explicit focus() call, as select() implicitly focuses the element when needed for the copy operation.
| textarea.focus(); | |
| textarea.setSelectionRange(0, text.length); | |
| textarea.select(); |
| Copied | ||
| <CopyCheck className="w-3 h-3 dark:text-green-600" /> | ||
| </> | ||
| ) : ( | ||
| <> | ||
| Copy | ||
| <Copy className="w-3 h-3" /> |
There was a problem hiding this comment.
The icon and text order is inconsistent. In the "copied" state, the text comes before the icon ("Copied" then CopyCheck), but in the default state, the text comes before the icon ("Copy" then Copy icon). This is different from other components like sprite-download.tsx where the icon consistently appears before the text. Consider using a consistent order for better visual predictability.
| Copied | |
| <CopyCheck className="w-3 h-3 dark:text-green-600" /> | |
| </> | |
| ) : ( | |
| <> | |
| Copy | |
| <Copy className="w-3 h-3" /> | |
| <CopyCheck className="w-3 h-3 dark:text-green-600" /> | |
| <span className="ml-1">Copied</span> | |
| </> | |
| ) : ( | |
| <> | |
| <Copy className="w-3 h-3" /> | |
| <span className="ml-1">Copy</span> |
There was a problem hiding this comment.
Not inconsistent, this is on purpose. looks very janky otherwise
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
## 🤖 New release * `martin-tile-utils`: 0.6.8 -> 0.6.9 (✓ API compatible changes) * `mbtiles`: 0.15.0 -> 0.15.1 (✓ API compatible changes) * `martin-core`: 0.2.5 -> 0.2.6 (✓ API compatible changes) * `martin`: 1.2.0 -> 1.3.0 <details><summary><i><b>Changelog</b></i></summary><p> ## `mbtiles` <blockquote> ## [0.15.1](mbtiles-v0.15.0...mbtiles-v0.15.1) - 2026-01-27 ### Added - add MLT decoding support ([#2512](#2512)) - migrate our log library to tracing ([#2494](#2494)) ### Other - unignore `diff_and_patch_bsdiff` test with unique SQLite database names ([#2480](#2480)) - *(mbtiles)* remove the prefix-ism around how files are named for binary diff copy and simpify their naming ([#2478](#2478)) - *(mbtiles)* add assertion messages what we are checking to the copy tests ([#2477](#2477)) </blockquote> ## `martin-core` <blockquote> ## [0.2.6](martin-core-v0.2.5...martin-core-v0.2.6) - 2026-01-27 ### Added - migrate our log library to tracing ([#2494](#2494)) - *(martin-core)* Allow glyph ranges more than 0xFFFF ([#2438](#2438)) ### Fixed - *(perf)* Remove FontSources.masks as they were consuming large amounts of memory even when no font sources were set ([#2519](#2519)) - improve error message if no SVG sprite files are present ([#2516](#2516)) ### Other - move our imports to tracing ([#2500](#2500)) - *(deps)* shear our dependencys ([#2497](#2497)) </blockquote> ## `martin` <blockquote> ## [1.3.0](martin-v1.2.0...martin-v1.3.0) - 2026-01-27 ### Added - *(srv)* Add `route_prefix` configuration for native subpath support without the need of a reverse proxy override ([#2523](#2523)) - add MLT decoding support ([#2512](#2512)) - migrate our log library to tracing ([#2494](#2494)) - improve martin-cp progress output time estimate ([#2491](#2491)) - *(pg)* include ID column info for tables ([#2485](#2485)) - *(pg)* support PostgreSQL materialized views ([#2279](#2279)) - *(martin-core)* Allow glyph ranges more than 0xFFFF ([#2438](#2438)) ### Fixed - *(ui)* clipboard copy for http://0.0.0.0:3000 and unify implementations ([#2487](#2487)) - the `Copy` icon displaying nicely, next to the text and with enough padding ot all items ([#2483](#2483)) - update copy text to include icon for better visibility ([#2482](#2482)) - *(perf)* Remove FontSources.masks as they were consuming large amounts of memory even when no font sources were set ([#2519](#2519)) - improve error message if no SVG sprite files are present ([#2516](#2516)) ### Other - move our request logging to tracing ([#2508](#2508)) - move our imports to tracing ([#2500](#2500)) - *(deps)* shear our dependencys ([#2497](#2497)) - *(ui)* adjust margin for copy icon in URL component ([#2489](#2489)) - unignore `diff_and_patch_bsdiff` test with unique SQLite database names ([#2480](#2480)) - *(mbtiles)* remove the prefix-ism around how files are named for binary diff copy and simpify their naming ([#2478](#2478)) - *(mbtiles)* add assertion messages what we are checking to the copy tests ([#2477](#2477)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
copyToClipboardutility function inmartin-ui/src/lib/utils.tsnavigator.clipboard.writeText()document.execCommand('copy')for non-secure contextsuseCopyToClipboardhookcopiedboolean andcopiedValuefor per-item trackingsuccessMessageis configurableOriginal prompt
http://0.0.0.0:3000#2486✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.