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

compose: Migrate compose_not_subscribed to use compose_banner. 

This is part several updates for #22524.

This commit also removes `compose_error.hide()` since there is only
one remaining use of it (which has been replaced by the contents of
`hide()`). Eventually `#compose-send-status` should be replaced
fully in a future PR, after we rewrite the upload banner to use
the new banner template.

Testing note: I removed the test test_compose_not_subscribed_clicked
since it heavily relies on the old way of rendering banners and is too
UI-focused (instead of logic focused) for me to feel like it's worth
testing that the banner is removed when clicking the button.
This commit is contained in:
evykassirer 2022-08-22 15:12:29 -07:00 committed by Tim Abbott
parent 8ced075643
commit 086f02a3ab
11 changed files with 46 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
frontend_tests/node_tests/compose.js
View File

@ -40,7 +40,6 @@ const rendered_markdown = mock_esm("../../static/js/rendered_markdown");
const resize = mock_esm("../../static/js/resize");
const sent_messages = mock_esm("../../static/js/sent_messages");
const server_events = mock_esm("../../static/js/server_events");
const stream_settings_ui = mock_esm("../../static/js/stream_settings_ui");
const transmit = mock_esm("../../static/js/transmit");
const upload = mock_esm("../../static/js/upload");
@ -566,85 +565,6 @@ test_ui("on_events", ({override}) => {
override(rendered_markdown, "update_elements", () => {});
function setup_parents_and_mock_remove(container_sel, target_sel, parent) {
const $container = $.create("fake " + container_sel);
let container_removed = false;
$container.remove = () => {
container_removed = true;
};
const $target = $.create("fake click target (" + target_sel + ")");
$target.set_parents_result(parent, $container);
const event = {
preventDefault: noop,
stopPropagation: noop,
// FIXME: event.target should not be a jQuery object
target: $target,
};
const helper = {
event,
$container,
$target,
container_was_removed: () => container_removed,
};
return helper;
}
(function test_compose_not_subscribed_clicked() {
const handler = $("#compose-send-status").get_on_handler("click", ".sub_unsub_button");
const subscription = {
stream_id: 102,
name: "test",
subscribed: false,
};
let compose_not_subscribed_called = false;
stream_settings_ui.sub_or_unsub = () => {
compose_not_subscribed_called = true;
};
const helper = setup_parents_and_mock_remove(
"compose-send-status",
"sub_unsub_button",
".compose_not_subscribed",
);
handler(helper.event);
assert.ok(compose_not_subscribed_called);
stream_data.add_sub(subscription);
$("#stream_message_recipient_stream").val("test");
$("#compose-send-status").show();
handler(helper.event);
assert.ok(!$("#compose-send-status").visible());
})();
(function test_compose_not_subscribed_close_clicked() {
const handler = $("#compose-send-status").get_on_handler(
"click",
"#compose_not_subscribed_close",
);
const helper = setup_parents_and_mock_remove(
"compose_user_not_subscribed_close",
"compose_not_subscribed_close",
".compose_not_subscribed",
);
$("#compose-send-status").show();
handler(helper.event);
assert.ok(!$("#compose-send-status").visible());
})();
(function test_attach_files_compose_clicked() {
const handler = $("#compose").get_on_handler("click", ".compose_upload_file");
let compose_file_input_clicked = false;

2
frontend_tests/node_tests/compose_error.js
View File

@ -22,5 +22,5 @@ run_test("compose_error_test", () => {
return "none";
};
compose_error.hide();
$("#compose-send-status").stop(true).fadeOut(500);
});

13
frontend_tests/node_tests/compose_validate.js
View File

@ -80,9 +80,15 @@ test_ui("validate_stream_message_address_info", ({mock_template}) => {
sub.subscribed = false;
stream_data.add_sub(sub);
mock_template("compose_not_subscribed.hbs", false, () => "compose_not_subscribed_stub");
$("#compose_banners .user_not_subscribed").length = 0;
let user_not_subscribed_rendered = false;
mock_template("compose_banner/compose_banner.hbs", true, (data, html) => {
assert.equal(data.classname, compose_error.CLASSNAMES.user_not_subscribed);
user_not_subscribed_rendered = true;
return html;
});
assert.ok(!compose_validate.validate_stream_message_address_info("social"));
assert.equal($("#compose-error-msg").html(), "compose_not_subscribed_stub");
assert.ok(user_not_subscribed_rendered);
page_params.narrow_stream = false;
channel.post = (payload) => {
@ -100,8 +106,9 @@ test_ui("validate_stream_message_address_info", ({mock_template}) => {
payload.data.subscribed = false;
payload.success(payload.data);
};
user_not_subscribed_rendered = false;
assert.ok(!compose_validate.validate_stream_message_address_info("Frontend"));
assert.equal($("#compose-error-msg").html(), "compose_not_subscribed_stub");
assert.ok(user_not_subscribed_rendered);
let stream_does_not_exist_rendered = false;
mock_template("compose_banner/stream_does_not_exist_error.hbs", false, (data) => {

3
frontend_tests/zjsunit/zjquery_element.js
View File

@ -215,9 +215,6 @@ function FakeElement(selector, opts) {
shown = true;
return $self;
},
stop() {
return $self;
},
text(...args) {
if (args.length !== 0) {
if (args[0] !== undefined) {

3
static/js/click_handlers.js
View File

@ -14,7 +14,6 @@ import * as buddy_data from "./buddy_data";
import * as channel from "./channel";
import * as compose from "./compose";
import * as compose_actions from "./compose_actions";
import * as compose_error from "./compose_error";
import * as compose_state from "./compose_state";
import {media_breakpoints_num} from "./css_variables";
import * as dark_theme from "./dark_theme";
@ -716,7 +715,7 @@ export function initialize() {
// COMPOSE
$("body").on("click", "#compose-send-status .compose-send-status-close", () => {
compose_error.hide();
$("#compose-send-status").stop(true).fadeOut(500);
});
$("body").on("click", ".empty_feed_compose_stream", (e) => {

17
static/js/compose.js
View File

@ -461,7 +461,11 @@ export function initialize() {
},
);
$("#compose-send-status").on("click", ".sub_unsub_button", (event) => {
const user_not_subscribed_classname = `.${compose_error.CLASSNAMES.user_not_subscribed}`;
$("#compose_banners").on(
"click",
`${user_not_subscribed_classname} .compose_banner_action_button`,
(event) => {
event.preventDefault();
const stream_name = $("#stream_message_recipient_stream").val();
@ -470,14 +474,9 @@ export function initialize() {
}
const sub = stream_data.get_sub(stream_name);
stream_settings_ui.sub_or_unsub(sub);
$("#compose-send-status").hide();
});
$("#compose-send-status").on("click", "#compose_not_subscribed_close", (event) => {
event.preventDefault();
$("#compose-send-status").hide();
});
$(user_not_subscribed_classname).remove();
},
);
$("#compose_banners").on(
"click",

26
static/js/compose_error.ts
View File

@ -3,8 +3,6 @@ import $ from "jquery";
import render_compose_banner from "../templates/compose_banner/compose_banner.hbs";
import render_stream_does_not_exist_error from "../templates/compose_banner/stream_does_not_exist_error.hbs";
import * as common from "./common";
// banner types
export const WARNING = "warning";
export const ERROR = "error";
@ -30,6 +28,7 @@ export const CLASSNAMES = {
topic_missing: "topic_missing",
zephyr_not_running: "zephyr_not_running",
generic_compose_error: "generic_compose_error",
user_not_subscribed: "user_not_subscribed",
};
// TODO: Replace with compose_ui.hide_compose_spinner() when it is converted to ts.
@ -60,20 +59,6 @@ export function show_error_message(message: string, classname: string, $bad_inpu
}
}
export function show(error_html: string, $bad_input?: JQuery, alert_class = "alert-error"): void {
$("#compose-send-status")
.removeClass(common.status_classes)
.addClass(alert_class)
.stop(true)
.fadeTo(0, 1);
$("#compose-error-msg").html(error_html);
hide_compose_spinner();
if ($bad_input !== undefined) {
$bad_input.trigger("focus").trigger("select");
}
}
export function show_stream_does_not_exist_error(stream_name: string): void {
// Remove any existing banners with this warning.
$(`#compose_banners .${CLASSNAMES.stream_does_not_exist}`).remove();
@ -88,12 +73,3 @@ export function show_stream_does_not_exist_error(stream_name: string): void {
hide_compose_spinner();
$("#stream_message_recipient_stream").trigger("focus").trigger("select");
}
export function show_not_subscribed(error_html: string, $bad_input?: JQuery): void {
show(error_html, $bad_input, "home-error-bar");
$(".compose-send-status-close").hide();
}
export function hide(): void {
$("#compose-send-status").stop(true).fadeOut(500);
}

21
static/js/compose_validate.js
View File

@ -4,7 +4,6 @@ import * as resolved_topic from "../shared/js/resolved_topic";
import render_compose_banner from "../templates/compose_banner/compose_banner.hbs";
import render_not_subscribed_warning from "../templates/compose_banner/not_subscribed_warning.hbs";
import render_wildcard_warning from "../templates/compose_banner/wildcard_warning.hbs";
import render_compose_not_subscribed from "../templates/compose_not_subscribed.hbs";
import render_compose_private_stream_alert from "../templates/compose_private_stream_alert.hbs";
import * as channel from "./channel";
@ -397,11 +396,25 @@ export function validation_error(error_type, stream_name) {
);
return false;
case "not-subscribed": {
if ($(`#compose_banners .${compose_error.CLASSNAMES.user_not_subscribed}`).length) {
return false;
}
const sub = stream_data.get_sub(stream_name);
const new_row = render_compose_not_subscribed({
should_display_sub_button: stream_data.can_toggle_subscription(sub),
const new_row = render_compose_banner({
banner_type: compose_error.ERROR,
banner_text: $t({
defaultMessage:
"You're not subscribed to this stream. You will not be notified if other users reply to your message.",
}),
button_text: stream_data.can_toggle_subscription(sub)
? $t({defaultMessage: "Subscribe"})
: null,
classname: compose_error.CLASSNAMES.user_not_subscribed,
// The message cannot be sent until the user subscribes to the stream, so
// closing the banner would be more confusing than helpful.
hide_close_button: true,
});
compose_error.show_not_subscribed(new_row, $("#stream_message_recipient_stream"));
$("#compose_banners").append(new_row);
return false;
}
}

19
static/styles/compose.css
View File

@ -381,20 +381,13 @@
}
}
.compose_private_stream_alert,
.compose_not_subscribed {
.compose_private_stream_alert {
padding: 4px 0;
display: flex;
justify-content: space-between;
position: relative;
}
#compose_not_subscribed_close {
top: 0;
right: 0;
position: absolute;
}
.compose_private_stream_alert_controls .compose_private_stream_alert_close {
display: inline-block;
position: absolute;
@ -408,16 +401,6 @@
position: relative;
}
.compose_not_subscribed p {
margin: 5px 20px;
display: inline-block;
max-width: calc(100% - 100px);
}
#error-message-sub-button {
margin-right: 30px;
}
/* Like .nav-tabs > li > a */
div[id^="message-edit-send-status"],
#compose-send-status {

1
static/templates/compose.hbs
View File

@ -51,7 +51,6 @@
<span class="compose-send-status-close">&times;</span>
<span id="compose-error-msg"></span>
</div>
<div id="compose_not_subscribed" class="alert home-error-bar"></div>
<div id="compose_private_stream_alert" class="alert home-error-bar"></div>
<div id="out-of-view-notification" class="notification-alert"></div>
<div class="composition-area">

12
static/templates/compose_not_subscribed.hbs
View File

@ -1,12 +0,0 @@
<div class="compose_not_subscribed">
<p>
{{#tr}}
You're not subscribed to this stream. You will not be notified if other users reply to your message.
{{/tr}}
</p>
<div>
{{#if should_display_sub_button}}
<button id="error-message-sub-button" class="btn btn-warning subscribe-button sub_unsub_button">{{t "Subscribe" }}</button><button type="button" id="compose_not_subscribed_close" class="close">&times;</button>
{{/if}}
</div>
</div>