display_settings: Change success/failure feedback interface.

This replaces the cumbersome system we had for giving users feedback
on settings state changes in the display settings UI.

We expect this new system to be what we will attempt to migrate other
settings widgets to match over the coming weeks and months.  It also
provides the opportunity to significant refactor away a lot of the
code duplication in settings_display.js.

Thanks to Brock Whittaker for redoing the styling and improving the
code simplicity.

Fixes #7622.
This commit is contained in:
Balaji2198
2018-02-06 21:33:03 +05:30
committed by Tim Abbott
parent b846ab1000
commit 5aa7098c81
6 changed files with 124 additions and 44 deletions

View File

@@ -263,8 +263,8 @@ casper.waitUntilVisible('#default_language_modal');
casper.thenClick('a[data-code="zh-hans"]');
casper.waitUntilVisible('#display-settings-status', function () {
casper.test.assertSelectorHasText('#display-settings-status', '简体中文 is now the default language');
casper.waitUntilVisible('#language-settings-status a', function () {
casper.test.assertSelectorHasText('#language-settings-status', 'Saved. Please reload for the change to take effect.');
casper.test.info("Reloading the page.");
casper.reload();
});
@@ -301,8 +301,8 @@ casper.thenClick('a[data-code="en"]');
/*
* Changing the language back to English so that subsequent tests pass.
*/
casper.waitUntilVisible('#display-settings-status', function () {
casper.test.assertSelectorHasText('#display-settings-status', 'English ist die neue Standardsprache! Du musst das Fenster neu laden um die Änderungen anzuwenden');
casper.waitUntilVisible('#language-settings-status a', function () {
casper.test.assertSelectorHasText('#language-settings-status', 'Saved. Please reload for the change to take effect.');
});
casper.thenOpen("http://zulip.zulipdev.com:9981/");

View File

@@ -46,7 +46,7 @@ exports.make_indicator = function (outer_container, opts) {
// These width calculations are tied to the spinner width and
// margins defined via CSS
container.css({width: 38 + text_width,
height: 38});
height: 0});
outer_container.data("destroying", false);
};

View File

@@ -2,14 +2,23 @@ var settings_display = (function () {
var exports = {};
// this is set down at the top of `exports.set_up` because i18n does not exist
// yet. This object should eventually have a `success` and `failure` translated
// string within it.
var strings = {};
exports.display_checkmark = function ($elem) {
var check_mark = document.createElement("img");
check_mark.src = "/static/images/checkbox-green.svg";
$elem.prepend(check_mark);
$(check_mark).css("width","13px");
};
exports.set_night_mode = function (bool) {
var night_mode = bool;
var data = { night_mode: JSON.stringify(night_mode) };
var context = {
enable_text: data.night_mode === "true" ?
i18n.t("enabled") :
i18n.t("disabled"),
};
var spinner = $("#display-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
@@ -17,18 +26,25 @@ exports.set_night_mode = function (bool) {
success: function () {
page_params.night_mode = night_mode;
if (overlays.settings_open()) {
ui_report.success(i18n.t("Saved."), $('#display-settings-status').expectOne());
ui_report.success(strings.success, $('#display-settings-status').expectOne());
exports.display_checkmark(spinner);
}
},
error: function (xhr) {
if (overlays.settings_open()) {
ui_report.error(i18n.t("Save failed"), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#display-settings-status').expectOne());
}
},
});
};
exports.set_up = function () {
strings = {
success: i18n.t("Saved"),
failure: i18n.t("Save failed"),
saving: i18n.t("Saving"),
};
$("#display-settings-status").hide();
$("#user_timezone").val(page_params.timezone);
@@ -53,16 +69,22 @@ exports.set_up = function () {
var context = {};
context.lang = new_language;
var spinner = $("#language-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
data: data,
success: function () {
ui_report.success(i18n.t("Saved. Please <a>reload</a> for the change to take effect."),
$('#display-settings-status').expectOne());
$('#language-settings-status').expectOne());
exports.display_checkmark(spinner);
$('#language-settings-status').click(function () {
window.location.reload();
});
},
error: function (xhr) {
ui_report.error(i18n.t("Save failed"), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#language-settings-status').expectOne());
},
});
});
@@ -83,15 +105,18 @@ exports.set_up = function () {
} else {
context.enabled_or_disabled = i18n.t('Disabled');
}
var spinner = $("#display-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
data: data,
success: function () {
ui_report.success(i18n.t("Saved."), $('#display-settings-status').expectOne());
ui_report.success(strings.success, $('#display-settings-status').expectOne());
exports.display_checkmark(spinner);
},
error: function (xhr) {
ui_report.error(i18n.t("Save failed"), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#display-settings-status').expectOne());
},
});
});
@@ -110,6 +135,8 @@ exports.set_up = function () {
} else {
context.side = i18n.t('right');
}
var spinner = $("#display-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
@@ -117,9 +144,13 @@ exports.set_up = function () {
success: function () {
ui_report.success(i18n.t("Saved. Please <a>reload</a> for the change to take effect."),
$('#display-settings-status').expectOne());
exports.display_checkmark(spinner);
$('#display-settings-status').click(function () {
window.location.reload();
});
},
error: function (xhr) {
ui_report.error(i18n.t("Save failed"), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#display-settings-status').expectOne());
},
});
});
@@ -134,15 +165,18 @@ exports.set_up = function () {
} else {
context.format = '12';
}
var spinner = $("#time-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
data: data,
success: function () {
ui_report.success(i18n.t("Saved."), $('#display-settings-status').expectOne());
ui_report.success(strings.success, $('#time-settings-status').expectOne());
exports.display_checkmark(spinner);
},
error: function (xhr) {
ui_report.error(i18n.t("Save failed"), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#time-settings-status').expectOne());
},
});
});
@@ -151,15 +185,18 @@ exports.set_up = function () {
var data = {};
var timezone = this.value;
data.timezone = JSON.stringify(timezone);
var spinner = $("#time-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
data: data,
success: function () {
ui_report.success(i18n.t("Saved."), $('#display-settings-status').expectOne());
ui_report.success(strings.success, $('#time-settings-status').expectOne());
exports.display_checkmark(spinner);
},
error: function (xhr) {
ui_report.error(i18n.t("Save failed"), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#time-settings-status').expectOne());
},
});
});
@@ -168,16 +205,16 @@ exports.set_up = function () {
var emojiset = $(this).val();
var data = {};
data.emojiset = JSON.stringify(emojiset);
var spinner = $("#emoji-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
data: data,
success: function () {
var spinner = $("#emojiset_spinner").expectOne();
loading.make_indicator(spinner, {text: 'Changing emojiset.'});
},
error: function (xhr) {
ui_report.error(i18n.t("Error changing emojiset."), xhr, $('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#emoji-settings-status').expectOne());
},
});
});
@@ -186,23 +223,18 @@ exports.set_up = function () {
var data = {};
var setting_value = $("#translate_emoticons").is(":checked");
data.translate_emoticons = JSON.stringify(setting_value);
var context = {};
if (data.translate_emoticons === "true") {
context.new_mode = i18n.t("be");
} else {
context.new_mode = i18n.t("not be");
}
var spinner = $("#emoji-settings-status").expectOne();
loading.make_indicator(spinner, {text: strings.saving });
channel.patch({
url: '/json/settings/display',
data: data,
success: function () {
ui_report.success(i18n.t("Emoticons will now __new_mode__ translated!", context),
$('#display-settings-status').expectOne());
ui_report.success(strings.success, $('#emoji-settings-status').expectOne());
exports.display_checkmark(spinner);
},
error: function (xhr) {
ui_report.error(i18n.t("Error updating emoticon translation setting"), xhr,
$('#display-settings-status').expectOne());
ui_report.error(strings.failure, xhr, $('#emoji-settings-status').expectOne());
},
});
});
@@ -210,11 +242,13 @@ exports.set_up = function () {
exports.report_emojiset_change = function () {
function emoji_success() {
if ($("#display-settings-status").length) {
if ($("#emoji-settings-status").length) {
loading.destroy_indicator($("#emojiset_spinner"));
$("#emojiset_select").val(page_params.emojiset);
ui_report.success(i18n.t("Emojiset changed successfully!"),
$('#display-settings-status').expectOne());
$('#emoji-settings-status').expectOne());
var spinner = $("#emoji-settings-status").expectOne();
exports.display_checkmark(spinner);
}
}

View File

@@ -443,6 +443,50 @@ input[type=checkbox].inline-block {
margin-right: 8px;
}
#settings_page .alert-notification:not(:empty) {
color: hsl(170, 47%, 54%);
display: inline-block !important;
vertical-align: top;
height: auto !important;
width: auto !important;
background: transparent;
border: 1px solid hsl(178, 100%, 40%);
color: hsl(178, 100%, 40%);
padding: 2px 5px;
border-radius: 4px;
margin-top: 14px;
margin-left: 10px;
}
#settings_page .alert-notification.alert-error {
color: hsl(2, 46%, 68%);
border-color: hsl(2, 46%, 68%);
}
#settings_page .alert-notification .loading_indicator_spinner {
width: 13px;
height: 20px;
margin: 0;
}
/* make the spinner green like the text and box. */
#settings_page .alert-notification .loading_indicator_spinner svg path {
fill: hsl(178, 100%, 40%);
}
#settings_page .alert-notification .loading_indicator_text {
margin-top: 0px;
font-size: inherit;
vertical-align: top;
}
#settings_page .alert-notification img {
margin-right: 5px;
}
#notification-settings .notification-reminder {
text-align: left;
}

