fix: image block issues (#3637)

frontend/appflowy_flutter/integration_test/document/document_test_runner.dart

@@ -9,6 +9,7 @@ import 'document_option_action_test.dart' as document_option_action_test;
 import 'document_text_direction_test.dart' as document_text_direction_test;
 import 'document_with_cover_image_test.dart' as document_with_cover_image_test;
 import 'document_with_database_test.dart' as document_with_database_test;
+import 'document_with_image_block_test.dart' as document_with_image_block_test;
 import 'document_with_inline_math_equation_test.dart'
     as document_with_inline_math_equation_test;
 import 'document_with_inline_page_test.dart' as document_with_inline_page_test;
@@ -33,4 +34,5 @@ void startTesting() {
   document_alignment_test.main();
   document_text_direction_test.main();
   document_option_action_test.main();
+  document_with_image_block_test.main();
 }

frontend/appflowy_flutter/integration_test/document/document_with_image_block_test.dart

@@ -0,0 +1,147 @@
+import 'dart:io';
+
+import 'package:appflowy/generated/locale_keys.g.dart';
+import 'package:appflowy/plugins/document/presentation/editor_plugins/image/custom_image_block_component.dart';
+import 'package:appflowy/plugins/document/presentation/editor_plugins/image/embed_image_url_widget.dart';
+import 'package:appflowy/plugins/document/presentation/editor_plugins/image/image_placeholder.dart';
+import 'package:appflowy/plugins/document/presentation/editor_plugins/image/unsplash_image_widget.dart';
+import 'package:appflowy/plugins/document/presentation/editor_plugins/image/upload_image_menu.dart';
+import 'package:appflowy_backend/protobuf/flowy-folder2/protobuf.dart';
+import 'package:appflowy_editor/appflowy_editor.dart' hide UploadImageMenu;
+import 'package:easy_localization/easy_localization.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:integration_test/integration_test.dart';
+import 'package:path/path.dart' as p;
+import 'package:path_provider/path_provider.dart';
+import 'package:run_with_network_images/run_with_network_images.dart';
+
+import '../util/mock/mock_file_picker.dart';
+import '../util/util.dart';
+
+void main() {
+  IntegrationTestWidgetsFlutterBinding.ensureInitialized();
+  TestWidgetsFlutterBinding.ensureInitialized();
+
+  group('image block in document', () {
+    testWidgets('insert an image from local file', (tester) async {
+      await tester.initializeAppFlowy();
+      await tester.tapGoButton();
+
+      // create a new document
+      await tester.createNewPageWithName(
+        name: LocaleKeys.document_plugins_image_addAnImage.tr(),
+        layout: ViewLayoutPB.Document,
+      );
+
+      // tap the first line of the document
+      await tester.editor.tapLineOfEditorAt(0);
+      await tester.editor.showSlashMenu();
+      await tester.editor.tapSlashMenuItemWithName('Image');
+      expect(find.byType(CustomImageBlockComponent), findsOneWidget);
+      expect(find.byType(ImagePlaceholder), findsOneWidget);
+
+      await tester.tapButton(find.byType(ImagePlaceholder));
+      expect(find.byType(UploadImageMenu), findsOneWidget);
+
+      final image = await rootBundle.load('assets/test/images/sample.jpeg');
+      final tempDirectory = await getTemporaryDirectory();
+      final imagePath = p.join(tempDirectory.path, 'sample.jpeg');
+      final file = File(imagePath)
+        ..writeAsBytesSync(image.buffer.asUint8List());
+
+      mockPickFilePaths(
+        paths: [imagePath],
+      );
+
+      await tester.tapButtonWithName(
+        LocaleKeys.document_imageBlock_upload_placeholder.tr(),
+      );
+      await tester.pumpAndSettle();
+      expect(find.byType(ResizableImage), findsOneWidget);
+      final node = tester.editor.getCurrentEditorState().getNodeAtPath([0])!;
+      expect(node.type, ImageBlockKeys.type);
+      expect(node.attributes[ImageBlockKeys.url], isNotEmpty);
+
+      // remove the temp file
+      file.deleteSync();
+    });
+
+    testWidgets('insert an image from network', (tester) async {
+      await tester.initializeAppFlowy();
+      await tester.tapGoButton();
+
+      // create a new document
+      await tester.createNewPageWithName(
+        name: LocaleKeys.document_plugins_image_addAnImage.tr(),
+        layout: ViewLayoutPB.Document,
+      );
+
+      // tap the first line of the document
+      await tester.editor.tapLineOfEditorAt(0);
+      await tester.editor.showSlashMenu();
+      await tester.editor.tapSlashMenuItemWithName('Image');
+      expect(find.byType(CustomImageBlockComponent), findsOneWidget);
+      expect(find.byType(ImagePlaceholder), findsOneWidget);
+
+      await tester.tapButton(find.byType(ImagePlaceholder));
+      expect(find.byType(UploadImageMenu), findsOneWidget);
+
+      await tester.tapButtonWithName(
+        LocaleKeys.document_imageBlock_embedLink_label.tr(),
+      );
+      const url =
+          'https://images.unsplash.com/photo-1469474968028-56623f02e42e?ixlib=rb-4.0.3&q=85&fm=jpg&crop=entropy&cs=srgb&dl=david-marcu-78A265wPiO4-unsplash.jpg&w=640';
+      await tester.enterText(
+        find.descendant(
+          of: find.byType(EmbedImageUrlWidget),
+          matching: find.byType(TextField),
+        ),
+        url,
+      );
+      await tester.tapButton(
+        find.descendant(
+          of: find.byType(EmbedImageUrlWidget),
+          matching: find.text(
+            LocaleKeys.document_imageBlock_embedLink_label.tr(),
+            findRichText: true,
+          ),
+        ),
+      );
+      await tester.pumpAndSettle();
+      expect(find.byType(ResizableImage), findsOneWidget);
+      final node = tester.editor.getCurrentEditorState().getNodeAtPath([0])!;
+      expect(node.type, ImageBlockKeys.type);
+      expect(node.attributes[ImageBlockKeys.url], url);
+    });
+
+    testWidgets('insert an image from unsplash', (tester) async {
+      await runWithNetworkImages(() async {
+        await tester.initializeAppFlowy();
+        await tester.tapGoButton();
+
+        // create a new document
+        await tester.createNewPageWithName(
+          name: LocaleKeys.document_plugins_image_addAnImage.tr(),
+          layout: ViewLayoutPB.Document,
+        );
+
+        // tap the first line of the document
+        await tester.editor.tapLineOfEditorAt(0);
+        await tester.editor.showSlashMenu();
+        await tester.editor.tapSlashMenuItemWithName('Image');
+        expect(find.byType(CustomImageBlockComponent), findsOneWidget);
+        expect(find.byType(ImagePlaceholder), findsOneWidget);
+
+        await tester.tapButton(find.byType(ImagePlaceholder));
+        expect(find.byType(UploadImageMenu), findsOneWidget);
+
+        await tester.tapButtonWithName(
+          'Unsplash',
+        );
+        expect(find.byType(UnsplashImageWidget), findsOneWidget);
+      });
+    });
+  });
+}

frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/custom_image_block_component.dart

@@ -206,11 +206,8 @@ class CustomImageBlockComponentState extends State<CustomImageBlockComponent>
     Position position, {
     bool shiftWithBaseOffset = false,
   }) {
-    if (_renderBox == null) {
+    final rects = getRectsInSelection(Selection.collapsed(position));
-      return null;
+    return rects.firstOrNull;
-    }
-    final size = _renderBox!.size;
-    return Rect.fromLTWH(-size.width / 2.0, 0, size.width, size.height);
   }
 
   @override

frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/image_placeholder.dart

@@ -41,13 +41,24 @@ class _ImagePlaceholderState extends State<ImagePlaceholder> {
       direction: PopoverDirection.bottomWithCenterAligned,
       constraints: const BoxConstraints(
         maxWidth: 540,
-        maxHeight: 260,
+        maxHeight: 360,
         minHeight: 80,
       ),
+      clickHandler: PopoverClickHandler.gestureDetector,
       popupBuilder: (context) {
         return UploadImageMenu(
-          onPickFile: insertLocalImage,
+          onPickFile: (path) {
-          onSubmit: insertNetworkImage,
+            controller.close();
+            WidgetsBinding.instance.addPostFrameCallback((timeStamp) {
+              insertLocalImage(path);
+            });
+          },
+          onSubmit: (url) {
+            controller.close();
+            WidgetsBinding.instance.addPostFrameCallback((timeStamp) {
+              insertNetworkImage(url);
+            });
+          },
         );
       },
       child: DecoratedBox(

frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/unsplash_image_widget.dart

@@ -50,29 +50,39 @@ class _UnsplashImageWidgetState extends State<UnsplashImageWidget> {
   @override
   Widget build(BuildContext context) {
     return Column(
+      mainAxisSize: MainAxisSize.min,
       children: [
-        FlowyTextField(
+        Row(
+          mainAxisSize: MainAxisSize.min,
+          children: [
+            Expanded(
+              child: FlowyTextField(
                 autoFocus: true,
                 hintText: LocaleKeys.document_imageBlock_searchForAnImage.tr(),
-          // textAlign: TextAlign.left,
                 onChanged: (value) => query = value,
-          onEditingComplete: () => setState(() {
+                onEditingComplete: _search,
-            randomPhotos = client.photos
-                .random(
-                  count: 18,
-                  orientation: PhotoOrientation.landscape,
-                  query: query,
-                )
-                .goAndGet();
-          }),
               ),
-        const HSpace(12.0),
+            ),
+            const HSpace(4.0),
+            FlowyButton(
+              useIntrinsicWidth: true,
+              text: FlowyText(
+                LocaleKeys.search_label.tr(),
+              ),
+              onTap: _search,
+            ),
+          ],
+        ),
+        const VSpace(12.0),
         Expanded(
           child: FutureBuilder(
             future: randomPhotos,
             builder: (context, value) {
               final data = value.data;
-              if (!value.hasData || data == null || data.isEmpty) {
+              if (!value.hasData ||
+                  value.connectionState != ConnectionState.done ||
+                  data == null ||
+                  data.isEmpty) {
                 return const CircularProgressIndicator.adaptive();
               }
               return GridView.count(
@@ -97,6 +107,18 @@ class _UnsplashImageWidgetState extends State<UnsplashImageWidget> {
       ],
     );
   }
+
+  void _search() {
+    setState(() {
+      randomPhotos = client.photos
+          .random(
+            count: 18,
+            orientation: PhotoOrientation.landscape,
+            query: query,
+          )
+          .goAndGet();
+    });
+  }
 }
 
 class _UnsplashImage extends StatelessWidget {
@@ -121,6 +143,7 @@ class _UnsplashImage extends StatelessWidget {
               fit: BoxFit.cover,
             ),
           ),
+          const HSpace(2.0),
           FlowyText(
             'by ${photo.name}',
             fontSize: 10.0,

frontend/appflowy_flutter/lib/workspace/presentation/settings/widgets/settings_appearance/brightness_setting.dart

@@ -6,7 +6,6 @@ import 'package:appflowy/workspace/application/appearance.dart';
 import 'package:appflowy_popover/appflowy_popover.dart';
 import 'package:easy_localization/easy_localization.dart';
 import 'package:flowy_infra_ui/flowy_infra_ui.dart';
-import 'package:flowy_infra_ui/widget/flowy_tooltip.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_bloc/flutter_bloc.dart';
 
@@ -18,15 +17,10 @@ class BrightnessSetting extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    return FlowyTooltip.delayed(
+    return ThemeSettingEntryTemplateWidget(
-      margin: const EdgeInsets.only(left: 180),
-      richMessage: themeModeTooltipTextSpan(
-        LocaleKeys.settings_appearance_themeMode_label.tr(),
-      ),
-      child: ThemeSettingEntryTemplateWidget(
       label: LocaleKeys.settings_appearance_themeMode_label.tr(),
-        onResetRequested:
+      hint: hintText,
-            context.read<AppearanceSettingsCubit>().resetThemeMode,
+      onResetRequested: context.read<AppearanceSettingsCubit>().resetThemeMode,
       trailing: [
         ThemeValueDropDown(
           currentValue: _themeModeLabelText(currentThemeMode),
@@ -40,20 +34,11 @@ class BrightnessSetting extends StatelessWidget {
           ),
         ),
       ],
-      ),
     );
   }
 
-  TextSpan themeModeTooltipTextSpan(String hintText) => TextSpan(
+  String get hintText =>
-        children: [
+      '${LocaleKeys.settings_files_change.tr()} ${LocaleKeys.settings_appearance_themeMode_label.tr()} : ${Platform.isMacOS ? '⌘+Shift+L' : 'Ctrl+Shift+L'}';
-          TextSpan(
-            text: "${LocaleKeys.settings_files_change.tr()} $hintText\n",
-          ),
-          TextSpan(
-            text: Platform.isMacOS ? "⌘+Shift+L" : "Ctrl+Shift+L",
-          ),
-        ],
-      );
 
   Widget _themeModeItemButton(
     BuildContext context,

frontend/appflowy_flutter/packages/appflowy_popover/lib/src/popover.dart

@@ -48,6 +48,11 @@ enum PopoverDirection {
   custom,
 }
 
+enum PopoverClickHandler {
+  listener,
+  gestureDetector,
+}
+
 class Popover extends StatefulWidget {
   final PopoverController? controller;
 
@@ -78,11 +83,18 @@ class Popover extends StatefulWidget {
 
   final bool asBarrier;
 
+  /// The widget that will be used to trigger the popover.
+  ///
+  /// Why do we need this?
+  /// Because if the parent widget of the popover is GestureDetector,
+  ///  the conflict won't be resolve by using Listener, we want these two gestures exclusive.
+  final PopoverClickHandler clickHandler;
+
   /// The content area of the popover.
   final Widget child;
 
   const Popover({
-    Key? key,
+    super.key,
     required this.child,
     required this.popupBuilder,
     this.controller,
@@ -97,7 +109,8 @@ class Popover extends StatefulWidget {
     this.onClose,
     this.canClose,
     this.asBarrier = false,
-  }) : super(key: key);
+    this.clickHandler = PopoverClickHandler.listener,
+  });
 
   @override
   State<Popover> createState() => PopoverState();
@@ -203,9 +216,9 @@ class PopoverState extends State<Popover> {
           showOverlay();
         }
       },
-      child: Listener(
+      child: _buildClickHandler(
-        child: widget.child,
+        widget.child,
-        onPointerDown: (_) {
+        () {
           if (widget.triggerActions & PopoverTriggerFlags.click != 0) {
             showOverlay();
           }
@@ -213,6 +226,21 @@ class PopoverState extends State<Popover> {
       ),
     );
   }
+
+  Widget _buildClickHandler(Widget child, VoidCallback handler) {
+    switch (widget.clickHandler) {
+      case PopoverClickHandler.listener:
+        return Listener(
+          onPointerDown: (_) => handler(),
+          child: child,
+        );
+      case PopoverClickHandler.gestureDetector:
+        return GestureDetector(
+          onTap: handler,
+          child: child,
+        );
+    }
+  }
 }
 
 class PopoverContainer extends StatefulWidget {

frontend/appflowy_flutter/packages/flowy_infra_ui/lib/src/flowy_overlay/appflowy_popover.dart

@@ -18,8 +18,15 @@ class AppFlowyPopover extends StatelessWidget {
   final EdgeInsets windowPadding;
   final Decoration? decoration;
 
+  /// The widget that will be used to trigger the popover.
+  ///
+  /// Why do we need this?
+  /// Because if the parent widget of the popover is GestureDetector,
+  ///  the conflict won't be resolve by using Listener, we want these two gestures exclusive.
+  final PopoverClickHandler clickHandler;
+
   const AppFlowyPopover({
-    Key? key,
+    super.key,
     required this.child,
     required this.popupBuilder,
     this.direction = PopoverDirection.rightWithTopAligned,
@@ -34,7 +41,8 @@ class AppFlowyPopover extends StatelessWidget {
     this.margin = const EdgeInsets.all(6),
     this.windowPadding = const EdgeInsets.all(8.0),
     this.decoration,
-  }) : super(key: key);
+    this.clickHandler = PopoverClickHandler.listener,
+  });
 
   @override
   Widget build(BuildContext context) {
@@ -48,6 +56,7 @@ class AppFlowyPopover extends StatelessWidget {
       triggerActions: triggerActions,
       windowPadding: windowPadding,
       offset: offset,
+      clickHandler: clickHandler,
       popupBuilder: (context) {
         final child = popupBuilder(context);
         return _PopoverContainer(

frontend/appflowy_flutter/pubspec.lock

@@ -54,8 +54,8 @@ packages:
     dependency: "direct main"
     description:
       path: "."
-      ref: e996c92
+      ref: af8d96b
-      resolved-ref: e996c9279d873f55a1b6aa919144763a60f83d32
+      resolved-ref: af8d96bc1aab07046f4febdd991e1787c75c6e38
       url: "https://github.com/AppFlowy-IO/appflowy-editor.git"
     source: git
     version: "1.4.3"
@@ -889,6 +889,14 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "1.0.4"
+  mockito:
+    dependency: transitive
+    description:
+      name: mockito
+      sha256: "8b46d7eb40abdda92d62edd01546051f0c27365e65608c284de336dccfef88cc"
+      url: "https://pub.dev"
+    source: hosted
+    version: "5.4.1"
   mocktail:
     dependency: "direct main"
     description:
@@ -1217,6 +1225,14 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "1.0.2"
+  run_with_network_images:
+    dependency: "direct dev"
+    description:
+      name: run_with_network_images
+      sha256: "8bf2de4e5120ab24037eda09596408938aa8f5b09f6afabd49683bd01c7baa36"
+      url: "https://pub.dev"
+    source: hosted
+    version: "0.0.1"
   rxdart:
     dependency: transitive
     description:

frontend/appflowy_flutter/pubspec.yaml

@@ -47,7 +47,7 @@ dependencies:
   appflowy_editor:
     git:
       url: https://github.com/AppFlowy-IO/appflowy-editor.git
-      ref: 'e996c92'
+      ref: 'af8d96b'
   appflowy_popover:
     path: packages/appflowy_popover
 
@@ -132,6 +132,8 @@ dev_dependencies:
 
   plugin_platform_interface: any
   url_launcher_platform_interface: any
+  run_with_network_images: ^0.0.1
+
 
 dependency_overrides:
   http: ^1.0.0