fix: recent icon doesn't work in space icon (#7133)

AppFlowy-IO · Jan 4, 2025 · 6e55d60 · 6e55d60
1 parent 1e916a0
commit 6e55d60
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 35 deletions.
diff --git a/frontend/appflowy_flutter/integration_test/desktop/cloud/cloud_runner.dart b/frontend/appflowy_flutter/integration_test/desktop/cloud/cloud_runner.dart
@@ -2,6 +2,7 @@ import 'data_migration/data_migration_test_runner.dart'
     as data_migration_test_runner;
 import 'document/document_test_runner.dart' as document_test_runner;
 import 'set_env.dart' as preset_af_cloud_env_test;
+import 'sidebar/sidebar_icon_test.dart' as sidebar_icon_test;
 import 'sidebar/sidebar_move_page_test.dart' as sidebar_move_page_test;
 import 'sidebar/sidebar_rename_untitled_test.dart'
     as sidebar_rename_untitled_test;
@@ -26,4 +27,5 @@ Future<void> main() async {
   // sidebar
   sidebar_move_page_test.main();
   sidebar_rename_untitled_test.main();
+  sidebar_icon_test.main();
 }
diff --git a/frontend/appflowy_flutter/integration_test/desktop/cloud/sidebar/sidebar_icon_test.dart b/frontend/appflowy_flutter/integration_test/desktop/cloud/sidebar/sidebar_icon_test.dart
@@ -0,0 +1,62 @@
+import 'dart:convert';
+
+import 'package:appflowy/env/cloud_env.dart';
+import 'package:appflowy/shared/icon_emoji_picker/flowy_icon_emoji_picker.dart';
+import 'package:appflowy/shared/icon_emoji_picker/icon_picker.dart';
+import 'package:appflowy/shared/icon_emoji_picker/recent_icons.dart';
+import 'package:appflowy/workspace/presentation/home/menu/sidebar/space/sidebar_space_header.dart';
+import 'package:appflowy/workspace/presentation/home/menu/sidebar/space/space_action_type.dart';
+import 'package:appflowy/workspace/presentation/home/menu/sidebar/space/space_more_popup.dart';
+import 'package:flowy_svg/flowy_svg.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:integration_test/integration_test.dart';
+
+import '../../../shared/emoji.dart';
+import '../../../shared/util.dart';
+
+void main() {
+  setUpAll(() {
+    IntegrationTestWidgetsFlutterBinding.ensureInitialized();
+    RecentIcons.enable = false;
+  });
+
+  tearDownAll(() {
+    RecentIcons.enable = true;
+  });
+
+  testWidgets('Change slide bar space icon', (tester) async {
+    await tester.initializeAppFlowy(
+      cloudType: AuthenticatorType.appflowyCloudSelfHost,
+    );
+    await tester.tapGoogleLoginInButton();
+    await tester.expectToSeeHomePageWithGetStartedPage();
+    final emojiIconData = await tester.loadIcon();
+    final firstIcon = IconsData.fromJson(jsonDecode(emojiIconData.emoji));
+
+    await tester.hoverOnWidget(
+      find.byType(SidebarSpaceHeader),
+      onHover: () async {
+        final moreOption = find.byType(SpaceMorePopup);
+        await tester.tapButton(moreOption);
+        expect(find.byType(FlowyIconEmojiPicker), findsNothing);
+        await tester.tapSvgButton(SpaceMoreActionType.changeIcon.leftIconSvg);
+        expect(find.byType(FlowyIconEmojiPicker), findsOneWidget);
+      },
+    );
+
+    final icons = find.byWidgetPredicate(
+      (w) => w is FlowySvg && w.svgString == firstIcon.iconContent,
+    );
+    expect(icons, findsOneWidget);
+    await tester.tapIcon(EmojiIconData.icon(firstIcon));
+
+    final spaceHeader = find.byType(SidebarSpaceHeader);
+    final spaceIcon = find.descendant(
+      of: spaceHeader,
+      matching: find.byWidgetPredicate(
+        (w) => w is FlowySvg && w.svgString == firstIcon.iconContent,
+      ),
+    );
+    expect(spaceIcon, findsOneWidget);
+  });
+}
diff --git a/frontend/appflowy_flutter/integration_test/shared/emoji.dart b/frontend/appflowy_flutter/integration_test/shared/emoji.dart
@@ -31,10 +31,7 @@ extension EmojiTestExtension on WidgetTester {
       matching: find.text(PickerTabType.icon.tr),
     );
     expect(iconTab, findsOneWidget);
