-
Notifications
You must be signed in to change notification settings - Fork 578
[MISC] Add copy-to-clipboard button for file execution ID in LogModal #1758
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
Adds a copy icon button next to the file execution ID in the Execution Log Details modal, allowing users to quickly copy the ID to clipboard similar to the main execution ID functionality. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a reusable Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Modal as LogModal / DetailedLogs (UI)
participant Hook as useCopyToClipboard
participant Clipboard as Browser Clipboard (navigator.clipboard)
participant Alert as Alert Store
User->>Modal: Click copy button
Modal->>Hook: copyToClipboard(displayId, "Label")
Hook->>Clipboard: writeText(displayId)
alt writeText succeeds
Clipboard-->>Hook: Promise resolved
Hook->>Alert: success("Label copied to clipboard")
Alert-->>Modal: show success
else writeText fails
Clipboard-->>Hook: Promise rejected
Hook->>Alert: error("Failed to copy: <err>")
Alert-->>Modal: show error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/logging/log-modal/LogModal.jsx`:
- Around line 180-193: The title and copy button in LogModal.jsx can produce a
trailing dash and copy an empty/undefined id; update the rendering for displayId
(derived from executionId/fileId) so the title omits " - {displayId}" when
displayId is falsy, and guard the copy action by either hiding/disabling the
Copy Button when displayId is missing or by preventing handleCopyToClipboard
from being called with an empty value; also add an accessible name to the
icon-only Button (e.g., aria-label or title) so the CopyOutlined icon is
announced, and reference the existing symbols displayId, fileId, executionId,
handleCopyToClipboard, and the Button/CopyOutlined in LogModal.jsx when making
the changes.
🧹 Nitpick comments (1)
frontend/src/components/logging/log-modal/LogModal.jsx (1)
35-57: Consider extracting the clipboard helper to a shared utility.
There’s a near-identical helper infrontend/src/components/logging/detailed-logs/DetailedLogs.jsx(lines 124-146). A shared hook/utility would avoid drift in error handling and alert wording.
- Only render copy button when displayId is truthy - Add aria-label for accessibility - Remove dash separator from title Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: Chandrasekharan M <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/logging/log-modal/LogModal.jsx`:
- Around line 33-35: The UI always labels the shown ID as "File Execution ID"
even when displayId comes from executionId; update the logic around displayId
(where fileId || executionId is set) to also compute a corresponding idLabel
(e.g., use "File ID" when fileId is present, otherwise "Execution ID") and
replace the hard-coded title strings in the LogModal component (references:
displayId, fileId, executionId) so both the top title and the secondary title
(the other occurrence around the later block) render the correct label based on
which ID is used.
- Create reusable useCopyToClipboard hook to avoid code duplication - Update LogModal.jsx to use the shared hook - Update DetailedLogs.jsx to use the shared hook Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
athul-rs
left a comment
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.
lgtm



What
Why
How
Can this PR break any existing features. If yes, please list possible items. If no, please explain why.
Notes on Testing
Screenshots
Checklist