fix: sub page block review fixes (#6491)

* fix: sub page block review fixes

* test: fix tests

frontend/appflowy_flutter/integration_test/desktop/document/document_sub_page_test.dart

@@ -26,11 +26,16 @@ import '../../shared/util.dart';
 // - [x] Redo adding a SubPageBlock (Expect the view to be restored)
 // - [x] Redo delete of a SubPageBlock (Expect the view to be moved to trash again)
 // - [x] Renaming a child view (Expect the view name to be updated in the document)
-// - [x] Deleting a view (to trash) linked to a SubPageBlock shows a hint that the view is in trash (Expect a hint to be shown)
+// - [x] Deleting a view (to trash) linked to a SubPageBlock deleted the SubPageBlock (Expect the SubPageBlock to be deleted)
-// - [x] Deleting a view (in trash) linked to a SubPageBlock deletes the SubPageBlock (Expect the SubPageBlock to be deleted)
 // - [x] Duplicating a SubPageBlock node from Action Menu (Expect a new view is created under current view with same content and name + (copy))
 // - [x] Dragging a SubPageBlock node to a new position in the document (Expect everything to be normal)
 
+/// The defaut page name is empty, if we're looking for a "text" we can look for
+/// [LocaleKeys.menuAppHeader_defaultNewPageName] but it won't work for eg. hoverOnPageName
+/// as it looks at the text provided instead of the actual displayed text.
+///
+const _defaultPageName = "";
+
 void main() {
   IntegrationTestWidgetsFlutterBinding.ensureInitialized();
 
@@ -66,9 +71,9 @@ void main() {
         pageName: 'SubPageBlock',
         layout: ViewLayoutPB.Document,
       );
+      await tester.pumpAndSettle();
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -94,8 +99,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -151,8 +155,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -213,8 +216,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -258,8 +260,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -300,39 +301,6 @@ void main() {
       expect(find.text('Child page (copy)'), findsNothing);
     });
 
-    testWidgets('Undo adding a SubPageBlock', (tester) async {
-      await tester.initializeAppFlowy();
-      await tester.tapAnonymousSignInButton();
-      await tester.createNewPageWithNameUnderParent(name: 'SubPageBlock');
-
-      await tester.insertSubPageFromSlashMenu(true);
-
-      await tester.expandOrCollapsePage(
-        pageName: 'SubPageBlock',
-        layout: ViewLayoutPB.Document,
-      );
-
-      await tester
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
-      await tester.renamePage('Child page');
-      await tester.pumpAndSettle();
-
-      expect(find.text('Child page'), findsNWidgets(2));
-
-      await tester.editor.tapLineOfEditorAt(0);
-
-      // Undo
-      await tester.simulateKeyEvent(
-        LogicalKeyboardKey.keyZ,
-        isControlPressed: Platform.isLinux || Platform.isWindows,
-        isMetaPressed: Platform.isMacOS,
-      );
-      await tester.pumpAndSettle();
-
-      expect(find.byType(SubPageBlockComponent), findsNothing);
-      expect(find.text('Child page'), findsNothing);
-    });
-
     testWidgets('Undo delete of a SubPageBlock', (tester) async {
       await tester.initializeAppFlowy();
       await tester.tapAnonymousSignInButton();
@@ -345,8 +313,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -370,55 +337,6 @@ void main() {
       expect(find.byType(SubPageBlockComponent), findsOneWidget);
     });
 
-    // Redo: undoing adding a subpage block, then redoing to bring it back
-    // -> Add a subpage block
-    // -> Undo
-    // -> Redo
-    testWidgets('Redo adding of a SubPageBlock', (tester) async {
-      await tester.initializeAppFlowy();
-      await tester.tapAnonymousSignInButton();
-      await tester.createNewPageWithNameUnderParent(name: 'SubPageBlock');
-
-      await tester.insertSubPageFromSlashMenu(true);
-
-      await tester.expandOrCollapsePage(
-        pageName: 'SubPageBlock',
-        layout: ViewLayoutPB.Document,
-      );
-
-      await tester
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
-      await tester.renamePage('Child page');
-      await tester.pumpAndSettle();
-
-      expect(find.text('Child page'), findsNWidgets(2));
-
-      await tester.editor.tapLineOfEditorAt(0);
-
-      // Undo
-      await tester.simulateKeyEvent(
-        LogicalKeyboardKey.keyZ,
-        isControlPressed: Platform.isLinux || Platform.isWindows,
-        isMetaPressed: Platform.isMacOS,
-      );
-      await tester.pumpAndSettle();
-
-      expect(find.byType(SubPageBlockComponent), findsNothing);
-      expect(find.text('Child page'), findsNothing);
-
-      // Redo
-      await tester.simulateKeyEvent(
-        LogicalKeyboardKey.keyZ,
-        isShiftPressed: true,
-        isControlPressed: Platform.isLinux || Platform.isWindows,
-        isMetaPressed: Platform.isMacOS,
-      );
-      await tester.pumpAndSettle();
-
-      expect(find.byType(SubPageBlockComponent), findsOneWidget);
-      expect(find.text('Child page'), findsNWidgets(2));
-    });
-
     // Redo: undoing deleting a subpage block, then redoing to delete it again
     // -> Add a subpage block
     // -> Delete
@@ -436,8 +354,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -476,8 +393,7 @@ void main() {
       expect(find.text('Child page'), findsNothing);
     });
 
-    testWidgets('Delete a view first to trash, then from trash',
+    testWidgets('Delete a view from sidebar', (tester) async {
-        (tester) async {
       await tester.initializeAppFlowy();
       await tester.tapAnonymousSignInButton();
       await tester.createNewPageWithNameUnderParent(name: 'SubPageBlock');
@@ -489,38 +405,17 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
       expect(find.text('Child page'), findsNWidgets(2));
       expect(find.byType(SubPageBlockComponent), findsOneWidget);
 
-      final hintText = LocaleKeys.document_plugins_subPage_inTrashHint.tr();
-      expect(find.text(hintText), findsNothing);
-
       await tester.hoverOnPageName('Child page');
       await tester.tapDeletePageButton();
       await tester.pumpAndSettle(const Duration(seconds: 1));
 
-      expect(find.text(hintText), findsOne);
-
-      // Go to trash
-      await tester.tapTrashButton();
-      await tester.pumpAndSettle();
-
-      // Tap on delete all button
-      await tester.tap(find.text(LocaleKeys.trash_deleteAll.tr()));
-      await tester.pumpAndSettle();
-
-      // Tap ok to delete app pages in trash
-      await tester.tap(find.text(LocaleKeys.button_delete.tr()));
-      await tester.pumpAndSettle();
-
-      await tester.openPage('SubPageBlock');
-      await tester.pumpAndSettle();
-
       expect(find.text('Child page'), findsNothing);
       expect(find.byType(SubPageBlockComponent), findsNothing);
     });
@@ -537,8 +432,7 @@ void main() {
         layout: ViewLayoutPB.Document,
       );
 
-      await tester
+      await tester.hoverOnPageName(_defaultPageName);
-          .hoverOnPageName(LocaleKeys.menuAppHeader_defaultNewPageName.tr());
       await tester.renamePage('Child page');
       await tester.pumpAndSettle();
 
@@ -578,10 +472,6 @@ void main() {
       expect(afterNode.type, SubPageBlockKeys.type);
       expect(afterNode.type, beforeNode.type);
       expect(find.byType(SubPageBlockComponent), findsOneWidget);
-      expect(
-        find.text(LocaleKeys.document_plugins_subPage_inTrashHint.tr()),
-        findsNothing,
-      );
     });
   });
 }
