Codeblock copy button #3501

sxyxs · 2022-05-24T04:26:32Z

Description

Added a plugin to add a button to copy the contents of a codeblock as #2375.

Changes

Added the plugin source/common/modules/markdown-editor/plugins/render-codeblock-button.ts
Modified relevant files to enable toggling the plugin in display preferences.
New translation string: dialog.preferences.display.render_codeblock_button

Additional Information

error when running yarn lint:
- in file source/common/modules/markdown-editor/plugins/render-codeblock-button.ts
- error The array literal notation [] is preferable
- However, if we define the array with [] on line 29 , I cannot do the clipboard.writeText operation on line 64.

Tested on:
Ubuntu 20.04 LTS

Codeblock copy button sxy

boring-cyborg · 2022-05-24T04:26:34Z

Thank you for opening your first PR! 🎉 We are very happy and would like to thank you very much for your contribution. If everything checks out, we'll make sure to review the PR as soon as possible and give feedback. In the meantime, to make the reviewing process as fast as possible, you can help us by checking the following things:

Did you follow the JSStandard coding style? - Did you comment everywhere where the necessity of a piece of code or the
way it was implemented is not immediately obvious?
Did you attempt to stick as much to current coding habits as possible?
(Note that this does not apply to pieces of code where we ourselves
obviously violated good coding practices, which unfortunately happens
sometimes. But please indicate this in your PR so that we know what you
rectified!)

Furthermore, make sure that the linter does not complain, which will check your code on every new commit. If the linter task fails, make sure to run yarn lint locally and check the file eslint_report.htm which will tell you precisely what went wrong.
Stay sharp, and thanks again!

EdgarTang · 2022-05-24T07:43:21Z

error when running yarn lint:

in file source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

error The array literal notation [] is preferable

However, if we define the array with [] on line 29 , I cannot do the clipboard.writeText operation on line 64.

In typescript you can declare a sting array like:
let codesblocks = new Array<string>()
More examples here.
By the way, you should better remove the test code like console.log(i) in source/common/modules/markdown-editor/plugins/render-codeblock-button.ts line 48

Mic112345 · 2022-05-26T02:31:37Z

error when running yarn lint:

in file source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

error The array literal notation [] is preferable

However, if we define the array with [] on line 29 , I cannot do the clipboard.writeText operation on line 64.

In typescript you can declare a sting array like: let codesblocks = new Array<string>() More examples here. By the way, you should better remove the test code like console.log(i) in source/common/modules/markdown-editor/plugins/render-codeblock-button.ts line 48

Thanks for the feedback, I've addressed those areas in the most recent commit

sxyxs · 2022-05-28T06:43:57Z

Hi @nathanlesage, could you help us to review this pull request?

sxyxs · 2022-05-28T11:26:27Z

Hi @kyaso, could you help us to review this pull request?

kyaso

Overall, this looks promising! I have added some comments. Maybe @nathanlesage can also have a look when he has time again.

One thing I noticed: When you edit an existing codeblock, save the document, and then click the copy button, the newly added lines are not copied; only the previous version of the codeblock is copied. Not sure what might be causing this, but it's something we probably want to address before merging.

source/win-preferences/schema/display.ts

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

kyaso · 2022-05-28T15:54:58Z

source/common/less/theme-berlin/light.less

@@ -210,6 +210,22 @@ body {
        border-bottom-left-radius: 4px;
        border-bottom-right-radius: 4px;
    }
+
+    .code-block-copy-button{


Is it possible to make the button and its placement look similar to this: #2375 (comment)

Currently, the button looks a bit out of place:

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

sxyxs · 2022-05-28T17:06:42Z

Overall, this looks promising! I have added some comments. Maybe @nathanlesage can also have a look when he has time again.

One thing I noticed: When you edit an existing codeblock, save the document, and then click the copy button, the newly added lines are not copied; only the previous version of the codeblock is copied. Not sure what might be causing this, but it's something we probably want to address before merging.

Thank you for the reply. We will fix that.

static/lang/en-GB.json

static/lang/en-US.json

nathanlesage · 2022-05-29T18:11:21Z

Heya, I'm back from a week-long conference, and just had a look. First of all, thank you for your efforts, it looks as if this will be a great addition to the app! Also, thanks to @kyaso for providing a very good and extensive code review. I think his review provides very good guidance in improving your PR. I added a few comments in there where I deemed it fit, but please orient yourself at Kyaso's comments first for a first round of review! :)

