Merge remote-tracking branch 'pr/1506'

Author: gnprice

lib/model/emoji.dart

@@ -115,13 +115,7 @@ mixin EmojiStore {
   ///
   /// See description in the web code:
   ///   https://github.com/zulip/zulip/blob/83a121c7e/web/shared/src/typeahead.ts#L3-L21
-  // Someday this list may start varying rather than being hard-coded,
-  // and then this will become a non-static member on EmojiStore.
-  // For now, though, the fact it's constant is convenient when writing
-  // tests of the logic that uses this data; so we guarantee it in the API.
-  static Iterable<EmojiCandidate> get popularEmojiCandidates {
-    return EmojiStoreImpl._popularCandidates;
-  }
+  Iterable<EmojiCandidate> popularEmojiCandidates();
 
   Iterable<EmojiCandidate> allEmojiCandidates();
 
@@ -218,36 +212,54 @@ class EmojiStoreImpl extends PerAccountStoreBase with EmojiStore {
   /// retrieving the data.
   Map<String, List<String>>? _serverEmojiData;
 
-  static final _popularCandidates = _generatePopularCandidates();
+  List<EmojiCandidate>? _popularCandidates;
 
-  static List<EmojiCandidate> _generatePopularCandidates() {
-    EmojiCandidate candidate(String emojiCode, String emojiUnicode,
-        List<String> names) {
-      final emojiName = names.removeAt(0);
-      assert(emojiUnicode == tryParseEmojiCodeToUnicode(emojiCode));
+  List<EmojiCandidate> _generatePopularCandidates() {
+    EmojiCandidate candidate(String emojiCode, List<String> names) {
+      final [emojiName, ...aliases] = names;
+      final emojiUnicode = tryParseEmojiCodeToUnicode(emojiCode);
+      assert(emojiUnicode != null);
       return EmojiCandidate(emojiType: ReactionType.unicodeEmoji,
-        emojiCode: emojiCode, emojiName: emojiName, aliases: names,
+        emojiCode: emojiCode, emojiName: emojiName, aliases: aliases,
         emojiDisplay: UnicodeEmojiDisplay(
-          emojiName: emojiName, emojiUnicode: emojiUnicode));
+          emojiName: emojiName, emojiUnicode: emojiUnicode!));
     }
-    return [
-      // This list should match web:
-      //   https://github.com/zulip/zulip/blob/83a121c7e/web/shared/src/typeahead.ts#L22-L29
-      candidate('1f44d', '👍', ['+1', 'thumbs_up', 'like']),
-      candidate('1f389', '🎉', ['tada']),
-      candidate('1f642', '🙂', ['smile']),
-      candidate( '2764', '❤', ['heart', 'love', 'love_you']),
-      candidate('1f6e0', '🛠', ['working_on_it', 'hammer_and_wrench', 'tools']),
-      candidate('1f419', '🐙', ['octopus']),
-    ];
+    if (_serverEmojiData == null) return [];
+
+    final result = <EmojiCandidate>[];
+    for (final emojiCode in _popularEmojiCodesList) {
+      final names = _serverEmojiData![emojiCode];
+      if (names == null) continue; // TODO(log)
+      result.add(candidate(emojiCode, names));
+    }
+    return result;
   }
 
-  static final _popularEmojiCodes = (() {
-    assert(_popularCandidates.every((c) =>
-      c.emojiType == ReactionType.unicodeEmoji));
-    return Set.of(_popularCandidates.map((c) => c.emojiCode));
+  @override
+  Iterable<EmojiCandidate> popularEmojiCandidates() {
+    return _popularCandidates ??= _generatePopularCandidates();
+  }
+
+  /// Codes for the popular emoji, in order; all are Unicode emoji.
+  // This list should match web:
+  //   https://github.com/zulip/zulip/blob/9feba0f16/web/shared/src/typeahead.ts#L22-L29
+  static final List<String> _popularEmojiCodesList = (() {
+    String check(String emojiCode, String emojiUnicode) {
+      assert(emojiUnicode == tryParseEmojiCodeToUnicode(emojiCode));
+      return emojiCode;
+    }
+    return [
+      check('1f44d', '👍'),
+      check('1f389', '🎉'),
+      check('1f642', '🙂'),
+      check('2764', '❤'),
+      check('1f6e0', '🛠'),
+      check('1f419', '🐙'),
+    ];
   })();
 
+  static final Set<String> _popularEmojiCodes = Set.of(_popularEmojiCodesList);
+
   static bool _isPopularEmoji(EmojiCandidate candidate) {
     return candidate.emojiType == ReactionType.unicodeEmoji
       && _popularEmojiCodes.contains(candidate.emojiCode);
@@ -307,7 +319,7 @@ class EmojiStoreImpl extends PerAccountStoreBase with EmojiStore {
 
     // Include the "popular" emoji, in their canonical order
     // relative to each other.
-    results.addAll(_popularCandidates);
+    results.addAll(popularEmojiCandidates());
 
     final namesOverridden = {
       for (final emoji in activeRealmEmoji) emoji.name,
@@ -366,6 +378,7 @@ class EmojiStoreImpl extends PerAccountStoreBase with EmojiStore {
   @override
   void setServerEmojiData(ServerEmojiData data) {
     _serverEmojiData = data.codeToNames;
+    _popularCandidates = null;
     _allEmojiCandidates = null;
   }
 

lib/model/store.dart

@@ -612,6 +612,9 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
     notifyListeners();
   }
 
+  @override
+  Iterable<EmojiCandidate> popularEmojiCandidates() => _emoji.popularEmojiCandidates();
+
   @override
   Iterable<EmojiCandidate> allEmojiCandidates() => _emoji.allEmojiCandidates();
 

lib/widgets/action_sheet.dart

@@ -575,6 +575,8 @@ void showMessageActionSheet({required BuildContext context, required Message mes
   final pageContext = PageRoot.contextOf(context);
   final store = PerAccountStoreWidget.of(pageContext);
 
+  final popularEmojiLoaded = store.popularEmojiCandidates().isNotEmpty;
+
   // The UI that's conditioned on this won't live-update during this appearance
   // of the action sheet (we avoid calling composeBoxControllerOf in a build
   // method; see its doc).
@@ -588,7 +590,8 @@ void showMessageActionSheet({required BuildContext context, required Message mes
   final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead;
 
   final optionButtons = [
-    ReactionButtons(message: message, pageContext: pageContext),
+    if (popularEmojiLoaded)
+      ReactionButtons(message: message, pageContext: pageContext),
     StarButton(message: message, pageContext: pageContext),
     if (isComposeBoxOffered)
       QuoteAndReplyButton(message: message, pageContext: pageContext),
@@ -718,11 +721,18 @@ class ReactionButtons extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    assert(EmojiStore.popularEmojiCandidates.every(
+    final store = PerAccountStoreWidget.of(pageContext);
+    final popularEmojiCandidates = store.popularEmojiCandidates();
+    assert(popularEmojiCandidates.every(
       (emoji) => emoji.emojiType == ReactionType.unicodeEmoji));
+    // (if this is empty, the widget isn't built in the first place)
+    assert(popularEmojiCandidates.isNotEmpty);
+    // UI not designed to handle more than 6 popular emoji.
+    // (We might have fewer if ServerEmojiData is lacking expected data,
+    // but that looks fine in manual testing, even when there's just one.)
+    assert(popularEmojiCandidates.length <= 6);
 
     final zulipLocalizations = ZulipLocalizations.of(context);
-    final store = PerAccountStoreWidget.of(pageContext);
     final designVariables = DesignVariables.of(context);
 
     bool hasSelfVote(EmojiCandidate emoji) {
@@ -738,7 +748,7 @@ class ReactionButtons extends StatelessWidget {
         color: designVariables.contextMenuItemBg.withFadedAlpha(0.12)),
       child: Row(children: [
         Flexible(child: Row(spacing: 1, children: List.unmodifiable(
-          EmojiStore.popularEmojiCandidates.mapIndexed((index, emoji) =>
+          popularEmojiCandidates.mapIndexed((index, emoji) =>
             _buildButton(
               context: context,
               emoji: emoji,

test/api/route/realm_test.dart

@@ -22,7 +22,7 @@ void main() {
     }
 
     final fakeResult = ServerEmojiData(codeToNames: {
-      '1f642': ['smile'],
+      '1f642': ['slight_smile'],
       '1f34a': ['orange', 'tangerine', 'mandarin'],
     });
 

test/example_data.dart

@@ -130,6 +130,42 @@ GetServerSettingsResult serverSettings({
   );
 }
 
+ServerEmojiData serverEmojiDataPopular = ServerEmojiData(codeToNames: {
+  '1f44d': ['+1', 'thumbs_up', 'like'],
+  '1f389': ['tada'],
+  '1f642': ['slight_smile'],
+  '2764': ['heart', 'love', 'love_you'],
+  '1f6e0': ['working_on_it', 'hammer_and_wrench', 'tools'],
+  '1f419': ['octopus'],
+});
+
+ServerEmojiData serverEmojiDataPopularPlus(ServerEmojiData data) {
+  final a = serverEmojiDataPopular;
+  final b = data;
+  final result = ServerEmojiData(
+    codeToNames: {...a.codeToNames, ...b.codeToNames},
+  );
+  assert(
+    result.codeToNames.length == a.codeToNames.length + b.codeToNames.length,
+    'eg.serverEmojiDataPopularPlus called with data that collides with eg.serverEmojiDataPopular',
+  );
+  return result;
+}
+
+/// Like [serverEmojiDataPopular], but with the legacy '1f642': ['smile']
+/// instead of '1f642': ['slight_smile']; see zulip/zulip@9feba0f16f.
+///
+/// zulip/zulip@9feba0f16f is a Server 11 commit.
+// TODO(server-11) can drop this
+ServerEmojiData serverEmojiDataPopularLegacy = ServerEmojiData(codeToNames: {
+  '1f44d': ['+1', 'thumbs_up', 'like'],
+  '1f389': ['tada'],
+  '1f642': ['smile'],
+  '2764': ['heart', 'love', 'love_you'],
+  '1f6e0': ['working_on_it', 'hammer_and_wrench', 'tools'],
+  '1f419': ['octopus'],
+});
+
 RealmEmojiItem realmEmojiItem({
   required String emojiCode,
   required String emojiName,