Skip to content

Commit 9bc5552

Browse files
tyroneyehsilverwindwxiaoguang
authored
Improve attachment upload methods (#30513)
* Use dropzone to handle file uploading for all cases, including pasting and dragging * Merge duplicate code, use consistent behavior for link generating Close #20130 --------- Co-authored-by: silverwind <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent 00fc29a commit 9bc5552

File tree

10 files changed

+171
-99
lines changed

10 files changed

+171
-99
lines changed

‎web_src/js/features/comp/ComboMarkdownEditor.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@ import '@github/text-expander-element';
33
import $ from 'jquery';
44
import {attachTribute} from '../tribute.js';
55
import {hideElem, showElem, autosize, isElemVisible} from '../../utils/dom.js';
6-
import {initEasyMDEPaste, initTextareaPaste} from './Paste.js';
6+
import {initEasyMDEPaste, initTextareaUpload} from './EditorUpload.js';
77
import {handleGlobalEnterQuickSubmit} from './QuickSubmit.js';
88
import {renderPreviewPanelContent} from '../repo-editor.js';
99
import {easyMDEToolbarActions} from './EasyMDEToolbarActions.js';
1010
import {initTextExpander} from './TextExpander.js';
1111
import {showErrorToast} from '../../modules/toast.js';
1212
import {POST} from '../../modules/fetch.js';
1313
import {initTextareaMarkdown} from './EditorMarkdown.js';
14-
import {initDropzone} from '../dropzone.js';
14+
import {DropzoneCustomEventReloadFiles, initDropzone} from '../dropzone.js';
1515

1616
let elementIdCounter = 0;
1717

@@ -111,7 +111,7 @@ class ComboMarkdownEditor {
111111

112112
initTextareaMarkdown(this.textarea);
113113
if (this.dropzone) {
114-
initTextareaPaste(this.textarea, this.dropzone);
114+
initTextareaUpload(this.textarea, this.dropzone);
115115
}
116116
}
117117

@@ -130,13 +130,13 @@ class ComboMarkdownEditor {
130130

131131
dropzoneReloadFiles() {
132132
if (!this.dropzone) return;
133-
this.attachedDropzoneInst.emit('reload');
133+
this.attachedDropzoneInst.emit(DropzoneCustomEventReloadFiles);
134134
}
135135

136136
dropzoneSubmitReload() {
137137
if (!this.dropzone) return;
138138
this.attachedDropzoneInst.emit('submit');
139-
this.attachedDropzoneInst.emit('reload');
139+
this.attachedDropzoneInst.emit(DropzoneCustomEventReloadFiles);
140140
}
141141

142142
setupTab() {

‎web_src/js/features/comp/EditorMarkdown.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import {triggerEditorContentChanged} from './Paste.js';
1+
export function triggerEditorContentChanged(target) {
2+
target.dispatchEvent(new CustomEvent('ce-editor-content-changed', {bubbles: true}));
3+
}
24

35
function handleIndentSelection(textarea, e) {
46
const selStart = textarea.selectionStart;

‎web_src/js/features/comp/Paste.js‎ renamed to ‎web_src/js/features/comp/EditorUpload.js‎

Lines changed: 72 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,29 @@
1-
import {htmlEscape} from 'escape-goat';
2-
import {POST} from '../../modules/fetch.js';
31
import {imageInfo} from '../../utils/image.js';
4-
import {getPastedContent, replaceTextareaSelection} from '../../utils/dom.js';
2+
import {replaceTextareaSelection} from '../../utils/dom.js';
53
import {isUrl} from '../../utils/url.js';
6-
7-
async function uploadFile(file, uploadUrl) {
8-
const formData = new FormData();
9-
formData.append('file', file, file.name);
10-
11-
const res = await POST(uploadUrl, {data: formData});
12-
return await res.json();
13-
}
14-
15-
export function triggerEditorContentChanged(target) {
16-
target.dispatchEvent(new CustomEvent('ce-editor-content-changed', {bubbles: true}));
4+
import {triggerEditorContentChanged} from './EditorMarkdown.js';
5+
import {
6+
DropzoneCustomEventRemovedFile,
7+
DropzoneCustomEventUploadDone,
8+
generateMarkdownLinkForAttachment,
9+
} from '../dropzone.js';
10+
11+
let uploadIdCounter = 0;
12+
13+
function uploadFile(dropzoneEl, file) {
14+
return new Promise((resolve) => {
15+
const curUploadId = uploadIdCounter++;
16+
file._giteaUploadId = curUploadId;
17+
const dropzoneInst = dropzoneEl.dropzone;
18+
const onUploadDone = ({file}) => {
19+
if (file._giteaUploadId === curUploadId) {
20+
dropzoneInst.off(DropzoneCustomEventUploadDone, onUploadDone);
21+
resolve();
22+
}
23+
};
24+
dropzoneInst.on(DropzoneCustomEventUploadDone, onUploadDone);
25+
dropzoneInst.handleFiles([file]);
26+
});
1727
}
1828

1929
class TextareaEditor {
@@ -82,48 +92,25 @@ class CodeMirrorEditor {
8292
}
8393
}
8494

85-
async function handleClipboardImages(editor, dropzone, images, e) {
86-
const uploadUrl = dropzone.getAttribute('data-upload-url');
87-
const filesContainer = dropzone.querySelector('.files');
88-
89-
if (!dropzone || !uploadUrl || !filesContainer || !images.length) return;
90-
95+
async function handleUploadFiles(editor, dropzoneEl, files, e) {
9196
e.preventDefault();
92-
e.stopPropagation();
93-
94-
for (const img of images) {
95-
const name = img.name.slice(0, img.name.lastIndexOf('.'));
97+
for (const file of files) {
98+
const name = file.name.slice(0, file.name.lastIndexOf('.'));
99+
const {width, dppx} = await imageInfo(file);
100+
const placeholder = `[${name}](uploading ...)`;
96101

97-
const placeholder = `![${name}](uploading ...)`;
98102
editor.insertPlaceholder(placeholder);
99-
100-
const {uuid} = await uploadFile(img, uploadUrl);
101-
const {width, dppx} = await imageInfo(img);
102-
103-
let text;
104-
if (width > 0 && dppx > 1) {
105-
// Scale down images from HiDPI monitors. This uses the <img> tag because it's the only
106-
// method to change image size in Markdown that is supported by all implementations.
107-
// Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}"
108-
const url = `attachments/${uuid}`;
109-
text = `<img width="${Math.round(width / dppx)}" alt="${htmlEscape(name)}" src="${htmlEscape(url)}">`;
110-
} else {
111-
// Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}"
112-
// TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments"
113-
const url = `/attachments/${uuid}`;
114-
text = `![${name}](${url})`;
115-
}
116-
editor.replacePlaceholder(placeholder, text);
117-
118-
const input = document.createElement('input');
119-
input.setAttribute('name', 'files');
120-
input.setAttribute('type', 'hidden');
121-
input.setAttribute('id', uuid);
122-
input.value = uuid;
123-
filesContainer.append(input);
103+
await uploadFile(dropzoneEl, file); // the "file" will get its "uuid" during the upload
104+
editor.replacePlaceholder(placeholder, generateMarkdownLinkForAttachment(file, {width, dppx}));
124105
}
125106
}
126107

108+
export function removeAttachmentLinksFromMarkdown(text, fileUuid) {
109+
text = text.replace(new RegExp(`!?\\[([^\\]]+)\\]\\(/?attachments/${fileUuid}\\)`, 'g'), '');
110+
text = text.replace(new RegExp(`<img[^>]+src="/?attachments/${fileUuid}"[^>]*>`, 'g'), '');
111+
return text;
112+
}
113+
127114
function handleClipboardText(textarea, e, {text, isShiftDown}) {
128115
// pasting with "shift" means "paste as original content" in most applications
129116
if (isShiftDown) return; // let the browser handle it
@@ -139,16 +126,37 @@ function handleClipboardText(textarea, e, {text, isShiftDown}) {
139126
// else, let the browser handle it
140127
}
141128

142-
export function initEasyMDEPaste(easyMDE, dropzone) {
129+
// extract text and images from "paste" event
130+
function getPastedContent(e) {
131+
const images = [];
132+
for (const item of e.clipboardData?.items ?? []) {
133+
if (item.type?.startsWith('image/')) {
134+
images.push(item.getAsFile());
135+
}
136+
}
137+
const text = e.clipboardData?.getData?.('text') ?? '';
138+
return {text, images};
139+
}
140+
141+
export function initEasyMDEPaste(easyMDE, dropzoneEl) {
142+
const editor = new CodeMirrorEditor(easyMDE.codemirror);
143143
easyMDE.codemirror.on('paste', (_, e) => {
144144
const {images} = getPastedContent(e);
145-
if (images.length) {
146-
handleClipboardImages(new CodeMirrorEditor(easyMDE.codemirror), dropzone, images, e);
147-
}
145+
if (!images.length) return;
146+
handleUploadFiles(editor, dropzoneEl, images, e);
147+
});
148+
easyMDE.codemirror.on('drop', (_, e) => {
149+
if (!e.dataTransfer.files.length) return;
150+
handleUploadFiles(editor, dropzoneEl, e.dataTransfer.files, e);
151+
});
152+
dropzoneEl.dropzone.on(DropzoneCustomEventRemovedFile, ({fileUuid}) => {
153+
const oldText = easyMDE.codemirror.getValue();
154+
const newText = removeAttachmentLinksFromMarkdown(oldText, fileUuid);
155+
if (oldText !== newText) easyMDE.codemirror.setValue(newText);
148156
});
149157
}
150158

151-
export function initTextareaPaste(textarea, dropzone) {
159+
export function initTextareaUpload(textarea, dropzoneEl) {
152160
let isShiftDown = false;
153161
textarea.addEventListener('keydown', (e) => {
154162
if (e.shiftKey) isShiftDown = true;
@@ -159,9 +167,17 @@ export function initTextareaPaste(textarea, dropzone) {
159167
textarea.addEventListener('paste', (e) => {
160168
const {images, text} = getPastedContent(e);
161169
if (images.length) {
162-
handleClipboardImages(new TextareaEditor(textarea), dropzone, images, e);
170+
handleUploadFiles(new TextareaEditor(textarea), dropzoneEl, images, e);
163171
} else if (text) {
164172
handleClipboardText(textarea, e, {text, isShiftDown});
165173
}
166174
});
175+
textarea.addEventListener('drop', (e) => {
176+
if (!e.dataTransfer.files.length) return;
177+
handleUploadFiles(new TextareaEditor(textarea), dropzoneEl, e.dataTransfer.files, e);
178+
});
179+
dropzoneEl.dropzone.on(DropzoneCustomEventRemovedFile, ({fileUuid}) => {
180+
const newText = removeAttachmentLinksFromMarkdown(textarea.value, fileUuid);
181+
if (textarea.value !== newText) textarea.value = newText;
182+
});
167183
}

‎web_src/js/features/comp/EditorUpload.test.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import {removeAttachmentLinksFromMarkdown} from './EditorUpload.js';
2+
3+
test('removeAttachmentLinksFromMarkdown', () => {
4+
expect(removeAttachmentLinksFromMarkdown('a foo b', 'foo')).toBe('a foo b');
5+
expect(removeAttachmentLinksFromMarkdown('a [x](attachments/foo) b', 'foo')).toBe('a b');
6+
expect(removeAttachmentLinksFromMarkdown('a ![x](attachments/foo) b', 'foo')).toBe('a b');
7+
expect(removeAttachmentLinksFromMarkdown('a [x](/attachments/foo) b', 'foo')).toBe('a b');
8+
expect(removeAttachmentLinksFromMarkdown('a ![x](/attachments/foo) b', 'foo')).toBe('a b');
9+
10+
expect(removeAttachmentLinksFromMarkdown('a <img src="attachments/foo"> b', 'foo')).toBe('a b');
11+
expect(removeAttachmentLinksFromMarkdown('a <img width="100" src="attachments/foo"> b', 'foo')).toBe('a b');
12+
expect(removeAttachmentLinksFromMarkdown('a <img src="/attachments/foo"> b', 'foo')).toBe('a b');
13+
expect(removeAttachmentLinksFromMarkdown('a <img src="/attachments/foo" width="100"/> b', 'foo')).toBe('a b');
14+
});

‎web_src/js/features/dropzone.js‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@ import {showTemporaryTooltip} from '../modules/tippy.js';
55
import {GET, POST} from '../modules/fetch.js';
66
import {showErrorToast} from '../modules/toast.js';
77
import {createElementFromHTML, createElementFromAttrs} from '../utils/dom.js';
8+
import {isImageFile, isVideoFile} from '../utils.js';
89

910
const {csrfToken, i18n} = window.config;
1011

12+
// dropzone has its owner event dispatcher (emitter)
13+
export const DropzoneCustomEventReloadFiles = 'dropzone-custom-reload-files';
14+
export const DropzoneCustomEventRemovedFile = 'dropzone-custom-removed-file';
15+
export const DropzoneCustomEventUploadDone = 'dropzone-custom-upload-done';
16+
1117
async function createDropzone(el, opts) {
1218
const [{Dropzone}] = await Promise.all([
1319
import(/* webpackChunkName: "dropzone" */'dropzone'),
@@ -16,6 +22,26 @@ async function createDropzone(el, opts) {
1622
return new Dropzone(el, opts);
1723
}
1824

25+
export function generateMarkdownLinkForAttachment(file, {width, dppx} = {}) {
26+
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
27+
if (isImageFile(file)) {
28+
fileMarkdown = `!${fileMarkdown}`;
29+
if (width > 0 && dppx > 1) {
30+
// Scale down images from HiDPI monitors. This uses the <img> tag because it's the only
31+
// method to change image size in Markdown that is supported by all implementations.
32+
// Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}"
33+
fileMarkdown = `<img width="${Math.round(width / dppx)}" alt="${htmlEscape(file.name)}" src="attachments/${htmlEscape(file.uuid)}">`;
34+
} else {
35+
// Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}"
36+
// TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments"
37+
fileMarkdown = `![${file.name}](/attachments/${file.uuid})`;
38+
}
39+
} else if (isVideoFile(file)) {
40+
fileMarkdown = `<video src="attachments/${htmlEscape(file.uuid)}" title="${htmlEscape(file.name)}" controls></video>`;
41+
}
42+
return fileMarkdown;
43+
}
44+
1945
function addCopyLink(file) {
2046
// Create a "Copy Link" element, to conveniently copy the image or file link as Markdown to the clipboard
2147
// The "<a>" element has a hardcoded cursor: pointer because the default is overridden by .dropzone
@@ -25,13 +51,7 @@ function addCopyLink(file) {
2551
</div>`);
2652
copyLinkEl.addEventListener('click', async (e) => {
2753
e.preventDefault();
28-
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
29-
if (file.type?.startsWith('image/')) {
30-
fileMarkdown = `!${fileMarkdown}`;
31-
} else if (file.type?.startsWith('video/')) {
32-
fileMarkdown = `<video src="/attachments/${htmlEscape(file.uuid)}" title="${htmlEscape(file.name)}" controls></video>`;
33-
}
34-
const success = await clippie(fileMarkdown);
54+
const success = await clippie(generateMarkdownLinkForAttachment(file));
3555
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
3656
});
3757
file.previewTemplate.append(copyLinkEl);
@@ -68,16 +88,19 @@ export async function initDropzone(dropzoneEl) {
6888
// "http://localhost:3000/owner/repo/issues/[object%20Event]"
6989
// the reason is that the preview "callback(dataURL)" is assign to "img.onerror" then "thumbnail" uses the error object as the dataURL and generates '<img src="[object Event]">'
7090
const dzInst = await createDropzone(dropzoneEl, opts);
71-
dzInst.on('success', (file, data) => {
72-
file.uuid = data.uuid;
91+
dzInst.on('success', (file, resp) => {
92+
file.uuid = resp.uuid;
7393
fileUuidDict[file.uuid] = {submitted: false};
74-
const input = createElementFromAttrs('input', {name: 'files', type: 'hidden', id: `dropzone-file-${data.uuid}`, value: data.uuid});
94+
const input = createElementFromAttrs('input', {name: 'files', type: 'hidden', id: `dropzone-file-${resp.uuid}`, value: resp.uuid});
7595
dropzoneEl.querySelector('.files').append(input);
7696
addCopyLink(file);
97+
dzInst.emit(DropzoneCustomEventUploadDone, {file});
7798
});
7899

79100
dzInst.on('removedfile', async (file) => {
80101
if (disableRemovedfileEvent) return;
102+
103+
dzInst.emit(DropzoneCustomEventRemovedFile, {fileUuid: file.uuid});
81104
document.querySelector(`#dropzone-file-${file.uuid}`)?.remove();
82105
// when the uploaded file number reaches the limit, there is no uuid in the dict, and it doesn't need to be removed from server
83106
if (removeAttachmentUrl && fileUuidDict[file.uuid] && !fileUuidDict[file.uuid].submitted) {
@@ -91,7 +114,7 @@ export async function initDropzone(dropzoneEl) {
91114
}
92115
});
93116

94-
dzInst.on('reload', async () => {
117+
dzInst.on(DropzoneCustomEventReloadFiles, async () => {
95118
try {
96119
const resp = await GET(listAttachmentsUrl);
97120
const respData = await resp.json();
@@ -104,13 +127,14 @@ export async function initDropzone(dropzoneEl) {
104127
for (const el of dropzoneEl.querySelectorAll('.dz-preview')) el.remove();
105128
fileUuidDict = {};
106129
for (const attachment of respData) {
107-
const imgSrc = `${attachmentBaseLinkUrl}/${attachment.uuid}`;
108-
dzInst.emit('addedfile', attachment);
109-
dzInst.emit('thumbnail', attachment, imgSrc);
110-
dzInst.emit('complete', attachment);
111-
addCopyLink(attachment);
112-
fileUuidDict[attachment.uuid] = {submitted: true};
113-
const input = createElementFromAttrs('input', {name: 'files', type: 'hidden', id: `dropzone-file-${attachment.uuid}`, value: attachment.uuid});
130+
const file = {name: attachment.name, uuid: attachment.uuid, size: attachment.size};
131+
const imgSrc = `${attachmentBaseLinkUrl}/${file.uuid}`;
132+
dzInst.emit('addedfile', file);
133+
dzInst.emit('thumbnail', file, imgSrc);
134+
dzInst.emit('complete', file);
135+
addCopyLink(file); // it is from server response, so no "type"
136+
fileUuidDict[file.uuid] = {submitted: true};
137+
const input = createElementFromAttrs('input', {name: 'files', type: 'hidden', id: `dropzone-file-${file.uuid}`, value: file.uuid});
114138
dropzoneEl.querySelector('.files').append(input);
115139
}
116140
if (!dropzoneEl.querySelector('.dz-preview')) {
@@ -129,6 +153,6 @@ export async function initDropzone(dropzoneEl) {
129153
dzInst.removeFile(file);
130154
});
131155

132-
if (listAttachmentsUrl) dzInst.emit('reload');
156+
if (listAttachmentsUrl) dzInst.emit(DropzoneCustomEventReloadFiles);
133157
return dzInst;
134158
}

‎web_src/js/utils.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import {encode, decode} from 'uint8-to-base64';
22

33
// transform /path/to/file.ext to file.ext
4-
export function basename(path = '') {
4+
export function basename(path) {
55
const lastSlashIndex = path.lastIndexOf('/');
66
return lastSlashIndex < 0 ? path : path.substring(lastSlashIndex + 1);
77
}
88

99
// transform /path/to/file.ext to .ext
10-
export function extname(path = '') {
10+
export function extname(path) {
11+
const lastSlashIndex = path.lastIndexOf('/');
1112
const lastPointIndex = path.lastIndexOf('.');
13+
if (lastSlashIndex > lastPointIndex) return '';
1214
return lastPointIndex < 0 ? '' : path.substring(lastPointIndex);
1315
}
1416

@@ -142,3 +144,11 @@ export function serializeXml(node) {
142144
}
143145

144146
export const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
147+
148+
export function isImageFile({name, type}) {
149+
return /\.(jpe?g|png|gif|webp|svg|heic)$/i.test(name || '') || type?.startsWith('image/');
150+
}
151+
152+
export function isVideoFile({name, type}) {
153+
return /\.(mpe?g|mp4|mkv|webm)$/i.test(name || '') || type?.startsWith('video/');
154+
}

0 commit comments

Comments
 (0)