From e7311cdf5d5da9d061f8e4acbb3740c978771c60 Mon Sep 17 00:00:00 2001 From: Harshit Bansal Date: Sat, 25 Aug 2018 12:48:49 +0530 Subject: [PATCH] emoji: Bring back the feature of changing emojisets. This is largely inspired by requests from people not liking the Google's new emojiset. A lot of people were requesting to revert back to old blobs emojiset so we are re-enabling this feature after making relevant infrastructure changes for supporting google's old blob emojiset and re-adding support for twitter emojiset. Fixes: #10158. --- static/js/settings.js | 1 - static/js/settings_display.js | 49 ++++++++++++++----- .../settings/display-settings.handlebars | 7 --- version.py | 2 +- .../0189_userprofile_add_some_emojisets.py | 20 ++++++++ zerver/models.py | 14 ++++-- zerver/tests/test_settings.py | 4 +- zerver/tests/test_signup.py | 2 +- zerver/tests/test_users.py | 2 +- 9 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 zerver/migrations/0189_userprofile_add_some_emojisets.py diff --git a/static/js/settings.js b/static/js/settings.js index 54845ae85e..8772a9bc4c 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -113,7 +113,6 @@ function setup_settings_label() { night_mode: i18n.t("Night mode"), starred_message_counts: i18n.t("Show counts for starred messages"), twenty_four_hour_time: i18n.t("24-hour time (17:00 instead of 5:00 PM)"), - translate_emoji_to_text: i18n.t("View emoji as text (see :smile: when others write 😃)"), translate_emoticons: i18n.t("Convert emoticons before sending (:) becomes 😃)"), }; } diff --git a/static/js/settings_display.js b/static/js/settings_display.js index 46220d0ba0..22abbd60e3 100644 --- a/static/js/settings_display.js +++ b/static/js/settings_display.js @@ -34,8 +34,7 @@ exports.set_up = function () { $("#user_timezone").val(page_params.timezone); - // $(".emojiset_choice[value=" + page_params.emojiset + "]").prop("checked", true); - $("#translate_emoji_to_text").prop('checked', page_params.emojiset === "text"); + $(".emojiset_choice[value=" + page_params.emojiset + "]").prop("checked", true); $("#default_language_modal [data-dismiss]").click(function () { overlays.close_modal('default_language_modal'); @@ -115,16 +114,22 @@ exports.set_up = function () { data.timezone = JSON.stringify(timezone); change_display_setting(data, '#time-settings-status'); }); - - $("#translate_emoji_to_text").change(function () { + $(".emojiset_choice").click(function () { + var emojiset = $(this).val(); var data = {}; - var is_checked = $("#translate_emoji_to_text").is(":checked"); - if (is_checked) { - data.emojiset = JSON.stringify("text"); - } else { - data.emojiset = JSON.stringify("google"); - } - change_display_setting(data, '#emoji-settings-status'); + data.emojiset = JSON.stringify(emojiset); + var spinner = $("#emoji-settings-status").expectOne(); + loading.make_indicator(spinner, {text: settings_ui.strings.saving }); + + channel.patch({ + url: '/json/settings/display', + data: data, + success: function () { + }, + error: function (xhr) { + ui_report.error(settings_ui.strings.failure, xhr, $('#emoji-settings-status').expectOne()); + }, + }); }); $("#translate_emoticons").change(function () { @@ -136,8 +141,25 @@ exports.set_up = function () { }; exports.report_emojiset_change = function () { - // This function still has full support for multiple emojiset options. + // TODO: Clean up how this works so we can use + // change_display_setting. The challenge is that we don't want to + // report success before the server_events request returns that + // causes the actual sprite sheet to change. The current + // implementation is wrong, though, in that it displays the UI + // update in all active browser windows. + function emoji_success() { + if ($("#emoji-settings-status").length) { + loading.destroy_indicator($("#emojiset_spinner")); + $("#emojiset_select").val(page_params.emojiset); + ui_report.success(i18n.t("Emojiset changed successfully!"), + $('#emoji-settings-status').expectOne()); + var spinner = $("#emoji-settings-status").expectOne(); + settings_ui.display_checkmark(spinner); + } + } + if (page_params.emojiset === 'text') { + emoji_success(); return; } @@ -145,6 +167,7 @@ exports.report_emojiset_change = function () { sprite.onload = function () { var sprite_css_href = "/static/generated/emoji/" + page_params.emojiset + "-sprite.css"; $("#emoji-spritesheet").attr('href', sprite_css_href); + emoji_success(); }; sprite.src = "/static/generated/emoji/sheet-" + page_params.emojiset + "-64.png"; }; @@ -153,9 +176,9 @@ exports.update_page = function () { $("#twenty_four_hour_time").prop('checked', page_params.twenty_four_hour_time); $("#left_side_userlist").prop('checked', page_params.left_side_userlist); $("#default_language_name").text(page_params.default_language_name); - $("#translate_emoji_to_text").prop('checked', page_params.emojiset === "text"); $("#translate_emoticons").prop('checked', page_params.translate_emoticons); $("#night_mode").prop('checked', page_params.night_mode); + // TODO: Set emojiset selector here. // Longer term, we'll want to automate this function }; diff --git a/static/templates/settings/display-settings.handlebars b/static/templates/settings/display-settings.handlebars index 70cb673840..34b4837be9 100644 --- a/static/templates/settings/display-settings.handlebars +++ b/static/templates/settings/display-settings.handlebars @@ -82,7 +82,6 @@