View File

@@ -1,7 +1,9 @@
<div id="display-settings" class="settings-section" data-name="display-settings">
<div class="alert" id="display-settings-status"></div>
<form class="display-settings-form">
<h3>{{t "Language settings" }}</h3>
<!-- this is inline block so that the alert notification can sit beside
it. If there's not an alert, don't make it inline-block. -->
<h3 class="inline-block">{{t "Language settings" }}</h3>
<div class="alert-notification" id="language-settings-status"></div>
<div class="input-group user-name-section">
<label class="inline-block title">{{t "Default language" }}</label>
@@ -15,7 +17,8 @@
{{ partial "default_language_modal"}}
<h3>{{t "Display settings" }}</h3>
<h3 class="inline-block">{{t "Display settings" }}</h3>
<div class="alert-notification" id="display-settings-status"></div>
<div class="input-group">
<label class="checkbox">
@@ -50,7 +53,8 @@
<label for="left_side_userlist" class="inline-block">{{t "User list on left sidebar in narrow windows" }}</label>
</div>
<h3>{{t "Time settings" }}</h3>
<h3 class="inline-block">{{t "Time settings" }}</h3>
<div class="alert-notification" id="time-settings-status"></div>
<div class="input-group">
<label class="checkbox">
@@ -78,7 +82,8 @@
</select>
</div>
<h3 class="light">Emoji style</h3>
<h3 class="inline-block light">Emoji style</h3>
<div class="alert-notification" id="emoji-settings-status"></div>
<div class="input-group side-padded-container">
<label class="checkbox">
@@ -112,7 +117,6 @@
</label>
{{/each}}
</div>
<div id="emojiset_spinner"></div>
</div>
</form>
</div>

View File

@@ -66,8 +66,6 @@ IGNORED_PHRASES = [
r"images",
r"enabled",
r"disabled",
r"be",
r"^not be$",
# Fragments of larger strings
(r'your subscriptions on your Streams page'),