@@ -602,6 +492,10 @@ extension _SubPageTestHelper on WidgetTester {
       offset: 100,
     );
 
+    // Navigate to the previous page to see the SubPageBlock
+    await openPage('SubPageBlock');
+    await pumpAndSettle();
+
     await pumpUntilFound(find.byType(SubPageBlockComponent));
   }
 }

frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/sub_page/block_transaction_handler.dart

@@ -1,16 +1,20 @@
 import 'package:flutter/material.dart';
 
 import 'package:appflowy/generated/locale_keys.g.dart';
+import 'package:appflowy/mobile/application/mobile_router.dart';
 import 'package:appflowy/plugins/document/presentation/editor_plugins/block_transaction_handler/block_transaction_handler.dart';
 import 'package:appflowy/plugins/document/presentation/editor_plugins/mention/mention_page_block.dart';
 import 'package:appflowy/plugins/document/presentation/editor_plugins/sub_page/sub_page_block_component.dart';
 import 'package:appflowy/plugins/trash/application/trash_service.dart';
+import 'package:appflowy/startup/startup.dart';
+import 'package:appflowy/workspace/application/tabs/tabs_bloc.dart';
 import 'package:appflowy/workspace/application/view/view_service.dart';
 import 'package:appflowy_backend/log.dart';
 import 'package:appflowy_backend/protobuf/flowy-folder/view.pbenum.dart';
 import 'package:appflowy_editor/appflowy_editor.dart';
 import 'package:easy_localization/easy_localization.dart';
 import 'package:flowy_infra_ui/style_widget/snap_bar.dart';
