Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

upload: Separate uploads into one upload bar per file. 

Fixes #24287.
This commit is contained in:
evykassirer 2023-03-27 22:54:27 -07:00 committed by Tim Abbott
parent 4e7f9100fc
commit b07bc23e07
4 changed files with 142 additions and 225 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

193
web/src/upload.js
View File

@ -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));
});

2
web/templates/compose_banner/upload_banner.hbs
View File

@ -1,4 +1,4 @@
<div class="upload_banner compose_banner {{banner_type}}">
<div class="upload_banner file_{{file_id}} compose_banner {{banner_type}}">
<div class="moving_bar"></div>
<p class="upload_msg">{{banner_text}}</p>
<a role="button" class="zulip-icon zulip-icon-close compose_banner_close_button"></a>

2
web/templates/message_edit_form.hbs
View File

@ -1,7 +1,7 @@
{{! Client-side Handlebars template for rendering the message edit form. }}
<form id="edit_form_{{message_id}}" class="new-style">
<div class="banners"></div>
<div class="edit_form_banners"></div>
<div class="edit-controls">
{{> copy_message_button message_id=this.message_id}}
<textarea class="message_edit_content" maxlength="{{ max_message_length }}">{{content}}</textarea>

170
web/tests/upload.test.js
View File

@ -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");
});