-
Notifications
You must be signed in to change notification settings - Fork 119
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
Implement ASM Diff contents Copy Button #1357
Conversation
Fixes #805 |
navigator.clipboard.writeText(text) | ||
} | ||
|
||
const getContentsFromDiffOutput = (diff: api.DiffOutput, kind: string): string => { |
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.
Is it possible to do this in a more lazy manner - i.e. only when the user clicks on the copy? otherwise we're processing the base/current/previous on every update...
I was also thinking it could be cool to 'flash' the contents pane briefly on copy so the user gets some visual signal (but that could be another PR and certainly shouldnt prevent this one getting merged).
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.
something like...
diff --git a/frontend/src/components/Diff/Diff.tsx b/frontend/src/components/Diff/Diff.tsx
index b6f21cd5..b14fe5d6 100644
--- a/frontend/src/components/Diff/Diff.tsx
+++ b/frontend/src/components/Diff/Diff.tsx
@@ -20,12 +20,8 @@ import * as Objdiff from "./DiffRowObjdiff"
import DragBar from "./DragBar"
import { useHighlighers } from "./Highlighter"
-// Utility function to copy content to clipboard
-const copyToClipboard = (text: string) => {
- navigator.clipboard.writeText(text)
-}
-const getContentsFromDiffOutput = (diff: api.DiffOutput, kind: string): string => {
+const copyDiffContentsToClipboard = (diff: api.DiffOutput, kind: string) => {
// kind is either "base", "current", or "previous"
const contents = diff.rows.map(row => {
let text = ""
@@ -39,15 +35,14 @@ const getContentsFromDiffOutput = (diff: api.DiffOutput, kind: string): string =
return text
})
- return contents.join("\n")
+ navigator.clipboard.writeText(contents.join("\n"))
}
-// Small component for the copy button
-function CopyButton({ content }: { content: string }) {
+function CopyButton({ diff, kind }: { diff: api.DiffOutput, kind: string }) {
return (
<button
className={styles.copyButton} // Add a new style for the button
- onClick={() => copyToClipboard(content)}
+ onClick={() => copyDiffContentsToClipboard(diff, kind)}
title="Copy content"
>
<CopyIcon size={16} />
@@ -211,17 +206,17 @@ export default function Diff({ diff, diffLabel, isCompiling, isCurrentOutdated,
<div className={styles.headers}>
<div className={styles.header}>
Target
- <CopyButton content={getContentsFromDiffOutput(diff as api.DiffOutput, "base")} />
+ <CopyButton diff={diff as api.DiffOutput} kind="base" />
</div>
<div className={styles.header}>
Current
- <CopyButton content={getContentsFromDiffOutput(diff as api.DiffOutput, "current")} />
+ <CopyButton diff={diff as api.DiffOutput} kind="current" />
{isCompiling && <Loading width={20} height={20} />}
{!threeWayDiffEnabled && threeWayButton}
</div>
{threeWayDiffEnabled && <div className={styles.header}>
{threeWayDiffBase === ThreeWayDiffBase.SAVED ? "Saved" : "Previous"}
- <CopyButton content={getContentsFromDiffOutput(diff as api.DiffOutput, "previous")} />
+ <CopyButton diff={diff as api.DiffOutput} kind="previous" />
{threeWayButton}
</div>}
</div>
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.
Added changes!
* Implement diff copy button * Make vercel deployment happy * Make vercel deployment happier * Just remove the alert * Tweak styles * Retrigger deployments * Lazy loaded and refactored CopyButton component logic
* Implement ASM Diff contents Copy Button (#1357) * Implement diff copy button * Make vercel deployment happy * Make vercel deployment happier * Just remove the alert * Tweak styles * Retrigger deployments * Lazy loaded and refactored CopyButton component logic * Allow scratch name to be highlighted and copied for non-owners (#1359) * Add scratchName class to allow highlighting and copying the scratch name * Use .name to allow selecting scratch name * Properly center the copy button icon --------- Co-authored-by: PerikiyoXD <[email protected]>
It's working, but it needs some toast or anything instead of the alert.