-
Notifications
You must be signed in to change notification settings - Fork 148
feat(JsonViewCard): Add click-to-copy for JSON keys and values #1332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for aptos-explorer ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds click-to-copy functionality to the JSON viewer component, allowing users to copy individual JSON keys and values by clicking on them. The implementation uses a custom useCopyTooltip hook for tooltip state management and event delegation to handle clicks on react-json-view DOM elements.
Key changes:
- Extracted
useCopyTooltiphook for managing tooltip visibility, copied state, and timer-based interactions - Added
findCopyableElementhelper function to identify clickable JSON keys and values in the DOM - Integrated event handlers (onClick, onMouseOver, onMouseOut, onMouseLeave) to enable click-to-copy with visual feedback
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const span = target.closest("span") as HTMLElement | null; | ||
| if (span?.parentElement?.classList.contains("variable-value")) { | ||
| return { | ||
| text: (span.textContent || "").replace(/^"|"$/g, ""), |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern /^"|"$/g uses the global flag g which is unnecessary for a single replacement operation on start and end quotes. The global flag can cause unexpected behavior with the replace method. Use /^"|"$/ instead (without the g flag).
| const handleMouseOver = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| const copyable = findCopyableElement(e.target as HTMLElement); | ||
| if (!copyable || copied) return; | ||
|
|
||
| setAnchor(copyable.element); | ||
| if (!hoverTimer.current) { | ||
| hoverTimer.current = window.setTimeout(() => { | ||
| setOpen(true); | ||
| hoverTimer.current = null; | ||
| }, HOVER_DELAY_MS); | ||
| } | ||
| }, | ||
| [copied], | ||
| ); |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handleMouseOver event handler doesn't handle the case where the user moves from one copyable element directly to another without triggering handleMouseOut first. This can result in the tooltip staying anchored to the first element even when hovering over a different element. The logic should clear the previous timer and reset the anchor when detecting a different copyable element.
| <Tooltip | ||
| open={tooltipOpen} | ||
| title={tooltipText} | ||
| placement="top" | ||
| PopperProps={{anchorEl: anchor}} | ||
| componentsProps={{ | ||
| tooltip: {sx: {fontSize: "0.75rem"}}, | ||
| }} | ||
| > | ||
| <Box sx={{display: "contents"}} /> | ||
| </Tooltip> |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Tooltip is configured with a custom anchorEl via PopperProps, but wraps a Box element. This pattern may not work as expected because Material-UI Tooltip expects to anchor to its child element, not an external element. When anchorEl is provided, the Tooltip should be used without wrapping a child, or the implementation should be restructured to use Popper directly for more control over positioning.
| "& .object-key, & .array-key": clickableHoverStyle, | ||
| "& .string-value": clickableHoverStyle, | ||
| "& .variable-value > span:first-of-type": clickableHoverStyle, |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The copyable elements lack ARIA labels or roles to indicate to screen reader users that they are interactive and can be copied. Consider adding appropriate ARIA attributes such as role="button" and aria-label="Copy [key/value]" to copyable elements to improve screen reader support.
| const handleMouseOver = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| const copyable = findCopyableElement(e.target as HTMLElement); | ||
| if (!copyable || copied) return; | ||
|
|
||
| setAnchor(copyable.element); | ||
| if (!hoverTimer.current) { | ||
| hoverTimer.current = window.setTimeout(() => { | ||
| setOpen(true); | ||
| hoverTimer.current = null; | ||
| }, HOVER_DELAY_MS); | ||
| } | ||
| }, | ||
| [copied], | ||
| ); |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handleMouseOver event fires on every mouse move within the container due to event bubbling, causing findCopyableElement to be called repeatedly. This can impact performance, especially with large JSON structures. Consider debouncing the mouseover handler or using a more targeted approach such as adding event listeners only to copyable elements.
| const clickableHoverStyle = { | ||
| cursor: "pointer", | ||
| borderRadius: "2px", | ||
| transition: "background-color 0.15s ease", | ||
| "&:hover": { | ||
| backgroundColor: alpha(theme.palette.primary.main, 0.15), | ||
| }, | ||
| }; |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clickableHoverStyle object is recreated on every render, which can cause unnecessary re-renders of child components and style recalculations. Consider moving this object outside the component or memoizing it with useMemo to avoid recreation on every render.
| setAnchor(null); | ||
| }, COPIED_DISPLAY_MS); | ||
| } catch (err) { | ||
| console.error("Failed to copy:", err); |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling for clipboard write failures only logs to console without providing any user feedback. When clipboard access fails (e.g., in insecure contexts or when permissions are denied), users receive no indication that the copy operation failed. Consider showing an error tooltip or alternative feedback mechanism to inform users of the failure.
| console.error("Failed to copy:", err); | |
| console.error("Failed to copy:", err); | |
| window.alert( | |
| "Failed to copy to clipboard. Please copy the value manually or check your browser permissions.", | |
| ); |
| return null; | ||
| } | ||
|
|
||
| /** Hook for click-to-copy tooltip state management */ |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useCopyTooltip hook lacks JSDoc documentation explaining its purpose, return values, and behavior. Given the complexity of this hook with multiple state values and timing logic, comprehensive documentation would help future maintainers understand how it manages tooltip state, timer cleanup, and the interaction between hover and copied states.
| /** Hook for click-to-copy tooltip state management */ | |
| /** | |
| * React hook that manages the tooltip state for click-to-copy behavior in the JSON view. | |
| * | |
| * This hook coordinates: | |
| * - Tooltip visibility (`open`) and its anchor element (`anchor`). | |
| * - Whether the most recent interaction successfully copied text (`copied`). | |
| * - Timers used to delay showing the tooltip on hover and to automatically hide it | |
| * after a successful copy, ensuring previous timers are cleared to avoid leaks | |
| * and conflicting state updates. | |
| * | |
| * The hook is designed to be used with elements rendered by `ReactJson`. It | |
| * identifies copyable keys/values, writes the selected text to the clipboard on | |
| * click, and then briefly shows a "copied" tooltip near the clicked element. | |
| * Hover-related timers are cleared when a click-to-copy occurs so that the copied | |
| * tooltip takes precedence over hover behavior. | |
| * | |
| * @returns An object containing: | |
| * - `open`: whether the tooltip is currently visible. | |
| * - `copied`: whether the tooltip is currently showing a "copied" state. | |
| * - `anchor`: the HTMLElement the tooltip should be anchored to, or `null`. | |
| * - `handleClick`: mouse event handler that performs the copy action and updates | |
| * tooltip state when a copyable key or value is clicked. | |
| * - Any additional handlers/state needed by the caller to integrate hover and | |
| * click behavior with the tooltip. | |
| */ |
| function useCopyTooltip() { | ||
| const [open, setOpen] = useState(false); | ||
| const [copied, setCopied] = useState(false); | ||
| const [anchor, setAnchor] = useState<HTMLElement | null>(null); | ||
| const hoverTimer = useRef<number | null>(null); | ||
| const copiedTimer = useRef<number | null>(null); | ||
|
|
||
| const clearHoverTimer = () => { | ||
| if (hoverTimer.current) { | ||
| window.clearTimeout(hoverTimer.current); | ||
| hoverTimer.current = null; | ||
| } | ||
| }; | ||
|
|
||
| const handleClick = useCallback(async (e: React.MouseEvent) => { | ||
| const copyable = findCopyableElement(e.target as HTMLElement); | ||
| if (!copyable) return; | ||
|
|
||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| clearHoverTimer(); | ||
|
|
||
| try { | ||
| await navigator.clipboard.writeText(copyable.text); | ||
| setAnchor(copyable.element); | ||
| setCopied(true); | ||
| setOpen(true); | ||
|
|
||
| if (copiedTimer.current) window.clearTimeout(copiedTimer.current); | ||
| copiedTimer.current = window.setTimeout(() => { | ||
| setOpen(false); | ||
| setCopied(false); | ||
| setAnchor(null); | ||
| }, COPIED_DISPLAY_MS); | ||
| } catch (err) { | ||
| console.error("Failed to copy:", err); | ||
| } | ||
| }, []); | ||
|
|
||
| const handleMouseOver = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| const copyable = findCopyableElement(e.target as HTMLElement); | ||
| if (!copyable || copied) return; | ||
|
|
||
| setAnchor(copyable.element); | ||
| if (!hoverTimer.current) { | ||
| hoverTimer.current = window.setTimeout(() => { | ||
| setOpen(true); | ||
| hoverTimer.current = null; | ||
| }, HOVER_DELAY_MS); | ||
| } | ||
| }, | ||
| [copied], | ||
| ); | ||
|
|
||
| const handleMouseOut = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| if (!findCopyableElement(e.target as HTMLElement)) return; | ||
| clearHoverTimer(); | ||
| if (!copied) { | ||
| setOpen(false); | ||
| setAnchor(null); | ||
| } | ||
| }, | ||
| [copied], | ||
| ); | ||
|
|
||
| const handleMouseLeave = useCallback(() => { | ||
| clearHoverTimer(); | ||
| if (!copied) { | ||
| setOpen(false); | ||
| setAnchor(null); | ||
| } | ||
| }, [copied]); | ||
|
|
||
| return { | ||
| tooltipOpen: open && anchor !== null, | ||
| tooltipText: copied ? "Copied!" : "Click to copy", | ||
| anchor, | ||
| handleClick, | ||
| handleMouseOver, | ||
| handleMouseOut, | ||
| handleMouseLeave, | ||
| }; | ||
| } |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timer cleanup is incomplete - there's no cleanup effect for the timers when the component unmounts. If the component unmounts while a timer is active, it will still fire and attempt to call state setters on an unmounted component. Add a useEffect cleanup function to clear both hoverTimer.current and copiedTimer.current on unmount.
| // String value | ||
| if (target.classList.contains("string-value")) { | ||
| return { | ||
| text: (target.textContent || "").replace(/^"|"$/g, ""), |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern /^"|"$/g uses the global flag g which is unnecessary for a single replacement operation on start and end quotes. The global flag can cause unexpected behavior with the replace method. Use /^"|"$/ instead (without the g flag).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onClick={handleClick} | ||
| onMouseOver={handleMouseOver} | ||
| onMouseOut={handleMouseOut} | ||
| onMouseLeave={handleMouseLeave} |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The click-to-copy functionality is not keyboard accessible. Users navigating with keyboard cannot trigger the copy action since it only responds to mouse clicks. Consider adding keyboard event handlers (e.g., onKeyDown) to allow copying via Enter or Space key when elements are focused.
| onMouseLeave={handleMouseLeave} | |
| onMouseLeave={handleMouseLeave} | |
| tabIndex={0} | |
| role="button" | |
| onKeyDown={(event) => { | |
| if (event.key === "Enter" || event.key === " ") { | |
| // Mirror click-to-copy behavior for keyboard users | |
| handleClick(event); | |
| } | |
| }} |
| const clickableHoverStyle = { | ||
| cursor: "pointer", | ||
| borderRadius: "2px", | ||
| transition: "background-color 0.15s ease", | ||
| "&:hover": { | ||
| backgroundColor: alpha(theme.palette.primary.main, 0.15), | ||
| }, | ||
| }; |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clickableHoverStyle object is recreated on every render, causing unnecessary style recalculations and potential performance issues. Consider moving this outside the component or memoizing it with useMemo, especially since it depends on the theme which is stable.
| onMouseOver={handleMouseOver} | ||
| onMouseOut={handleMouseOut} |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onMouseOver and onMouseOut events will fire frequently as the mouse moves within the container, potentially causing performance issues with large JSON structures. Consider using onMouseEnter and onMouseLeave instead, or add throttling to reduce the frequency of event handler calls.
| onMouseOver={handleMouseOver} | |
| onMouseOut={handleMouseOut} | |
| onMouseEnter={handleMouseOver} |
| * @returns {Object} Tooltip state and event handlers | ||
| * @returns {boolean} tooltipOpen - Whether tooltip should be visible | ||
| * @returns {string} tooltipText - "Click to copy" | "Copied!" | "Failed to copy" | ||
| * @returns {boolean} isError - Whether copy failed (for error styling) | ||
| * @returns {HTMLElement|null} anchor - Element to anchor tooltip to | ||
| * @returns {Function} handleClick - Click handler for copying | ||
| * @returns {Function} handleMouseOver - Mouse enter handler for hover tooltip | ||
| * @returns {Function} handleMouseOut - Mouse leave handler for copyable elements | ||
| * @returns {Function} handleMouseLeave - Mouse leave handler for container |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSDoc return type documentation is incomplete. The return type lists individual properties but doesn't specify that it returns an object containing these properties. Consider adding @returns {{tooltipOpen: boolean, tooltipText: string, isError: boolean, anchor: HTMLElement|null, handleClick: Function, handleMouseOver: Function, handleMouseOut: Function, handleMouseLeave: Function}} as a single return statement instead of multiple @returns tags.
| * @returns {Object} Tooltip state and event handlers | |
| * @returns {boolean} tooltipOpen - Whether tooltip should be visible | |
| * @returns {string} tooltipText - "Click to copy" | "Copied!" | "Failed to copy" | |
| * @returns {boolean} isError - Whether copy failed (for error styling) | |
| * @returns {HTMLElement|null} anchor - Element to anchor tooltip to | |
| * @returns {Function} handleClick - Click handler for copying | |
| * @returns {Function} handleMouseOver - Mouse enter handler for hover tooltip | |
| * @returns {Function} handleMouseOut - Mouse leave handler for copyable elements | |
| * @returns {Function} handleMouseLeave - Mouse leave handler for container | |
| * @returns {{ | |
| * tooltipOpen: boolean, | |
| * tooltipText: string, | |
| * isError: boolean, | |
| * anchor: HTMLElement|null, | |
| * handleClick: Function, | |
| * handleMouseOver: Function, | |
| * handleMouseOut: Function, | |
| * handleMouseLeave: Function | |
| * }} Tooltip state and event handlers. |
| * @returns {boolean} isError - Whether copy failed (for error styling) | ||
| * @returns {HTMLElement|null} anchor - Element to anchor tooltip to | ||
| * @returns {Function} handleClick - Click handler for copying | ||
| * @returns {Function} handleMouseOver - Mouse enter handler for hover tooltip |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment mentions "Mouse enter handler" but the actual event being handled is onMouseOver, not onMouseEnter. These are different events - onMouseOver fires for every child element while onMouseEnter only fires when entering the element itself. The comment should accurately describe the event being handled.
| * @returns {Function} handleMouseOver - Mouse enter handler for hover tooltip | |
| * @returns {Function} handleMouseOver - Mouse over handler for hover tooltip |
| open={tooltipOpen} | ||
| anchorEl={anchor} | ||
| placement="top" | ||
| sx={{zIndex: 1500}} |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 1500 for z-index should be replaced with a named constant or referenced from a theme z-index configuration. This helps maintain consistent layering across the application and makes it easier to adjust if needed.
| sx={{zIndex: 1500}} | |
| sx={{zIndex: theme.zIndex.tooltip}} |
| borderRadius: "2px", | ||
| transition: "background-color 0.15s ease", | ||
| "&:hover": { | ||
| backgroundColor: alpha(theme.palette.primary.main, 0.15), |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 0.15 for alpha transparency in the hover background color should be extracted to a named constant for better maintainability and consistency with the rest of the codebase.
| <Paper | ||
| sx={{ | ||
| px: 1, | ||
| py: 0.5, | ||
| fontSize: "0.75rem", | ||
| backgroundColor: isError | ||
| ? theme.palette.error.main | ||
| : theme.palette.grey[800], | ||
| color: theme.palette.common.white, | ||
| }} | ||
| > | ||
| {tooltipText} | ||
| </Paper> |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Popper tooltip does not have proper ARIA attributes for accessibility. Consider adding role="tooltip" and aria-live="polite" to the Paper component so screen readers can announce when the tooltip content changes (e.g., from "Click to copy" to "Copied!").
| // String value | ||
| if (target.classList.contains("string-value")) { | ||
| return { | ||
| text: (target.textContent || "").replace(/^"|"$/g, ""), |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex /^"|"$/g removes only double quotes from the start and end of string values, but does not handle escaped quotes within the string. If a JSON string value contains escaped quotes (e.g., "He said \"hello\""), the copied text may still include the outer quotes or be incorrectly processed. Consider using a more robust approach to extract the actual string value.
| backgroundColor: isError | ||
| ? theme.palette.error.main | ||
| : theme.palette.grey[800], | ||
| color: theme.palette.common.white, |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tooltip uses a hardcoded dark background color (theme.palette.grey[800]) that may not work well in light mode. The white text on a dark grey background works for dark mode, but the component should use theme-aware colors that adapt based on theme.palette.mode to ensure proper contrast and visibility in both light and dark themes, similar to how other parts of the component use semantic colors.
Summary
Enhances the JSON viewer component with click-to-copy functionality for individual keys and values, improving user experience when inspecting JSON data.
Changes
Technical Details
useCopyTooltiphook for tooltip state management