-    expect(find.byType(FlowyIconPicker), findsNothing);
-    await tap(iconTab);
-    await pumpAndSettle();
-    expect(find.byType(FlowyIconPicker), findsOneWidget);
+    await tapButton(iconTab);
     final selectedSvg = find.descendant(
       of: find.byType(FlowyIconPicker),
       matching: find.byWidgetPredicate(

diff --git a/frontend/appflowy_flutter/lib/mobile/presentation/base/mobile_view_page.dart b/frontend/appflowy_flutter/lib/mobile/presentation/base/mobile_view_page.dart
@@ -268,8 +268,8 @@ class _MobileViewPageState extends State<MobileViewPage> {
     return Row(
       mainAxisSize: MainAxisSize.min,
       children: [
-        if (icon != null) ...[
-          EmojiIconWidget(
+        if (icon != null && icon.value.isNotEmpty) ...[
+          RawEmojiIconWidget(
             emoji: icon.toEmojiIconData(),
             emojiSize: 15,
           ),

diff --git a/frontend/appflowy_flutter/lib/mobile/presentation/home/mobile_home_page_header.dart b/frontend/appflowy_flutter/lib/mobile/presentation/home/mobile_home_page_header.dart
@@ -5,6 +5,7 @@ import 'package:appflowy/mobile/presentation/bottom_sheet/bottom_sheet.dart';
 import 'package:appflowy/mobile/presentation/home/workspaces/workspace_menu_bottom_sheet.dart';
 import 'package:appflowy/plugins/base/emoji/emoji_picker_screen.dart';
 import 'package:appflowy/shared/icon_emoji_picker/flowy_icon_emoji_picker.dart';
+import 'package:appflowy/shared/icon_emoji_picker/tab.dart';
 import 'package:appflowy/startup/startup.dart';
 import 'package:appflowy/util/built_in_svgs.dart';
 import 'package:appflowy/workspace/application/user/settings_user_bloc.dart';
@@ -234,6 +235,7 @@ class _UserIcon extends StatelessWidget {
             queryParameters: {
               MobileEmojiPickerScreen.pageTitle:
                   LocaleKeys.titleBar_userIcon.tr(),
+              MobileEmojiPickerScreen.selectTabs: [PickerTabType.emoji.name],
             },
           ).toString(),
         );

diff --git a/frontend/appflowy_flutter/lib/mobile/presentation/home/shared/mobile_page_card.dart b/frontend/appflowy_flutter/lib/mobile/presentation/home/shared/mobile_page_card.dart
@@ -178,17 +178,18 @@ class MobileViewPage extends StatelessWidget {
       overflow: TextOverflow.ellipsis,
       text: TextSpan(
         children: [
-          if (icon.isNotEmpty)
+          if (icon.isNotEmpty) ...[
             WidgetSpan(
               child: SizedBox(
                 width: 20,
                 child: EmojiIconWidget(
                   emoji: icon,
-                  emojiSize: 17.0,
+                  emojiSize: 18.0,
                 ),
               ),
             ),
-          if (icon.isNotEmpty) const WidgetSpan(child: HSpace(2.0)),
+            const WidgetSpan(child: HSpace(8.0)),
+          ],
           TextSpan(
             text: name,
             style: Theme.of(context).textTheme.bodyMedium!.copyWith(

diff --git a/...wy_flutter/lib/plugins/document/presentation/editor_plugins/header/emoji_icon_widget.dart b/...wy_flutter/lib/plugins/document/presentation/editor_plugins/header/emoji_icon_widget.dart
@@ -79,13 +79,10 @@ class RawEmojiIconWidget extends StatelessWidget {
     try {
       switch (emoji.type) {
         case FlowyIconType.emoji:
-          return SizedBox(
-            width: emojiSize,
-            child: EmojiText(
-              emoji: emoji.emoji,
-              fontSize: emojiSize,
-              textAlign: TextAlign.center,
-            ),
+          return EmojiText(
+            emoji: emoji.emoji,
+            fontSize: emojiSize,
+            textAlign: TextAlign.center,
           );
         case FlowyIconType.icon:
           final iconData = IconsData.fromJson(jsonDecode(emoji.emoji));

diff --git a/frontend/appflowy_flutter/lib/shared/icon_emoji_picker/recent_icons.dart b/frontend/appflowy_flutter/lib/shared/icon_emoji_picker/recent_icons.dart
@@ -46,6 +46,10 @@ class RecentIcons {
           .map(
             (e) => RecentIcon.fromJson(jsonDecode(e) as Map<String, dynamic>),
           )
+
+          /// skip the data that is already stored locally but has an empty
+          /// groupName to accommodate the issue of destructive data modifications
+          .skipWhile((e) => e.groupName.isEmpty)
           .toList();
     } catch (e) {
       Log.error('RecentIcons getIcons with :$iconList', e);

diff --git a/frontend/appflowy_flutter/lib/startup/tasks/generate_router.dart b/frontend/appflowy_flutter/lib/startup/tasks/generate_router.dart
@@ -29,6 +29,7 @@ import 'package:appflowy/user/application/auth/auth_service.dart';
 import 'package:appflowy/user/presentation/presentation.dart';
 import 'package:appflowy/workspace/presentation/home/desktop_home_screen.dart';
 import 'package:appflowy/workspace/presentation/settings/widgets/feature_flags/mobile_feature_flag_screen.dart';
+import 'package:appflowy_backend/log.dart';
 import 'package:appflowy_backend/protobuf/flowy-database2/protobuf.dart';
 import 'package:flowy_infra/time/duration.dart';
 import 'package:flutter/foundation.dart';
@@ -288,10 +289,15 @@ GoRoute _mobileEmojiPickerPageRoute() {
       final selectedType = state
           .uri.queryParameters[MobileEmojiPickerScreen.iconSelectedType]
           ?.toPickerTabType();
-      final tabs = selectTabs
-          .split('-')
-          .map((e) => PickerTabType.values.byName(e))
-          .toList();
+      List<PickerTabType> tabs = [];
+      try {
+        tabs = selectTabs
+            .split('-')
+            .map((e) => PickerTabType.values.byName(e))
+            .toList();
+      } on ArgumentError catch (e) {
+        Log.error('convert selectTabs to pickerTab error', e);
+      }
       return MaterialExtendedPage(
         child: tabs.isEmpty
             ? MobileEmojiPickerScreen(title: title, selectedType: selectedType)

diff --git a/...lowy_flutter/lib/workspace/presentation/home/menu/sidebar/space/sidebar_space_header.dart b/...lowy_flutter/lib/workspace/presentation/home/menu/sidebar/space/sidebar_space_header.dart
@@ -173,21 +173,22 @@ class _SidebarSpaceHeaderState extends State<SidebarSpaceHeader> {
         await _showRenameDialog();
         break;
       case SpaceMoreActionType.changeIcon:
-        final result = data as EmojiIconData;
-        if (data.type == FlowyIconType.icon) {
-          try {
-            final iconsData = IconsData.fromJson(jsonDecode(result.emoji));
-            context.read<SpaceBloc>().add(
-                  SpaceEvent.changeIcon(
-                    icon: '${iconsData.groupName}/${iconsData.iconName}',
-                    iconColor: iconsData.color,
-                  ),
-                );
-          } on FormatException catch (e) {
-            context
-                .read<SpaceBloc>()
-                .add(const SpaceEvent.changeIcon(icon: ''));
-            Log.warn('SidebarSpaceHeader changeIcon error:$e');
+        if (data is SelectedEmojiIconResult) {
+          if (data.type == FlowyIconType.icon) {
+            try {
+              final iconsData = IconsData.fromJson(jsonDecode(data.emoji));
+              context.read<SpaceBloc>().add(
+                    SpaceEvent.changeIcon(
+                      icon: '${iconsData.groupName}/${iconsData.iconName}',
+                      iconColor: iconsData.color,
+                    ),
+                  );
+            } on FormatException catch (e) {
+              context
+                  .read<SpaceBloc>()
+                  .add(const SpaceEvent.changeIcon(icon: ''));
+              Log.warn('SidebarSpaceHeader changeIcon error:$e');
+            }
           }
         }
         break;