From 23bd7b1211a80aa3b0dcb60ec4a1c0089ff28dd4 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 16 Nov 2021 09:16:05 +0100 Subject: [PATCH] Add copy button to markdown code blocks (#17638) * Add copy button to markdown code blocks Done mostly in JS because I think it's better not to try getting buttons past the markup sanitizer. * add svg module tests * fix sanitizer regexp * remove outdated comment * vertically center button in issue comments as well * add comment to css * fix undefined on view file line copy * combine animation less files * Update modules/markup/markdown/markdown.go Co-authored-by: wxiaoguang * add test for different sizes * add cloneNode and add tests for it * use deep clone * remove useless optional chaining * remove the svg node cache * unify clipboard copy string and i18n * remove unused var * remove unused localization * minor css tweaks to the button * comment tweak * remove useless attribute Co-authored-by: wxiaoguang --- jest.config.js | 4 ++- modules/markup/markdown/markdown.go | 17 +++-------- modules/markup/sanitizer.go | 5 ++- options/locale/locale_en-US.ini | 13 ++++---- package-lock.json | 13 ++++++++ package.json | 1 + templates/base/head.tmpl | 4 +++ templates/repo/clone_buttons.tmpl | 2 +- templates/repo/issue/view_title.tmpl | 2 +- web_src/js/features/clipboard.js | 34 +++++++++++---------- web_src/js/features/common-global.js | 2 +- web_src/js/markup/codecopy.js | 16 ++++++++++ web_src/js/markup/content.js | 4 ++- web_src/js/markup/mermaid.js | 5 +-- web_src/js/svg.js | 2 ++ web_src/js/svg.test.js | 7 +++++ web_src/less/{features => }/animations.less | 18 +++++++++++ web_src/less/index.less | 3 +- web_src/less/markup/codecopy.less | 32 +++++++++++++++++++ 19 files changed, 140 insertions(+), 44 deletions(-) create mode 100644 web_src/js/markup/codecopy.js create mode 100644 web_src/js/svg.test.js rename web_src/less/{features => }/animations.less (82%) create mode 100644 web_src/less/markup/codecopy.less diff --git a/jest.config.js b/jest.config.js index c94113d6f4..690f58d177 100644 --- a/jest.config.js +++ b/jest.config.js @@ -4,7 +4,9 @@ export default { testEnvironment: 'jsdom', testMatch: ['/**/*.test.js'], testTimeout: 20000, - transform: {}, + transform: { + '\\.svg$': 'jest-raw-loader', + }, verbose: false, }; diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index 554ee0d4be..2574585573 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -107,25 +107,18 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) languageStr := string(language) - preClasses := []string{} + preClasses := []string{"code-block"} if languageStr == "mermaid" { preClasses = append(preClasses, "is-loading") } - if len(preClasses) > 0 { - _, err := w.WriteString(`
`)
-								if err != nil {
-									return
-								}
-							} else {
-								_, err := w.WriteString(`
`)
-								if err != nil {
-									return
-								}
+							_, err := w.WriteString(`
`)
+							if err != nil {
+								return
 							}
 
 							// include language-x class as part of commonmark spec
-							_, err := w.WriteString(``)
+							_, err = w.WriteString(``)
 							if err != nil {
 								return
 							}
diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go
index c8f9de33b5..5ff26a3109 100644
--- a/modules/markup/sanitizer.go
+++ b/modules/markup/sanitizer.go
@@ -52,8 +52,11 @@ func InitializeSanitizer() {
 
 func createDefaultPolicy() *bluemonday.Policy {
 	policy := bluemonday.UGCPolicy()
+
+	// For JS code copy and Mermaid loading state
+	policy.AllowAttrs("class").Matching(regexp.MustCompile(`^code-block( is-loading)?$`)).OnElements("pre")
+
 	// For Chroma markdown plugin
-	policy.AllowAttrs("class").Matching(regexp.MustCompile(`^is-loading$`)).OnElements("pre")
 	policy.AllowAttrs("class").Matching(regexp.MustCompile(`^(chroma )?language-[\w-]+$`)).OnElements("code")
 
 	// Checkboxes
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 78efb3d3ff..af68876408 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -85,6 +85,12 @@ remove = Remove
 remove_all = Remove All
 edit = Edit
 
+copy = Copy
+copy_url = Copy URL
+copy_branch = Copy branch name
+copy_success = Copied!
+copy_error = Copy failed
+
 write = Write
 preview = Preview
 loading = Loading…
@@ -927,13 +933,6 @@ fork_from_self = You cannot fork a repository you own.
 fork_guest_user = Sign in to fork this repository.
 watch_guest_user = Sign in to watch this repository.
 star_guest_user = Sign in to star this repository.
-copy_link = Copy
-copy_link_success = Link has been copied
-copy_link_error = Use ⌘C or Ctrl-C to copy
-copy_branch = Copy
-copy_branch_success = Branch name has been copied
-copy_branch_error = Use ⌘C or Ctrl-C to copy
-copied = Copied OK
 unwatch = Unwatch
 watch = Watch
 unstar = Unstar
diff --git a/package-lock.json b/package-lock.json
index df4c575469..2ebeff0f30 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -51,6 +51,7 @@
         "eslint-plugin-vue": "8.0.3",
         "jest": "27.3.1",
         "jest-extended": "1.1.0",
+        "jest-raw-loader": "1.0.1",
         "postcss-less": "5.0.0",
         "stylelint": "14.0.1",
         "stylelint-config-standard": "23.0.0",
@@ -6221,6 +6222,12 @@
         }
       }
     },
+    "node_modules/jest-raw-loader": {
+      "version": "1.0.1",
+      "resolved": "https://registry.npmjs.org/jest-raw-loader/-/jest-raw-loader-1.0.1.tgz",
+      "integrity": "sha1-zp9W1UZQ8VfEp9FtIkul1hO81iY=",
+      "dev": true
+    },
     "node_modules/jest-regex-util": {
       "version": "27.0.6",
       "resolved": "https://registry.npmjs.org/jest-regex-util/-/jest-regex-util-27.0.6.tgz",
@@ -14693,6 +14700,12 @@
       "dev": true,
       "requires": {}
     },
+    "jest-raw-loader": {
+      "version": "1.0.1",
+      "resolved": "https://registry.npmjs.org/jest-raw-loader/-/jest-raw-loader-1.0.1.tgz",
+      "integrity": "sha1-zp9W1UZQ8VfEp9FtIkul1hO81iY=",
+      "dev": true
+    },
     "jest-regex-util": {
       "version": "27.0.6",
       "resolved": "https://registry.npmjs.org/jest-regex-util/-/jest-regex-util-27.0.6.tgz",
diff --git a/package.json b/package.json
index 71c9ab40fd..3c63141922 100644
--- a/package.json
+++ b/package.json
@@ -51,6 +51,7 @@
     "eslint-plugin-vue": "8.0.3",
     "jest": "27.3.1",
     "jest-extended": "1.1.0",
+    "jest-raw-loader": "1.0.1",
     "postcss-less": "5.0.0",
     "stylelint": "14.0.1",
     "stylelint-config-standard": "23.0.0",
diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl
index 23d1190d94..bf1fcd24bc 100644
--- a/templates/base/head.tmpl
+++ b/templates/base/head.tmpl
@@ -46,6 +46,10 @@
 			]).values()),
 			{{end}}
 			mermaidMaxSourceCharacters: {{MermaidMaxSourceCharacters}},
+			i18n: {
+				copy_success: '{{.i18n.Tr "copy_success"}}',
+				copy_error: '{{.i18n.Tr "copy_error"}}',
+			}
 		};
 	
 	
diff --git a/templates/repo/clone_buttons.tmpl b/templates/repo/clone_buttons.tmpl
index 0a86e586fc..37a88af945 100644
--- a/templates/repo/clone_buttons.tmpl
+++ b/templates/repo/clone_buttons.tmpl
@@ -14,7 +14,7 @@
 	
 {{end}}
 {{if or (not $.DisableHTTP) (and (not $.DisableSSH) (or $.IsSigned $.ExposeAnonSSH))}}
-	
 {{end}}
diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl
index 798ab7638c..a21e58068c 100644
--- a/templates/repo/issue/view_title.tmpl
+++ b/templates/repo/issue/view_title.tmpl
@@ -34,7 +34,7 @@
 		{{if .HeadBranchHTMLURL}}
 			{{$headHref = printf "%s" (.HeadBranchHTMLURL | Escape) $headHref}}
 		{{end}}
-		{{$headHref = printf "%s %s" $headHref (.i18n.Tr "repo.copy_branch") (.i18n.Tr "repo.copy_branch_success") (.i18n.Tr "repo.copy_branch_error") (.HeadTarget | Escape) (svg "octicon-copy" 14)}}
+		{{$headHref = printf "%s %s" $headHref (.i18n.Tr "copy_branch") (.HeadTarget | Escape) (svg "octicon-copy" 14)}}
 		{{$baseHref := .BaseTarget|Escape}}
 		{{if .BaseBranchHTMLURL}}
 			{{$baseHref = printf "%s" (.BaseBranchHTMLURL | Escape) $baseHref}}
diff --git a/web_src/js/features/clipboard.js b/web_src/js/features/clipboard.js
index 89aface93a..b0c4134537 100644
--- a/web_src/js/features/clipboard.js
+++ b/web_src/js/features/clipboard.js
@@ -1,27 +1,25 @@
-// For all DOM elements with [data-clipboard-target] or [data-clipboard-text], this copy-to-clipboard will work for them
+const {copy_success, copy_error} = window.config.i18n;
 
-// TODO: replace these with toast-style notifications
 function onSuccess(btn) {
-  if (!btn.dataset.content) return;
+  btn.setAttribute('data-variation', 'inverted tiny');
   $(btn).popup('destroy');
-  const oldContent = btn.dataset.content;
-  btn.dataset.content = btn.dataset.success;
+  const oldContent = btn.getAttribute('data-content');
+  btn.setAttribute('data-content', copy_success);
   $(btn).popup('show');
-  btn.dataset.content = oldContent;
+  btn.setAttribute('data-content', oldContent || '');
 }
 function onError(btn) {
-  if (!btn.dataset.content) return;
-  const oldContent = btn.dataset.content;
+  btn.setAttribute('data-variation', 'inverted tiny');
+  const oldContent = btn.getAttribute('data-content');
   $(btn).popup('destroy');
-  btn.dataset.content = btn.dataset.error;
+  btn.setAttribute('data-content', copy_error);
   $(btn).popup('show');
-  btn.dataset.content = oldContent;
+  btn.setAttribute('data-content', oldContent || '');
 }
 
-/**
- * Fallback to use if navigator.clipboard doesn't exist.
- * Achieved via creating a temporary textarea element, selecting the text, and using document.execCommand.
- */
+
+// Fallback to use if navigator.clipboard doesn't exist. Achieved via creating
+// a temporary textarea element, selecting the text, and using document.execCommand
 function fallbackCopyToClipboard(text) {
   if (!document.execCommand) return false;
 
@@ -37,7 +35,8 @@ function fallbackCopyToClipboard(text) {
 
   tempTextArea.select();
 
-  // if unsecure (not https), there is no navigator.clipboard, but we can still use document.execCommand to copy to clipboard
+  // if unsecure (not https), there is no navigator.clipboard, but we can still
+  // use document.execCommand to copy to clipboard
   const success = document.execCommand('copy');
 
   document.body.removeChild(tempTextArea);
@@ -45,10 +44,13 @@ function fallbackCopyToClipboard(text) {
   return success;
 }
 
+// For all DOM elements with [data-clipboard-target] or [data-clipboard-text],
+// this copy-to-clipboard will work for them
 export default function initGlobalCopyToClipboardListener() {
   document.addEventListener('click', (e) => {
     let target = e.target;
-    // in case , so we just search up to 3 levels for performance.
+    // in case , so we just search
+    // up to 3 levels for performance
     for (let i = 0; i < 3 && target; i++) {
       let text;
       if (target.dataset.clipboardText) {
diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js
index da3fb9d1e3..ac9d0cc92d 100644
--- a/web_src/js/features/common-global.js
+++ b/web_src/js/features/common-global.js
@@ -104,7 +104,7 @@ export function initGlobalCommon() {
   $('.ui.progress').progress({
     showActivity: false
   });
-  $('.poping.up').popup();
+  $('.poping.up').attr('data-variation', 'inverted tiny').popup();
   $('.top.menu .poping.up').popup({
     onShow() {
       if ($('.top.menu .menu.transition').hasClass('visible')) {
diff --git a/web_src/js/markup/codecopy.js b/web_src/js/markup/codecopy.js
new file mode 100644
index 0000000000..2aa7070c72
--- /dev/null
+++ b/web_src/js/markup/codecopy.js
@@ -0,0 +1,16 @@
+import {svg} from '../svg.js';
+
+export function renderCodeCopy() {
+  const els = document.querySelectorAll('.markup .code-block code');
+  if (!els.length) return;
+
+  const button = document.createElement('button');
+  button.classList.add('code-copy', 'ui', 'button');
+  button.innerHTML = svg('octicon-copy');
+
+  for (const el of els) {
+    const btn = button.cloneNode(true);
+    btn.setAttribute('data-clipboard-text', el.textContent);
+    el.after(btn);
+  }
+}
diff --git a/web_src/js/markup/content.js b/web_src/js/markup/content.js
index 0564199bbf..ef5067fd66 100644
--- a/web_src/js/markup/content.js
+++ b/web_src/js/markup/content.js
@@ -1,9 +1,11 @@
 import {renderMermaid} from './mermaid.js';
+import {renderCodeCopy} from './codecopy.js';
 import {initMarkupTasklist} from './tasklist.js';
 
 // code that runs for all markup content
 export function initMarkupContent() {
-  const _promise = renderMermaid(document.querySelectorAll('code.language-mermaid'));
+  renderMermaid();
+  renderCodeCopy();
 }
 
 // code that only runs for comments
diff --git a/web_src/js/markup/mermaid.js b/web_src/js/markup/mermaid.js
index f9f069ed1e..7c7ee26c3c 100644
--- a/web_src/js/markup/mermaid.js
+++ b/web_src/js/markup/mermaid.js
@@ -8,8 +8,9 @@ function displayError(el, err) {
   el.closest('pre').before(errorNode);
 }
 
-export async function renderMermaid(els) {
-  if (!els || !els.length) return;
+export async function renderMermaid() {
+  const els = document.querySelectorAll('.markup code.language-mermaid');
+  if (!els.length) return;
 
   const {default: mermaid} = await import(/* webpackChunkName: "mermaid" */'mermaid');
 
diff --git a/web_src/js/svg.js b/web_src/js/svg.js
index 11be6b476c..77aa1e7ca7 100644
--- a/web_src/js/svg.js
+++ b/web_src/js/svg.js
@@ -1,5 +1,6 @@
 import octiconChevronDown from '../../public/img/svg/octicon-chevron-down.svg';
 import octiconChevronRight from '../../public/img/svg/octicon-chevron-right.svg';
+import octiconCopy from '../../public/img/svg/octicon-copy.svg';
 import octiconGitMerge from '../../public/img/svg/octicon-git-merge.svg';
 import octiconGitPullRequest from '../../public/img/svg/octicon-git-pull-request.svg';
 import octiconIssueClosed from '../../public/img/svg/octicon-issue-closed.svg';
@@ -20,6 +21,7 @@ import Vue from 'vue';
 export const svgs = {
   'octicon-chevron-down': octiconChevronDown,
   'octicon-chevron-right': octiconChevronRight,
+  'octicon-copy': octiconCopy,
   'octicon-git-merge': octiconGitMerge,
   'octicon-git-pull-request': octiconGitPullRequest,
   'octicon-issue-closed': octiconIssueClosed,
diff --git a/web_src/js/svg.test.js b/web_src/js/svg.test.js
new file mode 100644
index 0000000000..f1939c3a46
--- /dev/null
+++ b/web_src/js/svg.test.js
@@ -0,0 +1,7 @@
+import {svg} from './svg.js';
+
+test('svg', () => {
+  expect(svg('octicon-repo')).toStartWith('