sxyxs · 2022-06-04T11:26:30Z

Hi, I update my code based on the feedback from @kyaso. Also, I do not really understand the meaning of "Do not alter the language files." from @nathanlesage feedback. Does it mean I need to remove that line?

nathanlesage · 2022-06-06T13:35:18Z

@sxyxs This meant that you should not modify any of the language files. We have to do it via Zettlr Translate, i.e.: I need you to provide a list of new identifiers that you've introduced in your PR, and then I can add them to Zettlr Translate for everyone to translate.

sxyxs · 2022-06-06T14:36:14Z

@sxyxs This meant that you should not modify any of the language files. We have to do it via Zettlr Translate, i.e.: I need you to provide a list of new identifiers that you've introduced in your PR, and then I can add them to Zettlr Translate for everyone to translate.

I understand that and start dealing with it. Thank you for the reply.

sxyxs · 2022-06-09T11:37:32Z

Hi @nathanlesage, the only thing we have changed is to add a new copy button in each code block introduced in the PR's introduction.

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

sxyxs · 2022-06-20T09:09:37Z

@kyaso Thanks for your feedback and sorry for missing one point. I have fixed that.

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

source/common/less/theme-berlin/light.less

sxyxs · 2022-06-23T14:09:35Z

Hi @kyaso, I have already fixed what you want me to fix. Please check.

kyaso · 2022-06-23T22:04:54Z

@sxyxs It looks fine to me. I think there is one minor improvement left which you could implement: #3501 (comment)

@nathanlesage Would you mind having a final look over it again? Also, currently, the button text is a hardcoded "Copy", but I guess it should be translated? And last point: Do you think we should squash some commits before merging to clean up the commit history a bit?

sxyxs · 2022-06-24T10:40:00Z

@sxyxs It looks fine to me. I think there is one minor improvement left which you could implement: #3501 (comment)

@nathanlesage Would you mind having a final look over it again? Also, currently, the button text is a hardcoded "Copy", but I guess it should be translated? And last point: Do you think we should squash some commits before merging to clean up the commit history a bit?

Hi @kyaso, fixed, please check.

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

kyaso · 2022-07-07T16:49:22Z

@sxyxs Looks fine from my side.

Would you mind rebasing your commits on top of the latest develop branch and also squash the temporary commits to clean up the commit history? And once that's done, @nathanlesage could have another look, when he has time?

sxyxs · 2022-07-10T16:53:12Z

@sxyxs Looks fine from my side.

Would you mind rebasing your commits on top of the latest develop branch and also squash the temporary commits to clean up the commit history? And once that's done, @nathanlesage could have another look, when he has time?

Ok, I will do it these days.

sxyxs · 2022-07-14T14:30:32Z

Hi @kyaso, Is that what you want?

sxyxs · 2022-07-26T15:39:56Z

@kyaso Hello?

nathanlesage · 2022-08-01T09:57:48Z

@sxyxs I think Kyaso is on vacation at the moment. I will have a look at the PR in the upcoming days.

sxyxs · 2022-08-01T11:44:49Z

@sxyxs I think Kyaso is on vacation at the moment. I will have a look at the PR in the upcoming days.

That will be great, thank you!

nathanlesage

I found a few things, styling issues mainly. Please fix them while I pull the thing and test it out locally.

nathanlesage · 2022-08-02T13:26:59Z

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

+  const lineCount = cm.lineCount()
+  let incodeblock = false
+  let codesblocks = new Array<string>()
+  let codeblock = ''


Why don't you store the codeblock as an array? That would (a) make line 39 less awkward since you just had to push the line, would (b) save you from slicing in line 44, and (c) in line 71 you could simply write codeblock.join('\n') into the clipboard.

nathanlesage · 2022-08-02T13:27:34Z

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