Emoji settings

- {{#if false}}
{{#each page_params.emojiset_choices }} @@ -103,12 +102,6 @@ {{/each}}
- {{/if}} - - {{partial "settings_checkbox" - "setting_name" "translate_emoji_to_text" - "is_checked" false - "label" settings_label.translate_emoji_to_text}} {{partial "settings_checkbox" "setting_name" "translate_emoticons" diff --git a/version.py b/version.py index 3b38f0eb3f..f7973d0be5 100644 --- a/version.py +++ b/version.py @@ -8,4 +8,4 @@ ZULIP_VERSION = "1.9.0-rc1+git" # Typically, adding a dependency only requires a minor version bump, and # removing a dependency requires a major version bump. -PROVISION_VERSION = '26.5' +PROVISION_VERSION = '26.6' diff --git a/zerver/migrations/0189_userprofile_add_some_emojisets.py b/zerver/migrations/0189_userprofile_add_some_emojisets.py new file mode 100644 index 0000000000..117aeff7b7 --- /dev/null +++ b/zerver/migrations/0189_userprofile_add_some_emojisets.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.14 on 2018-08-28 19:01 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0188_userprofile_enable_login_emails'), + ] + + operations = [ + migrations.AlterField( + model_name='userprofile', + name='emojiset', + field=models.CharField(choices=[('google', 'Google modern'), ('google-blob', 'Google classic'), ('twitter', 'Twitter'), ('text', 'Plain text')], default='google-blob', max_length=20), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 7c44d80511..82c93e086c 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -784,11 +784,15 @@ class UserProfile(AbstractBaseUser, PermissionsMixin): timezone = models.CharField(max_length=40, default=u'') # type: str # Emojisets - GOOGLE_EMOJISET = u'google' - TEXT_EMOJISET = u'text' - EMOJISET_CHOICES = ((GOOGLE_EMOJISET, "Google"), - (TEXT_EMOJISET, "Plain text")) - emojiset = models.CharField(default=GOOGLE_EMOJISET, choices=EMOJISET_CHOICES, max_length=20) # type: str + GOOGLE_EMOJISET = 'google' + GOOGLE_BLOB_EMOJISET = 'google-blob' + TEXT_EMOJISET = 'text' + TWITTER_EMOJISET = 'twitter' + EMOJISET_CHOICES = ((GOOGLE_EMOJISET, "Google modern"), + (GOOGLE_BLOB_EMOJISET, "Google classic"), + (TWITTER_EMOJISET, "Twitter"), + (TEXT_EMOJISET, "Plain text")) + emojiset = models.CharField(default=GOOGLE_BLOB_EMOJISET, choices=EMOJISET_CHOICES, max_length=20) # type: str AVATAR_FROM_GRAVATAR = u'G' AVATAR_FROM_USER = u'U' diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index 956e208e60..ada2ea8f81 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -270,8 +270,8 @@ class ChangeSettingsTest(ZulipTestCase): def test_emojiset(self) -> None: """Test banned emojisets are not accepted.""" - banned_emojisets = ['apple', 'emojione', 'twitter'] - valid_emojisets = ['google', 'text'] + banned_emojisets = ['apple', 'emojione'] + valid_emojisets = ['google', 'google-blob', 'text', 'twitter'] for emojiset in banned_emojisets: result = self.do_change_emojiset(emojiset) diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index bf5eb6e3a6..4f8fa1b9d7 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -2170,7 +2170,7 @@ class UserSignUpTest(ZulipTestCase): hamlet = get_user(self.example_email("hamlet"), realm) self.assertEqual(hamlet.left_side_userlist, False) self.assertEqual(hamlet.default_language, "en") - self.assertEqual(hamlet.emojiset, "google") + self.assertEqual(hamlet.emojiset, "google-blob") self.assertEqual(hamlet.high_contrast_mode, False) self.assertEqual(hamlet.enable_stream_sounds, False) self.assertEqual(hamlet.enter_sends, False) diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 190e806ac1..36fd4c29d0 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -496,7 +496,7 @@ class UserProfileTest(ZulipTestCase): self.assertEqual(iago.emojiset, "apple") self.assertEqual(cordelia.emojiset, "apple") - self.assertEqual(hamlet.emojiset, "google") + self.assertEqual(hamlet.emojiset, "google-blob") self.assertEqual(iago.timezone, "America/Phoenix") self.assertEqual(cordelia.timezone, "America/Phoenix")