+import 'package:universal_platform/universal_platform.dart';
 
 class SubPageBlockTransactionHandler extends BlockTransactionHandler {
   SubPageBlockTransactionHandler() : super(blockType: SubPageBlockKeys.type);
@@ -138,19 +142,29 @@ class SubPageBlockTransactionHandler extends BlockTransactionHandler {
       final viewOrResult = await ViewBackendService.createView(
         layoutType: ViewLayoutPB.Document,
         parentViewId: parentViewId,
-        name: LocaleKeys.menuAppHeader_defaultNewPageName.tr(),
+        name: '',
       );
 
       await viewOrResult.fold(
         (view) async {
           final transaction = editorState.transaction
             ..updateNode(node, {SubPageBlockKeys.viewId: view.id});
-          await editorState.apply(
+          await editorState
+              .apply(
             transaction,
             withUpdateSelection: false,
             options: const ApplyOptions(recordUndo: false),
-          );
+          )
+              .then((_) async {
             editorState.reload();
+
+            // Open the new page
+            if (UniversalPlatform.isDesktop) {
+              getIt<TabsBloc>().openPlugin(view);
+            } else {
+              await context.pushView(view);
+            }
+          });
         },
         (error) async {
           Log.error(error);

frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/sub_page/sub_page_block_component.dart

@@ -2,14 +2,17 @@ import 'package:flutter/material.dart';
 
 import 'package:appflowy/generated/locale_keys.g.dart';
 import 'package:appflowy/plugins/document/presentation/editor_plugins/mention/mention_page_block.dart';
-import 'package:appflowy/plugins/trash/application/trash_service.dart';
+import 'package:appflowy/plugins/trash/application/trash_listener.dart';
 import 'package:appflowy/startup/startup.dart';
 import 'package:appflowy/workspace/application/tabs/tabs_bloc.dart';
 import 'package:appflowy/workspace/application/view/view_ext.dart';
 import 'package:appflowy/workspace/application/view/view_listener.dart';
 import 'package:appflowy/workspace/application/view/view_service.dart';
+import 'package:appflowy_backend/protobuf/flowy-error/errors.pb.dart';
+import 'package:appflowy_backend/protobuf/flowy-folder/trash.pb.dart';
 import 'package:appflowy_backend/protobuf/flowy-folder/view.pb.dart';
 import 'package:appflowy_editor/appflowy_editor.dart';
+import 'package:appflowy_result/appflowy_result.dart';
 import 'package:collection/collection.dart';
 import 'package:easy_localization/easy_localization.dart';
 import 'package:flowy_infra_ui/style_widget/text.dart';
@@ -86,15 +89,16 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
   final subPageKey = GlobalKey();
 
   ViewListener? viewListener;
+  TrashListener? trashListener;
   Future<ViewPB?>? viewFuture;
 
   bool isHovering = false;
   bool isHandlingPaste = false;
 
-  bool isInTrash = false;
-
   EditorState get editorState => context.read<EditorState>();
 
+  String? parentId;
+
   @override
   void initState() {
     super.initState();
@@ -102,13 +106,8 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
     if (viewId != null) {
       viewFuture = fetchView(viewId);
       viewListener = ViewListener(viewId: viewId)
-        ..start(
+        ..start(onViewUpdated: onViewUpdated);
-          onViewUpdated: (view) {
+      trashListener = TrashListener()..start(trashUpdated: didUpdateTrash);
-            pageMemorizer[view.id] = view;
-            viewFuture = fetchView(viewId);
-            editorState.reload();
-          },
-        );
     }
   }
 
@@ -121,17 +120,51 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
       viewFuture = fetchView(viewId);
       viewListener?.stop();
       viewListener = ViewListener(viewId: viewId)
-        ..start(
+        ..start(onViewUpdated: onViewUpdated);
-          onViewUpdated: (view) {
-            pageMemorizer[view.id] = view;
-            viewFuture = fetchView(viewId);
-            editorState.reload();
-          },
-        );
     }
     super.didUpdateWidget(oldWidget);
   }
 
+  void didUpdateTrash(FlowyResult<List<TrashPB>, FlowyError> trashOrFailed) {
+    final trashList = trashOrFailed.toNullable();
+    if (trashList == null) {
+      return;
+    }
+
+    final viewId = node.attributes[SubPageBlockKeys.viewId];
+    if (trashList.any((t) => t.id == viewId)) {
+      WidgetsBinding.instance.addPostFrameCallback((_) {
+        if (mounted) {
+          final transaction = editorState.transaction..deleteNode(node);
+          editorState.apply(
+            transaction,
+            withUpdateSelection: false,
+            options: const ApplyOptions(recordUndo: false),
+          );
+        }
+      });
+    }
+  }
+
+  void onViewUpdated(ViewPB view) {
+    pageMemorizer[view.id] = view;
+    viewFuture = Future<ViewPB>.value(view);
+    editorState.reload();
+
+    if (parentId != view.parentViewId && parentId != null) {
+      WidgetsBinding.instance.addPostFrameCallback((_) {
+        if (mounted) {
+          final transaction = editorState.transaction..deleteNode(node);
+          editorState.apply(
+            transaction,
+            withUpdateSelection: false,
+            options: const ApplyOptions(recordUndo: false),
+          );
+        }
+      });
+    }
+  }
+
   @override
   void dispose() {
     viewListener?.stop();
@@ -150,21 +183,22 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
 
         final view = snapshot.data;
         if (view == null) {
-          // Delete this node if the view is not found
           WidgetsBinding.instance.addPostFrameCallback((_) {
+            if (mounted) {
               final transaction = editorState.transaction..deleteNode(node);
               editorState.apply(
                 transaction,
                 withUpdateSelection: false,
                 options: const ApplyOptions(recordUndo: false),
               );
+            }
           });
 
           return const SizedBox.shrink();
         }
 
         Widget child = Padding(
-          padding: const EdgeInsets.symmetric(vertical: 4),
+          padding: const EdgeInsets.symmetric(vertical: 2),
           child: MouseRegion(
             cursor: SystemMouseCursors.click,
             onEnter: (_) => setState(() => isHovering = true),
@@ -202,13 +236,11 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
                     decoration: BoxDecoration(
                       color: isHovering
                           ? Theme.of(context).colorScheme.secondary
-                          : Theme.of(context)
+                          : null,
-                              .colorScheme
-                              .surfaceContainerHighest,
                       borderRadius: BorderRadius.circular(4),
                     ),
                     child: SizedBox(
-                      height: 36,
+                      height: 32,
                       child: Row(
                         children: [
                           const HSpace(10),
@@ -223,32 +255,38 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
                                   child: view.defaultIcon(),
                                 ),
                           const HSpace(10),
-                          FlowyText(
+                          Flexible(
-                            view.name,
+                            child: FlowyText(
+                              view.name.trim().isEmpty
+                                  ? LocaleKeys.menuAppHeader_defaultNewPageName
+                                      .tr()
+                                  : view.name,
                               fontSize: textStyle.fontSize,
                               fontWeight: textStyle.fontWeight,
                               lineHeight: textStyle.height,
+                              overflow: TextOverflow.ellipsis,
                             ),
-                          if (isInTrash) ...[
-                            const HSpace(4),
-                            FlowyText(
-                              LocaleKeys.document_plugins_subPage_inTrashHint.tr(),
-                              fontSize: textStyle.fontSize,
-                              fontWeight: textStyle.fontWeight,
-                              lineHeight: textStyle.height,
-                              color: Theme.of(context).hintColor,
                           ),
-                          ] else if (isHandlingPaste) ...[
+                          if (isHandlingPaste) ...[
                             FlowyText(
-                              LocaleKeys.document_plugins_subPage_handlingPasteHint.tr(),
+                              LocaleKeys
+                                  .document_plugins_subPage_handlingPasteHint
+                                  .tr(),
                               fontSize: textStyle.fontSize,
                               fontWeight: textStyle.fontWeight,
                               lineHeight: textStyle.height,
                               color: Theme.of(context).hintColor,
                             ),
                             const HSpace(10),
-                            const CircularProgressIndicator(),
+                            const SizedBox(
+                              height: 16,
+                              width: 16,
+                              child: CircularProgressIndicator(
+                                strokeWidth: 1.5,
+                              ),
+                            ),
                           ],
+                          const HSpace(10),
                         ],
                       ),
                     ),
@@ -278,20 +316,19 @@ class SubPageBlockComponentState extends State<SubPageBlockComponent>
     );
 
     if (view == null) {
-      // try to fetch from trash
+      WidgetsBinding.instance.addPostFrameCallback((_) {
-      final trashViews = await TrashService().readTrash();
+        if (mounted) {
-      final trash = trashViews.fold(
+          final transaction = editorState.transaction..deleteNode(node);
-        (l) => l.items.firstWhereOrNull((trash) => trash.id == pageId),
+          editorState.apply(
-        (r) => null,
+            transaction,
+            withUpdateSelection: false,
+            options: const ApplyOptions(recordUndo: false),
           );
-      if (trash != null) {
-        isInTrash = true;
-        return ViewPB()
-          ..id = trash.id
-          ..name = trash.name;
         }
+      });
     }
 
+    parentId = view?.parentViewId;
     return view;
   }
 

frontend/resources/translations/en.json

@@ -1575,7 +1575,7 @@
         "file": "File"
       },
       "subPage": {
-        "name": "Embed sub-page",
+        "name": "Insert page",
         "keyword1": "sub page",
         "keyword2": "page",
         "keyword3": "child page",
@@ -1764,7 +1764,6 @@
         "failedToOpenMsg": "Failed to open, file not found"
       },
       "subPage": {
-        "inTrashHint": " - (in trash)",
         "handlingPasteHint": " - (handling paste)",
         "errors": {
           "failedDeletePage": "Failed to delete page",

frontend/rust-lib/flowy-folder/src/manager.rs

@@ -933,8 +933,6 @@ impl FolderManager {
       ));
     }
 
-    tracing::warn!("[DEBUG] Duplicating view {}", view_id);
-
     // filter the view ids that in the trash or private section
     let filtered_view_ids = match self.mutex_folder.load_full() {
       None => Vec::default(),
@@ -1022,7 +1020,6 @@ impl FolderManager {
 
       if is_source_view {
         new_view_id = duplicated_view.id.clone();
-        tracing::warn!("[DEBUG] Setting new view id to {}", new_view_id);
       }
 
       if sync_after_create {
@@ -1075,8 +1072,6 @@ impl FolderManager {
 
     let duplicated_view = self.get_view_pb(&new_view_id).await?;
 
-    tracing::warn!("[DEBUG] Duplicated view: {:?}", duplicated_view);
-
     Ok(duplicated_view)
   }