Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feat/4750 Make code type fields copyable #5395

Merged
merged 6 commits into from
Feb 21, 2024
Merged

Feat/4750 Make code type fields copyable #5395

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd382e0
changed from link-specific message to general copy-paste message
elzody Feb 19, 2024
ca0d4b0
added code block copy button
elzody Feb 19, 2024
81960db
fixed cypress test and added test for copy code button
elzody Feb 20, 2024
0a70ed5
added optional chaining to hasCode property for consistency
elzody Feb 20, 2024
8b3360d
improved cypress element selectors according to best practices
elzody Feb 21, 2024
bb24bca
fix linting errors
elzody Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/mixins/CopyToClipboardMixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@ export default {
},

methods: {
async copyToClipboard(url) {
async copyToClipboard(content) {
// change to loading status
this.copyLoading = true

// copy link to clipboard
try {
await navigator.clipboard.writeText(url)
await navigator.clipboard.writeText(content)
this.copySuccess = true
this.copied = true

// Notify success
showSuccess(t('collectives', 'Link copied'))
showSuccess(t('text', 'Copied to the clipboard'))
} catch (error) {
this.copySuccess = false
this.copied = true
showError(
`<div>${t('collectives', 'Could not copy link to the clipboard:')}</div><div>${url}</div>`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly the url was in here on purpose. The idea was that if your browser cannot copy the url for you its still displaying it so you can copy it yourself.

I'm not sure how often people would need to do that. I remember that back in the days I could not copy share links because of my browser configuration and did not find a way around that.

However in this case I think it's different. I could open the link in a tab and copy the link from there - or I right click the link and select 'copy link' from the context menu.

So I'm fine with removing the url - just wanted to explain the context.

`<div>${t('text', 'Could not copy to the clipboard')}</div>`,
{ isHTML: true })
} finally {
this.copyLoading = false
Expand Down
34 changes: 31 additions & 3 deletions src/nodes/CodeBlockView.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,21 @@
<template>
<NodeViewWrapper as="div" :data-mode="viewMode" class="code-block">
<div v-if="isEditable" class="code-block-header">
<div class="code-block-header">
<div class="view-switch">
<NcActions :aria-label="t('text', 'Code block options')">
<NcActions :aria-label="t('text', 'Copy code block')">
<NcActionButton v-if="hasCode"
:aria-label="t('text', 'Copy code')"
:close-after-click="false"
@click="copyCode">
<template #icon>
<Check v-if="copySuccess" :size="20" />
<NcLoadingIcon v-else-if="copyLoading" :size="20" />
<ContentCopy v-else :size="20" />
</template>
</NcActionButton>
</NcActions>

<NcActions v-if="isEditable" :aria-label="t('text', 'Code block options')">
<NcActionInput :label="t('text', 'Code block language')"
:value="type"
:show-trailing-button="false"
Expand Down Expand Up @@ -59,7 +72,7 @@
<script>
import debounce from 'debounce'
import { NodeViewWrapper, NodeViewContent } from '@tiptap/vue-2'
import { NcActions, NcActionButton, NcActionInput, NcActionLink, NcActionSeparator } from '@nextcloud/vue'
import { NcActions, NcActionButton, NcActionInput, NcActionLink, NcActionSeparator, NcLoadingIcon } from '@nextcloud/vue'
import mermaid from 'mermaid'
import { v4 as uuidv4 } from 'uuid'

Expand All @@ -68,13 +81,19 @@ import CodeBraces from 'vue-material-design-icons/CodeBraces.vue'
import Eye from 'vue-material-design-icons/Eye.vue'
import MarkerIcon from 'vue-material-design-icons/Marker.vue'
import Help from 'vue-material-design-icons/Help.vue'
import ContentCopy from 'vue-material-design-icons/ContentCopy.vue'
import Check from 'vue-material-design-icons/Check.vue'

import CopyToClipboardMixin from '../mixins/CopyToClipboardMixin.js'

export default {
// eslint-disable-next-line vue/match-component-file-name
name: 'CodeBlockView',
components: {
MarkerIcon,
ContentCopy,
Help,
Check,
Eye,
ViewSplitVertical,
CodeBraces,
Expand All @@ -83,9 +102,11 @@ export default {
NcActionInput,
NcActionLink,
NcActionSeparator,
NcLoadingIcon,
NodeViewWrapper,
NodeViewContent,
},
mixins: [CopyToClipboardMixin],
props: {
node: {
type: Object,
Expand All @@ -104,6 +125,9 @@ export default {
}
},
computed: {
hasCode() {
return this.node.textContent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is safeguarded further down, do we need this here as well?

Suggested change
return this.node.textContent
return this.node?.textContent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought process was that, since it's just checking to see if there is any text content in the node (the code block has something written in it), it wouldn't matter if we used the optional chaining ?. or not, but I think for consistency you're right, it should be added there as well. As for the cypress tests, I was just about to look into those and give it a fix. Thank you :)

},
type() {
return this.node?.attrs?.language || ''
},
Expand Down Expand Up @@ -169,6 +193,9 @@ export default {
})
},
methods: {
async copyCode() {
await this.copyToClipboard(this.node?.textContent)
},
updateLanguage(event) {
this.updateAttributes({
language: event.target.value,
Expand All @@ -180,6 +207,7 @@ export default {
},
}
</script>

<style lang="scss" scoped>
.code-block {
background-color: var(--color-background-dark);
Expand Down
Loading