+  const codeBlockRE = /^(?:\s{0,3}`{3}|~{3}).*/
+  const lineCount = cm.lineCount()
+  let incodeblock = false
+  let codesblocks = new Array<string>()


Suggested change

let codesblocks = new Array<string>()

const codeblocks: string[] = []

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

nathanlesage · 2022-08-02T13:30:20Z

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

+  if (document.getElementsByClassName(codeblockClassClose).length === codeblocks.length) {
+    for (let i = 0; i < codeblocks.length; i++) {
+      const codeBlock = codeblocks[i]
+      if (codeBlock === undefined) {


This should never be true! Did you add this because you were encountering undefined-issues? If so then there is a serious bug further above in this file. If not, then it should be safe to delete this.

@nathanlesage Yes, there will have some issues if I delete this one. I will take look into it to find a better solution.

Hi @nathanlesage, should I delete line 53 or 56 or both?

nathanlesage · 2022-08-02T13:30:47Z

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

+        codeBlock.childNodes[1].addEventListener('click', () => clipboard.writeText(codeBlockText))
+      } else {
+        // Add property for copy button
+        let copyButton = document.createElement('button')


Suggested change

let copyButton = document.createElement('button')

const copyButton = document.createElement('button')

nathanlesage · 2022-08-02T13:31:20Z

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts

+        let copyButton = document.createElement('button')
+        copyButton.className = 'code-block-copy-button'
+        copyButton.innerHTML = '<clr-icon shape="copy"></clr-icon>'
+        copyButton.setAttribute('title', 'Copy this code block to clipboard')


Please make this translatable and provide the translation keys for me to add to Zettlr translate once it's finished.

nathanlesage · 2022-08-02T13:32:20Z

source/win-preferences/schema/display.ts

@@ -62,6 +62,11 @@ export default function (): any {
          type: 'checkbox',
          label: trans('dialog.preferences.display.render_emphasis'),
          model: 'display.renderEmphasis'
+        },
+        {
+          type: 'checkbox',


I think it is safe to not make this toggleable. The codeblock button does not change the rendering of any part of the editor, hence we should remove this option. Possibly this also involves removing the option from other files your PR touches.

nathanlesage · 2022-08-02T13:32:43Z

source/win-quicklook/QuicklookEditor.vue

@@ -182,7 +182,8 @@ export default defineComponent({
            math: (global as any).config.get('display.renderMath'),
            tasks: (global as any).config.get('display.renderTasks'),
            headingTags: (global as any).config.get('display.renderHTags'),
-            tables: (global as any).config.get('editor.enableTableHelper')
+            tables: (global as any).config.get('editor.enableTableHelper'),
+            codeBlockButton: (global as any).config.get('display.renderCodeBlockButton')


The quicklook editor has been nuked in the meantime, so you can remove this change.

a1747931 and others added 11 commits May 18, 2022 00:06

feat: add preliminary plugin template

809ad7a

Added copy button to a test code block

86d892b

Merge branch 'Zettlr:develop' into codeblock-copy-button-sxy

6ae6db0

stuck in read global codeblock-open, switch labtop

089bb9d

the code button can be present in all the codeblock

a8b2554

the copy button is working for static document

166ce49

Added style for the copy button

4df1aaf

fixed some small bugs and most of the lint issues

c5f6f65

Merge branch 'Zettlr:develop' into codeblock-copy-button

535a75f

Merge branch 'codeblock-copy-button' into codeblock-copy-button-sxy

0099fe9

Merge pull request #1 from Bboona/codeblock-copy-button-sxy

59f9d07

Codeblock copy button sxy

Specified code blocks array as string and removed console.log()

d646f43

sxyxs marked this pull request as ready for review May 27, 2022 05:18

sxyxs added 2 commits May 28, 2022 22:04

fix system crash button when delete code block, also yarn lint pass

0546c19

fix the buuton still occur when deleting a code block

0032b82

kyaso requested changes May 28, 2022

View reviewed changes

nathanlesage reviewed May 29, 2022

View reviewed changes

static/lang/en-GB.json Outdated Show resolved Hide resolved

nathanlesage reviewed May 29, 2022

View reviewed changes

static/lang/en-US.json Outdated Show resolved Hide resolved

fix some issues based on the suggestion from Kyaso

c597d5c

remove the change in language files

5782aea

sxyxs requested a review from kyaso June 14, 2022 11:10

kyaso requested changes Jun 16, 2022

View reviewed changes

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts Show resolved Hide resolved

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts Outdated Show resolved Hide resolved

add comments and optimize the functions

d04d6a0

kyaso requested changes Jun 22, 2022

View reviewed changes

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts Show resolved Hide resolved

add one more comment

11a087f

kyaso requested changes Jun 23, 2022

View reviewed changes

fix some style stuff

e23dd29

keep fix small issues

94d434f

kyaso requested changes Jun 24, 2022

View reviewed changes

source/common/modules/markdown-editor/plugins/render-codeblock-button.ts Show resolved Hide resolved

add aria label and title

c94f97a

sxyxs requested a review from kyaso July 4, 2022 11:03

Merge branch 'Zettlr:develop' into codeblock-copy-button

b7c52e0

clean some un-edit code

7a68a28

nathanlesage requested changes Aug 2, 2022

View reviewed changes

Eternality5 mentioned this pull request May 7, 2023

[Enhancement] [2.0] Code block copy button #2375

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Codeblock copy button #3501

Codeblock copy button #3501

sxyxs commented May 24, 2022

boring-cyborg bot commented May 24, 2022

EdgarTang commented May 24, 2022 •

edited

Mic112345 commented May 26, 2022

sxyxs commented May 28, 2022

sxyxs commented May 28, 2022

kyaso left a comment

kyaso May 28, 2022

sxyxs commented May 28, 2022

nathanlesage commented May 29, 2022

sxyxs commented Jun 4, 2022

nathanlesage commented Jun 6, 2022

sxyxs commented Jun 6, 2022

sxyxs commented Jun 9, 2022

sxyxs commented Jun 20, 2022

sxyxs commented Jun 23, 2022

kyaso commented Jun 23, 2022

sxyxs commented Jun 24, 2022 •

edited

kyaso commented Jul 7, 2022

sxyxs commented Jul 10, 2022

sxyxs commented Jul 14, 2022

sxyxs commented Jul 26, 2022

nathanlesage commented Aug 1, 2022

sxyxs commented Aug 1, 2022

nathanlesage left a comment

nathanlesage Aug 2, 2022

nathanlesage Aug 2, 2022

nathanlesage Aug 2, 2022

sxyxs Aug 19, 2022

sxyxs Aug 21, 2022

nathanlesage Aug 2, 2022

nathanlesage Aug 2, 2022

nathanlesage Aug 2, 2022

nathanlesage Aug 2, 2022

	let codesblocks = new Array<string>()
	const codeblocks: string[] = []

	let copyButton = document.createElement('button')
	const copyButton = document.createElement('button')

Codeblock copy button #3501

Are you sure you want to change the base?

Codeblock copy button #3501

Conversation

sxyxs commented May 24, 2022

Description

Changes

Additional Information

boring-cyborg bot commented May 24, 2022

EdgarTang commented May 24, 2022 • edited

Mic112345 commented May 26, 2022

sxyxs commented May 28, 2022

sxyxs commented May 28, 2022

kyaso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sxyxs commented May 28, 2022

nathanlesage commented May 29, 2022

sxyxs commented Jun 4, 2022

nathanlesage commented Jun 6, 2022

sxyxs commented Jun 6, 2022

sxyxs commented Jun 9, 2022

sxyxs commented Jun 20, 2022

sxyxs commented Jun 23, 2022

kyaso commented Jun 23, 2022

sxyxs commented Jun 24, 2022 • edited

kyaso commented Jul 7, 2022

sxyxs commented Jul 10, 2022

sxyxs commented Jul 14, 2022

sxyxs commented Jul 26, 2022

nathanlesage commented Aug 1, 2022

sxyxs commented Aug 1, 2022

nathanlesage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdgarTang commented May 24, 2022 •

edited

sxyxs commented Jun 24, 2022 •

edited