paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-17 04:12:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

compose: Migrate compose-all-everyone to use compose_banner.

Browse Source 
This is part several updates for #22524.

Testing note: I removed the test test_compose_all_everyone_confirm_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-20 12:07:32 -07:00
committed by Tim Abbott
parent ab5d088188
commit 8ced075643
12 changed files with 63 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
frontend_tests/node_tests/compose.js
View File

@@ -561,7 +561,7 @@ test_ui("trigger_submit_compose_form", ({override, override_rewire}) => {
assert.ok(compose_finish_checked); assert.ok(compose_finish_checked);
}); });
test_ui("on_events", ({override, override_rewire}) => { test_ui("on_events", ({override}) => {
initialize_handlers({override}); initialize_handlers({override});
override(rendered_markdown, "update_elements", () => {}); override(rendered_markdown, "update_elements", () => {});
@@ -595,34 +595,6 @@ test_ui("on_events", ({override, override_rewire}) => {
return helper; return helper;
} }
(function test_compose_all_everyone_confirm_clicked() {
const handler = $("#compose-all-everyone").get_on_handler(
"click",
".compose-all-everyone-confirm",
);
const helper = setup_parents_and_mock_remove(
"compose-all-everyone",
"compose-all-everyone",
".compose-all-everyone",
);
$("#compose-all-everyone").show();
$("#compose-send-status").show();
let compose_finish_checked = false;
override_rewire(compose, "finish", () => {
compose_finish_checked = true;
});
handler(helper.event);
assert.ok(helper.container_was_removed());
assert.ok(compose_finish_checked);
assert.ok(!$("#compose-all-everyone").visible());
assert.ok(!$("#compose-send-status").visible());
})();
(function test_compose_not_subscribed_clicked() { (function test_compose_not_subscribed_clicked() {
const handler = $("#compose-send-status").get_on_handler("click", ".sub_unsub_button"); const handler = $("#compose-send-status").get_on_handler("click", ".sub_unsub_button");
const subscription = { const subscription = {

16
frontend_tests/node_tests/compose_validate.js
View File

@@ -356,6 +356,7 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
// we are separating it up in different test. Though their relative position // we are separating it up in different test. Though their relative position
// of execution should not be changed. // of execution should not be changed.
mock_banners(); mock_banners();
$("#compose_banners .wildcard_warning").length = 0;
page_params.user_id = me.user_id; page_params.user_id = me.user_id;
page_params.realm_mandatory_topics = false; page_params.realm_mandatory_topics = false;
const sub = { const sub = {
@@ -373,22 +374,19 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
assert.equal(stream_id, 101); assert.equal(stream_id, 101);
return 16; return 16;
}; };
mock_template("compose_all_everyone.hbs", false, (data) => { let wildcard_warning_rendered = false;
assert.equal(data.count, 16); $("#compose_banner_area .wildcard_warning").length = 0;
return "compose_all_everyone_stub"; mock_template("compose_banner/wildcard_warning.hbs", false, (data) => {
wildcard_warning_rendered = true;
assert.equal(data.subscriber_count, 16);
}); });
let compose_content;
$("#compose-all-everyone").append = (data) => {
compose_content = data;
};
override_rewire(compose_validate, "wildcard_mention_allowed", () => true); override_rewire(compose_validate, "wildcard_mention_allowed", () => true);
compose_state.message_content("Hey @**all**"); compose_state.message_content("Hey @**all**");
assert.ok(!compose_validate.validate()); assert.ok(!compose_validate.validate());
assert.equal($("#compose-send-button").prop("disabled"), false); assert.equal($("#compose-send-button").prop("disabled"), false);
assert.ok(!$("#compose-send-status").visible()); assert.ok(!$("#compose-send-status").visible());
assert.equal(compose_content, "compose_all_everyone_stub"); assert.ok(wildcard_warning_rendered);
assert.ok($("#compose-all-everyone").visible());
let wildcards_not_allowed_rendered = false; let wildcards_not_allowed_rendered = false;
mock_template("compose_banner/compose_banner.hbs", false, (data) => { mock_template("compose_banner/compose_banner.hbs", false, (data) => {

11
frontend_tests/puppeteer_tests/mention.ts
View File

@@ -29,14 +29,9 @@ async function test_mention(page: Page): Promise<void> {
assert.ok(stream_size > threshold); assert.ok(stream_size > threshold);
await page.click("#compose-send-button"); await page.click("#compose-send-button");
await page.waitForSelector( await page.waitForSelector("#compose_banners .wildcard_warning", {visible: true});
`xpath///*[${common.has_class_x( await page.click("#compose_banners .wildcard_warning .compose_banner_action_button");
"compose-all-everyone-msg", await page.waitForSelector(".wildcard_warning", {hidden: true});
)} and contains(text(), "Are you sure you want to mention all")]`,
);
await page.click(".compose-all-everyone-confirm");
await page.waitForSelector(".compose-all-everyone-msg", {hidden: true});
await page.waitForSelector("#compose-send-status", {hidden: true});
await common.check_messages_sent(page, "zhome", [["Verona > Test mention all", ["@all"]]]); await common.check_messages_sent(page, "zhome", [["Verona > Test mention all", ["@all"]]]);
} }

18
static/js/compose.js
View File

@@ -450,14 +450,16 @@ export function initialize() {
upload.feature_check($("#compose .compose_upload_file")); upload.feature_check($("#compose .compose_upload_file"));
$("#compose-all-everyone").on("click", ".compose-all-everyone-confirm", (event) => { $("#compose_banners").on(
event.preventDefault(); "click",
`.${compose_error.CLASSNAMES.wildcard_warning} .compose_banner_action_button`,
$(event.target).parents(".compose-all-everyone").remove(); (event) => {
compose_validate.set_user_acknowledged_all_everyone_flag(true); event.preventDefault();
compose_validate.clear_all_everyone_warnings(); compose_validate.clear_wildcard_warnings();
finish(); compose_validate.set_user_acknowledged_wildcard_flag(true);
}); finish();
},
);
$("#compose-send-status").on("click", ".sub_unsub_button", (event) => { $("#compose-send-status").on("click", ".sub_unsub_button", (event) => {
event.preventDefault(); event.preventDefault();

4
static/js/compose_actions.js
View File

@@ -109,9 +109,9 @@ function clear_box() {
// TODO: Better encapsulate at-mention warnings. // TODO: Better encapsulate at-mention warnings.
compose_validate.clear_topic_resolved_warning(); compose_validate.clear_topic_resolved_warning();
compose_validate.clear_all_everyone_warnings(); compose_validate.clear_wildcard_warnings();
compose.clear_private_stream_alert(); compose.clear_private_stream_alert();
compose_validate.set_user_acknowledged_all_everyone_flag(undefined); compose_validate.set_user_acknowledged_wildcard_flag(undefined);
compose.clear_preview_area(); compose.clear_preview_area();
clear_textarea(); clear_textarea();

1
static/js/compose_error.ts
View File

@@ -13,6 +13,7 @@ export const CLASSNAMES = {
// warnings // warnings
topic_resolved: "topic_resolved", topic_resolved: "topic_resolved",
recipient_not_subscribed: "recipient_not_subscribed", recipient_not_subscribed: "recipient_not_subscribed",
wildcard_warning: "wildcard_warning",
// errors // errors
empty_message: "empty_message", empty_message: "empty_message",
wildcards_not_allowed: "wildcards_not_allowed", wildcards_not_allowed: "wildcards_not_allowed",

55
static/js/compose_validate.js
View File

@@ -1,9 +1,9 @@
import $ from "jquery"; import $ from "jquery";
import * as resolved_topic from "../shared/js/resolved_topic"; import * as resolved_topic from "../shared/js/resolved_topic";
import render_compose_all_everyone from "../templates/compose_all_everyone.hbs";
import render_compose_banner from "../templates/compose_banner/compose_banner.hbs"; 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_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_not_subscribed from "../templates/compose_not_subscribed.hbs";
import render_compose_private_stream_alert from "../templates/compose_private_stream_alert.hbs"; import render_compose_private_stream_alert from "../templates/compose_private_stream_alert.hbs";
@@ -22,7 +22,7 @@ import * as stream_data from "./stream_data";
import {user_settings} from "./user_settings"; import {user_settings} from "./user_settings";
import * as util from "./util"; import * as util from "./util";
let user_acknowledged_all_everyone = false; let user_acknowledged_wildcard = false;
let wildcard_mention; let wildcard_mention;
export let wildcard_mention_large_stream_threshold = 15; export let wildcard_mention_large_stream_threshold = 15;
@@ -226,32 +226,36 @@ export function warn_if_topic_resolved(topic_changed) {
} }
} }
function show_all_everyone_warnings(stream_id) { function show_wildcard_warnings(stream_id) {
const stream_count = peer_data.get_subscriber_count(stream_id) || 0; const subscriber_count = peer_data.get_subscriber_count(stream_id) || 0;
const all_everyone_template = render_compose_all_everyone({ const $compose_banner_area = $("#compose_banners");
count: stream_count, const classname = compose_error.CLASSNAMES.wildcard_warning;
mention: wildcard_mention, const $wildcard_template = render_wildcard_warning({
banner_type: compose_error.WARNING,
subscriber_count,
stream_name: compose_state.stream_name(),
wildcard_mention,
button_text: $t({defaultMessage: "Yes, send"}),
hide_close_button: true,
classname,
}); });
const $error_area_all_everyone = $("#compose-all-everyone");
// only show one error for any number of @all or @everyone mentions // only show one error for any number of @all or @everyone mentions
if (!$error_area_all_everyone.is(":visible")) { if ($(`#compose_banners .${classname}`).length === 0) {
$error_area_all_everyone.append(all_everyone_template); $compose_banner_area.append($wildcard_template);
} }
$error_area_all_everyone.show(); user_acknowledged_wildcard = false;
user_acknowledged_all_everyone = false;
} }
export function clear_all_everyone_warnings() { export function clear_wildcard_warnings() {
$("#compose-all-everyone").hide(); const classname = compose_error.CLASSNAMES.wildcard_warning;
$("#compose-all-everyone").empty(); $(`#compose_banners .${classname}`).remove();
$("#compose-send-status").hide();
} }
export function set_user_acknowledged_all_everyone_flag(value) { export function set_user_acknowledged_wildcard_flag(value) {
user_acknowledged_all_everyone = value; user_acknowledged_wildcard = value;
} }
export function get_invalid_recipient_emails() { export function get_invalid_recipient_emails() {
@@ -346,11 +350,11 @@ export function set_wildcard_mention_large_stream_threshold(value) {
} }
function validate_stream_message_mentions(stream_id) { function validate_stream_message_mentions(stream_id) {
const stream_count = peer_data.get_subscriber_count(stream_id) || 0; const subscriber_count = peer_data.get_subscriber_count(stream_id) || 0;
// If the user is attempting to do a wildcard mention in a large // If the user is attempting to do a wildcard mention in a large
// stream, check if they permission to do so. // stream, check if they permission to do so.
if (wildcard_mention !== null && stream_count > wildcard_mention_large_stream_threshold) { if (wildcard_mention !== null && subscriber_count > wildcard_mention_large_stream_threshold) {
if (!wildcard_mention_allowed()) { if (!wildcard_mention_allowed()) {
compose_error.show_error_message( compose_error.show_error_message(
$t({ $t({
@@ -362,12 +366,9 @@ function validate_stream_message_mentions(stream_id) {
return false; return false;
} }
if ( if (user_acknowledged_wildcard === undefined || user_acknowledged_wildcard === false) {
user_acknowledged_all_everyone === undefined ||
user_acknowledged_all_everyone === false
) {
// user has not seen a warning message yet if undefined // user has not seen a warning message yet if undefined
show_all_everyone_warnings(stream_id); show_wildcard_warnings(stream_id);
$("#compose-send-button").prop("disabled", false); $("#compose-send-button").prop("disabled", false);
compose_ui.hide_compose_spinner(); compose_ui.hide_compose_spinner();
@@ -375,10 +376,10 @@ function validate_stream_message_mentions(stream_id) {
} }
} else { } else {
// the message no longer contains @all or @everyone // the message no longer contains @all or @everyone
clear_all_everyone_warnings(); clear_wildcard_warnings();
} }
// at this point, the user has either acknowledged the warning or removed @all / @everyone // at this point, the user has either acknowledged the warning or removed @all / @everyone
user_acknowledged_all_everyone = undefined; user_acknowledged_wildcard = undefined;
return true; return true;
} }

2
static/styles/compose.css
View File

@@ -382,7 +382,6 @@
} }
.compose_private_stream_alert, .compose_private_stream_alert,
.compose-all-everyone,
.compose_not_subscribed { .compose_not_subscribed {
padding: 4px 0; padding: 4px 0;
display: flex; display: flex;
@@ -404,7 +403,6 @@
width: 10px; width: 10px;
} }
.compose-all-everyone-controls,
.compose_private_stream_alert_controls { .compose_private_stream_alert_controls {
float: right; float: right;
position: relative; position: relative;

1
static/templates/compose.hbs
View File

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

14
static/templates/compose_all_everyone.hbs
View File

@@ -1,14 +0,0 @@
<div class="compose-all-everyone">
<span class="compose-all-everyone-msg">
{{#tr}}
Are you sure you want to mention all <strong>{count}</strong> people in this stream?
<br />
This will send email and mobile push notifications to most of those <strong>{count}</strong> users.
<br />
If you don't want to do that, please edit your message to remove the <strong>@{mention}</strong> mention.
{{/tr}}
</span>
<span class="compose-all-everyone-controls">
<button type="button" class="btn btn-warning compose-all-everyone-confirm">{{t "Yes, send" }}</button>
</span>
</div>

4
static/templates/compose_banner/compose_banner.hbs
View File

@@ -11,5 +11,9 @@
{{#if button_text}} {{#if button_text}}
<button class="compose_banner_action_button" >{{button_text}}</button> <button class="compose_banner_action_button" >{{button_text}}</button>
{{/if}} {{/if}}
{{#if hide_close_button}}
{{!-- hide_close_button is null by default, and false if explicitly set as false. --}}
{{else}}
<a role="button" class="zulip-icon zulip-icon-close compose_banner_close_button"></a> <a role="button" class="zulip-icon zulip-icon-close compose_banner_close_button"></a>
{{/if}}
</div> </div>

7
static/templates/compose_banner/wildcard_warning.hbs Normal file
View File

@@ -0,0 +1,7 @@
{{#> compose_banner }}
<p>
{{#tr}}
Are you sure you want to send @-mention notifications to the <strong>{subscriber_count}</strong> users subscribed to #{stream_name}? If not, please edit your message to remove the <strong>@{wildcard_mention}</strong> mention.
{{/tr}}
</p>
{{/compose_banner}}