diff --git a/web/src/upload.js b/web/src/upload.js index ca35a1a118..40216ff6f8 100644 --- a/web/src/upload.js +++ b/web/src/upload.js @@ -23,7 +23,7 @@ export function get_translated_status(file) { return "[" + status + "]()"; } -export function get_item(key, config) { +export function get_item(key, config, file_id) { if (!config) { throw new Error("Missing config"); } @@ -36,13 +36,17 @@ export function get_item(key, config) { case "banner_container": return $("#compose_banners"); case "upload_banner_identifier": - return "#compose_banners .upload_banner"; + return `#compose_banners .upload_banner.file_${CSS.escape(file_id)}`; case "upload_banner": - return $("#compose_banners .upload_banner"); + return $(`#compose_banners .upload_banner.file_${CSS.escape(file_id)}`); case "upload_banner_close_button": - return $("#compose_banners .upload_banner .compose_banner_close_button"); + return $( + `#compose_banners .upload_banner.file_${CSS.escape( + file_id, + )} .compose_banner_close_button`, + ); case "upload_banner_message": - return $("#compose_banners .upload_banner .upload_msg"); + return $(`#compose_banners .upload_banner.file_${CSS.escape(file_id)} .upload_msg`); case "file_input_identifier": return "#compose .file_input"; case "source": @@ -66,19 +70,29 @@ export function get_item(key, config) { .closest(".message_edit_form") .find(".message_edit_save"); case "banner_container": - return $(`#edit_form_${CSS.escape(config.row)} .banners`); + return $(`#edit_form_${CSS.escape(config.row)} .edit_form_banners`); case "upload_banner_identifier": - return `#edit_form_${CSS.escape(config.row)} .upload_banner`; + return `#edit_form_${CSS.escape(config.row)} .upload_banner.file_${CSS.escape( + file_id, + )}`; case "upload_banner": - return $(`#edit_form_${CSS.escape(config.row)} .upload_banner`); + return $( + `#edit_form_${CSS.escape(config.row)} .upload_banner.file_${CSS.escape( + file_id, + )}`, + ); case "upload_banner_close_button": return $( - `#edit_form_${CSS.escape( - config.row, - )} .upload_banner .compose_banner_close_button`, + `#edit_form_${CSS.escape(config.row)} .upload_banner.file_${CSS.escape( + file_id, + )} .compose_banner_close_button`, ); case "upload_banner_message": - return $(`#edit_form_${CSS.escape(config.row)} .upload_banner .upload_msg`); + return $( + `#edit_form_${CSS.escape(config.row)} .upload_banner.file_${CSS.escape( + file_id, + )} .upload_msg`, + ); case "file_input_identifier": return `#edit_form_${CSS.escape(config.row)} .file_input`; case "source": @@ -95,36 +109,18 @@ export function get_item(key, config) { } } -export function hide_upload_banner(config) { - get_item("send_button", config).prop("disabled", false); - get_item("upload_banner", config).remove(); +export function hide_upload_banner(uppy, config, file_id) { + get_item("upload_banner", config, file_id).remove(); + if (uppy.getFiles().length === 0) { + get_item("send_button", config).prop("disabled", false); + } } -function show_upload_banner(config, banner_type, banner_text) { - // We only show one upload banner at a time per compose box, - // and all uploads are combined into the same progress bar. - // TODO: It would be nice to separate the error banner into - // a different element, so that we can show it at the same - // time as the upload bar and other uploads can still continue - // when an error occurs. - const $upload_banner = get_item("upload_banner", config); - if ($upload_banner.length) { - if (banner_type === "error") { - // Hide moving bar so that it doesn't do the 1s transition to 0 - const $moving_bar = $(`${get_item("upload_banner_identifier", config)} .moving_bar`); - $moving_bar.hide(); - $upload_banner.removeClass("info").addClass("error"); - // Show it again once the animation is complete. - setTimeout(() => $moving_bar.show(), 1000); - } else { - $upload_banner.removeClass("error").addClass("info"); - } - get_item("upload_banner_message", config).text(banner_text); - return; - } +function add_upload_banner(config, banner_type, banner_text, file_id) { const new_banner = render_upload_banner({ banner_type, banner_text, + file_id, }); get_item("banner_container", config).append(new_banner); } @@ -132,9 +128,19 @@ function show_upload_banner(config, banner_type, banner_text) { export function show_error_message( config, message = $t({defaultMessage: "An unknown error occurred."}), + file_id = null, ) { get_item("send_button", config).prop("disabled", false); - show_upload_banner(config, "error", message); + if (file_id) { + $(`${get_item("upload_banner_identifier", config, file_id)} .moving_bar`).hide(); + get_item("upload_banner", config, file_id).removeClass("info").addClass("error"); + get_item("upload_banner_message", config).text(message); + } else { + // We still use a "file_id" (that's not actually related to a file) + // to differentiate this banner from banners that *are* associated + // with files. This is notably relevant for the close click handler. + add_upload_banner(config, "error", message, "generic_error"); + } } export async function upload_files(uppy, config, files) { @@ -163,20 +169,6 @@ export async function upload_files(uppy, config, files) { } get_item("send_button", config).prop("disabled", true); - show_upload_banner(config, "info", $t({defaultMessage: "Uploading…"})); - get_item("upload_banner_close_button", config).one("click", () => { - for (const file of uppy.getFiles()) { - compose_ui.replace_syntax( - get_translated_status(file), - "", - get_item("textarea", config), - ); - } - compose_ui.autosize_textarea(get_item("textarea", config)); - uppy.cancelAll(); - get_item("textarea", config).trigger("focus"); - hide_upload_banner(config); - }); for (const file of files) { try { @@ -187,7 +179,7 @@ export async function upload_files(uppy, config, files) { 1, ); compose_ui.autosize_textarea(get_item("textarea", config)); - uppy.addFile({ + file.id = uppy.addFile({ source: get_item("source", config), name: file.name, type: file.type, @@ -195,8 +187,27 @@ export async function upload_files(uppy, config, files) { }); } catch { // Errors are handled by info-visible and upload-error event callbacks. - break; + continue; } + + add_upload_banner( + config, + "info", + $t({defaultMessage: "Uploading {filename}…"}, {filename: file.name}), + file.id, + ); + get_item("upload_banner_close_button", config, file.id).one("click", () => { + compose_ui.replace_syntax( + get_translated_status(file), + "", + get_item("textarea", config), + ); + compose_ui.autosize_textarea(get_item("textarea", config)); + get_item("textarea", config).trigger("focus"); + + uppy.removeFile(file.id); + hide_upload_banner(uppy, config, file.id); + }); } } @@ -238,13 +249,10 @@ export function setup_upload(config) { }, }); - uppy.on("progress", (progress) => { - // When upload is complete, it resets to 0, but we want to see it at 100%. - if (progress === 0) { - return; - } - $(`${get_item("upload_banner_identifier", config)} .moving_bar`).css({ - width: `${progress}%`, + uppy.on("upload-progress", (file, progress) => { + const percent_complete = (100 * progress.bytesUploaded) / progress.bytesTotal; + $(`${get_item("upload_banner_identifier", config, file.id)} .moving_bar`).css({ + width: `${percent_complete}%`, }); }); @@ -255,6 +263,26 @@ export function setup_upload(config) { event.target.value = ""; }); + // These are close-click handlers for error banners that aren't associated + // with a particular file. + $("#compose_banners").on( + "click", + ".upload_banner.file_generic_error .compose_banner_close_button", + (event) => { + event.preventDefault(); + $(event.target).parents(".upload_banner").remove(); + }, + ); + + $("#edit_form_banners").on( + "click", + ".upload_banner.file_generic_error .compose_banner_close_button", + (event) => { + event.preventDefault(); + $(event.target).parents(".upload_banner").remove(); + }, + ); + const $drag_drop_container = get_item("drag_drop_container", config); $drag_drop_container.on("dragover", (event) => event.preventDefault()); $drag_drop_container.on("dragenter", (event) => event.preventDefault()); @@ -299,42 +327,27 @@ export function setup_upload(config) { get_item("textarea", config), ); compose_ui.autosize_textarea(get_item("textarea", config)); - }); - uppy.on("complete", () => { - let uploads_in_progress = false; - for (const file of uppy.getFiles()) { - if (file.progress.uploadComplete) { - // The uploaded files should be removed since uppy don't allow files in the store - // to be re-uploaded again. - uppy.removeFile(file.id); - } else { - // Happens when user tries to upload files when there is already an existing batch - // being uploaded. So when the first batch of files complete, the second batch would - // still be in progress. - uploads_in_progress = true; - } - } - - const has_errors = get_item("upload_banner", config).hasClass("error"); - if (!uploads_in_progress && !has_errors) { - // Hide upload status for 100ms after the 1s transition to 100% - // so that the user can see the progress bar at 100%. - setTimeout(() => { - hide_upload_banner(config); - }, 1100); - } + // The uploaded files should be removed since uppy doesn't allow files in the store + // to be re-uploaded again. + uppy.removeFile(file.id); + // Hide upload status after waiting 100ms after the 1s transition to 100% + // so that the user can see the progress bar at 100%. + setTimeout(() => { + hide_upload_banner(uppy, config, file.id); + }, 1100); }); uppy.on("info-visible", () => { - // Uppy's `info-visible` event is issued after prepending the + // Uppy's `info-visible` event is issued after appending the // notice details into the list of event events accessed via // uppy.getState().info. Extract the notice details so that we // can potentially act on the error. // // TODO: Ideally, we'd be using the `.error()` hook or // something, not parsing error message strings. - const info = uppy.getState().info[0]; + const infoList = uppy.getState().info; + const info = infoList[infoList.length - 1]; if (info.type === "error" && info.message === "No Internet connection") { // server_events already handles the case of no internet. return; @@ -350,17 +363,13 @@ export function setup_upload(config) { if (info.type === "error") { // The remaining errors are mostly frontend errors like file being too large // for upload. - // TODO: It would be nice to keep the other uploads going if one fails, - // and show both an error message and the upload bar. - uppy.cancelAll(); show_error_message(config, info.message); } }); uppy.on("upload-error", (file, error, response) => { const message = response ? response.body.msg : undefined; - uppy.cancelAll(); - show_error_message(config, message); + show_error_message(config, message, file.id); compose_ui.replace_syntax(get_translated_status(file), "", get_item("textarea", config)); compose_ui.autosize_textarea(get_item("textarea", config)); }); diff --git a/web/templates/compose_banner/upload_banner.hbs b/web/templates/compose_banner/upload_banner.hbs index e877700fdd..de2b2ca6c0 100644 --- a/web/templates/compose_banner/upload_banner.hbs +++ b/web/templates/compose_banner/upload_banner.hbs @@ -1,4 +1,4 @@ -
+

{{banner_text}}

diff --git a/web/templates/message_edit_form.hbs b/web/templates/message_edit_form.hbs index daf5837e3e..e86f0fa15b 100644 --- a/web/templates/message_edit_form.hbs +++ b/web/templates/message_edit_form.hbs @@ -1,7 +1,7 @@ {{! Client-side Handlebars template for rendering the message edit form. }}
-
+
{{> copy_message_button message_id=this.message_id}} diff --git a/web/tests/upload.test.js b/web/tests/upload.test.js index bc1d94672e..6c02feefc8 100644 --- a/web/tests/upload.test.js +++ b/web/tests/upload.test.js @@ -46,12 +46,12 @@ test("feature_check", ({override}) => { test("get_item", () => { assert.equal(upload.get_item("textarea", {mode: "compose"}), $("#compose-textarea")); assert.equal( - upload.get_item("upload_banner_message", {mode: "compose"}), - $("#compose_banners .upload_banner .upload_msg"), + upload.get_item("upload_banner_message", {mode: "compose"}, "id_1"), + $("#compose_banners .upload_banner.file_id_1 .upload_msg"), ); assert.equal( - upload.get_item("upload_banner_close_button", {mode: "compose"}), - $("#compose_banners .upload_banner .compose_banner_close_button"), + upload.get_item("upload_banner_close_button", {mode: "compose"}, "id_2"), + $("#compose_banners .upload_banner.file_id_2 .compose_banner_close_button"), ); assert.equal( upload.get_item("file_input_identifier", {mode: "compose"}), @@ -76,12 +76,12 @@ test("get_item", () => { assert.equal(upload.get_item("send_button", {mode: "edit", row: 2}), $(".message_edit_save")); assert.equal( - upload.get_item("upload_banner_identifier", {mode: "edit", row: 11}), - `#edit_form_${CSS.escape(11)} .upload_banner`, + upload.get_item("upload_banner_identifier", {mode: "edit", row: 11}, "id_3"), + `#edit_form_${CSS.escape(11)} .upload_banner.file_id_3`, ); assert.equal( - upload.get_item("upload_banner", {mode: "edit", row: 75}), - $(`#edit_form_${CSS.escape(75)} .upload_banner`), + upload.get_item("upload_banner", {mode: "edit", row: 75}, "id_60"), + $(`#edit_form_${CSS.escape(75)} .upload_banner.file_id_60`), ); $(`#edit_form_${CSS.escape(2)} .upload_banner`).set_find_results( @@ -89,17 +89,17 @@ test("get_item", () => { $(".compose_banner_close_button"), ); assert.equal( - upload.get_item("upload_banner_close_button", {mode: "edit", row: 2}), - $(`#edit_form_${CSS.escape(2)} .upload_banner .compose_banner_close_button`), + upload.get_item("upload_banner_close_button", {mode: "edit", row: 2}, "id_34"), + $(`#edit_form_${CSS.escape(2)} .upload_banner.file_id_34 .compose_banner_close_button`), ); - $(`#edit_form_${CSS.escape(22)} .upload_banner`).set_find_results( + $(`#edit_form_${CSS.escape(22)} .upload_banner.file_id_234`).set_find_results( ".upload_msg", $(".upload_msg"), ); assert.equal( - upload.get_item("upload_banner_message", {mode: "edit", row: 22}), - $(`#edit_form_${CSS.escape(22)} .upload_banner .upload_msg`), + upload.get_item("upload_banner_message", {mode: "edit", row: 22}, "id_234"), + $(`#edit_form_${CSS.escape(22)} .upload_banner.file_id_234 .upload_msg`), ); assert.equal( @@ -163,20 +163,6 @@ test("get_item", () => { ); }); -test("hide_upload_banner", () => { - let banner_removed = false; - $("#compose_banners .upload_banner").remove = () => { - banner_removed = true; - }; - $("#compose-send-button").prop("disabled", true); - - upload.hide_upload_banner({mode: "compose"}); - - assert.ok(banner_removed); - assert.equal($("#compose-send-button").prop("disabled"), false); - assert.equal($("#compose-send-button").visible(), false); -}); - test("show_error_message", ({mock_template}) => { $("#compose_banners .upload_banner .moving_bar").css = () => {}; $("#compose_banners .upload_banner").length = 0; @@ -207,7 +193,6 @@ test("upload_files", async ({mock_template, override_rewire}) => { $("#compose_banners .upload_banner .moving_bar").css = () => {}; $("#compose_banners .upload_banner").length = 0; - let uppy_cancel_all_called = false; let files = [ { name: "budapest.png", @@ -215,21 +200,22 @@ test("upload_files", async ({mock_template, override_rewire}) => { }, ]; let uppy_add_file_called = false; + let remove_file_called = false; const uppy = { - cancelAll() { - uppy_cancel_all_called = true; - }, addFile(params) { uppy_add_file_called = true; assert.equal(params.source, "compose-file-input"); assert.equal(params.name, "budapest.png"); assert.equal(params.type, "image/png"); assert.equal(params.data, files[0]); + return "id_123"; + }, + removeFile() { + remove_file_called = true; }, - getFiles: () => [...files], }; let hide_upload_banner_called = false; - override_rewire(upload, "hide_upload_banner", (config) => { + override_rewire(upload, "hide_upload_banner", (uppy, config) => { hide_upload_banner_called = true; assert.equal(config.mode, "compose"); }); @@ -254,14 +240,15 @@ test("upload_files", async ({mock_template, override_rewire}) => { page_params.max_file_upload_size_mib = 25; let on_click_close_button_callback; - $("#compose_banners .upload_banner .compose_banner_close_button").one = (event, callback) => { + $("#compose_banners .upload_banner.file_id_123 .compose_banner_close_button").one = ( + event, + callback, + ) => { assert.equal(event, "click"); on_click_close_button_callback = callback; }; let compose_ui_insert_syntax_and_focus_called = false; - override_rewire(compose_ui, "insert_syntax_and_focus", (syntax, textarea) => { - assert.equal(syntax, "[translated: Uploading budapest.png…]()"); - assert.equal(textarea, $("#compose-textarea")); + override_rewire(compose_ui, "insert_syntax_and_focus", () => { compose_ui_insert_syntax_and_focus_called = true; }); let compose_ui_autosize_textarea_called = false; @@ -277,9 +264,7 @@ test("upload_files", async ({mock_template, override_rewire}) => { $("#compose .undo_markdown_preview").show(); banner_shown = false; - mock_template("compose_banner/upload_banner.hbs", false, (data) => { - assert.equal(data.banner_type, "info"); - assert.equal(data.banner_text, "translated: Uploading…"); + mock_template("compose_banner/upload_banner.hbs", false, () => { banner_shown = true; }); await upload.upload_files(uppy, config, files); @@ -303,23 +288,19 @@ test("upload_files", async ({mock_template, override_rewire}) => { ]; let add_file_counter = 0; uppy.addFile = (file) => { - assert.equal(file.name, "budapest.png"); add_file_counter += 1; - throw new Error("some error"); + if (file.name === "budapest.png") { + throw new Error("some error"); + } + return `id_${add_file_counter}`; }; await upload.upload_files(uppy, config, files); assert.ok(banner_shown); - assert.equal(add_file_counter, 1); + assert.equal(add_file_counter, 2); hide_upload_banner_called = false; - uppy_cancel_all_called = false; let compose_ui_replace_syntax_called = false; - files = [ - { - name: "budapest.png", - type: "image/png", - }, - ]; + override_rewire(compose_ui, "replace_syntax", (old_syntax, new_syntax, textarea) => { compose_ui_replace_syntax_called = true; assert.equal(old_syntax, "[translated: Uploading budapest.png…]()"); @@ -327,14 +308,17 @@ test("upload_files", async ({mock_template, override_rewire}) => { assert.equal(textarea, $("#compose-textarea")); }); on_click_close_button_callback(); - assert.ok(uppy_cancel_all_called); + assert.ok(remove_file_called); assert.ok(hide_upload_banner_called); assert.ok(compose_ui_autosize_textarea_called); assert.ok(compose_ui_replace_syntax_called); hide_upload_banner_called = false; compose_ui_replace_syntax_called = false; + remove_file_called = false; $("#compose-textarea").val("user modified text"); + on_click_close_button_callback(); + assert.ok(remove_file_called); assert.ok(hide_upload_banner_called); assert.ok(compose_ui_autosize_textarea_called); assert.ok(compose_ui_replace_syntax_called); @@ -487,24 +471,16 @@ test("uppy_events", ({override, override_rewire, mock_template}) => { $("#compose_banners .upload_banner").length = 0; const callbacks = {}; - let uppy_cancel_all_called = false; let state = {}; - let files = []; uppy_stub = function () { return { setMeta() {}, use() {}, - cancelAll() { - uppy_cancel_all_called = true; - }, on(event_name, callback) { callbacks[event_name] = callback; }, - getFiles: () => [...files], - removeFile(file_id) { - files = files.filter((file) => file.id !== file_id); - }, + removeFile() {}, getState: () => ({ info: [ { @@ -517,11 +493,12 @@ test("uppy_events", ({override, override_rewire, mock_template}) => { }; }; upload.setup_upload({mode: "compose"}); - assert.equal(Object.keys(callbacks).length, 6); + assert.equal(Object.keys(callbacks).length, 5); const on_upload_success_callback = callbacks["upload-success"]; const file = { name: "copenhagen.png", + id: "123", }; let response = { body: { @@ -564,58 +541,6 @@ test("uppy_events", ({override, override_rewire, mock_template}) => { assert.equal(compose_ui_replace_syntax_called, false); assert.equal(compose_ui_autosize_textarea_called, false); - const on_complete_callback = callbacks.complete; - set_global("setTimeout", (func) => { - func(); - }); - let hide_upload_banner_called = false; - override_rewire(upload, "hide_upload_banner", () => { - hide_upload_banner_called = true; - }); - $("#compose_banner .upload_banner").removeClass("error"); - files = [ - { - id: "uppy-zulip/jpeg-1e-image/jpeg-163515-1578367331279", - progress: { - uploadComplete: true, - }, - }, - { - id: "uppy-github/avatar/png-2v-1e-image/png-329746-1576507125831", - progress: { - uploadComplete: true, - }, - }, - ]; - on_complete_callback(); - assert.ok(hide_upload_banner_called); - assert.equal(files.length, 0); - - hide_upload_banner_called = false; - $("#compose_banners .upload_banner").addClass("error"); - on_complete_callback(); - assert.ok(!hide_upload_banner_called); - - $("#compose_banners .upload_banner").removeClass("error"); - hide_upload_banner_called = false; - files = [ - { - id: "uppy-zulip/jpeg-1e-image/jpeg-163515-1578367331279", - progress: { - uploadComplete: true, - }, - }, - { - id: "uppy-github/avatar/png-2v-1e-image/png-329746-1576507125831", - progress: { - uploadComplete: false, - }, - }, - ]; - on_complete_callback(); - assert.ok(!hide_upload_banner_called); - assert.equal(files.length, 1); - mock_template("compose_banner/upload_banner.hbs", false, (data) => { assert.equal(data.banner_type, "error"); assert.equal(data.banner_text, "Some error message"); @@ -627,11 +552,9 @@ test("uppy_events", ({override, override_rewire, mock_template}) => { }; const on_info_visible_callback = callbacks["info-visible"]; $("#compose_banners .upload_banner .upload_msg").text(""); - uppy_cancel_all_called = false; compose_ui_replace_syntax_called = false; const on_restriction_failed_callback = callbacks["restriction-failed"]; on_info_visible_callback(); - assert.ok(uppy_cancel_all_called); override_rewire(compose_ui, "replace_syntax", (old_syntax, new_syntax, textarea) => { compose_ui_replace_syntax_called = true; assert.equal(old_syntax, "[translated: Uploading copenhagen.png…]()"); @@ -650,47 +573,32 @@ test("uppy_events", ({override, override_rewire, mock_template}) => { type: "error", message: "No Internet connection", }; - uppy_cancel_all_called = false; on_info_visible_callback(); state = { type: "error", details: "Upload Error", }; - uppy_cancel_all_called = false; on_info_visible_callback(); const on_upload_error_callback = callbacks["upload-error"]; $("#compose_banners .upload_banner .upload_msg").text(""); - mock_template("compose_banner/upload_banner.hbs", false, (data) => { - assert.equal(data.banner_type, "error"); - assert.equal(data.banner_text, "Response message"); - }); compose_ui_replace_syntax_called = false; response = { body: { msg: "Response message", }, }; - uppy_cancel_all_called = false; on_upload_error_callback(file, null, response); - assert.ok(uppy_cancel_all_called); assert.ok(compose_ui_replace_syntax_called); - mock_template("compose_banner/upload_banner.hbs", false, (data) => { - assert.equal(data.banner_type, "error"); - assert.equal(data.banner_text, "translated: An unknown error occurred."); - }); compose_ui_replace_syntax_called = false; - uppy_cancel_all_called = false; on_upload_error_callback(file, null, null); - assert.ok(uppy_cancel_all_called); assert.ok(compose_ui_replace_syntax_called); + $("#compose_banners .upload_banner .upload_msg").text(""); $("#compose-textarea").val("user modified text"); - uppy_cancel_all_called = false; on_upload_error_callback(file, null); - assert.ok(uppy_cancel_all_called); assert.ok(compose_ui_replace_syntax_called); assert.equal($("#compose-textarea").val(), "user modified text"); });