fix(flutter_desktop): workspace menu ui issues (#7091)

richardshiue · LucasXu0 · LucasXu0 · commit 8826e479eb3e · 2024-12-30T17:55:54.000+08:00
* fix(flutter_desktop): remove log out and workspace option popovers conflict

* test: add integration test

* fix(flutter_desktop): workspace list scrollbar overlaps with list

* chore(flutter_desktop): fix padding around import from notion button

* chore(flutter_desktop): adjust popover conflict rules for workspace

* test: add integration tests

* chore(flutter_desktop): make the popoovers as barriers

* fix: regression from making the workspace item menu as barrier

* chore: update frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_actions.dart

Co-authored-by: Lucas &lt;lucas.xu@appflowy.io&gt;

---------

Co-authored-by: Lucas &lt;lucas.xu@appflowy.io&gt;
diff --git a/frontend/appflowy_flutter/integration_test/desktop/cloud/workspace/collaborative_workspace_test.dart b/frontend/appflowy_flutter/integration_test/desktop/cloud/workspace/collaborative_workspace_test.dart
@@ -5,6 +5,7 @@ import 'package:appflowy/shared/feature_flags.dart';
 import 'package:appflowy/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_actions.dart';
 import 'package:appflowy/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_menu.dart';
 import 'package:easy_localization/easy_localization.dart';
+import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:integration_test/integration_test.dart';
 
@@ -102,8 +103,7 @@ void main() {
       expect(memberCount, findsNWidgets(2));
     });
 
-    testWidgets('only display one menu item in the workspace menu',
-        (tester) async {
+    testWidgets('workspace menu popover behavior test', (tester) async {
       // only run the test when the feature flag is on
       if (!FeatureFlag.collaborativeWorkspace.isOn) {
         return;
@@ -128,6 +128,8 @@ void main() {
       final workspaceItem = find.byWidgetPredicate(
         (w) => w is WorkspaceMenuItem && w.workspace.name == name,
       );
+
+      // the workspace menu shouldn't conflict with logout
       await tester.hoverOnWidget(
         workspaceItem,
         onHover: () async {
@@ -136,15 +138,73 @@ void main() {
           );
           expect(moreButton, findsOneWidget);
           await tester.tapButton(moreButton);
+          expect(find.text(LocaleKeys.button_rename.tr()), findsOneWidget);
+
+          final logoutButton = find.byType(WorkspaceMoreButton);
+          await tester.tapButton(logoutButton);
+          expect(find.text(LocaleKeys.button_logout.tr()), findsOneWidget);
+          expect(moreButton, findsNothing);
+
+          await tester.tapButton(moreButton);
+          expect(find.text(LocaleKeys.button_logout.tr()), findsNothing);
+          expect(moreButton, findsOneWidget);
+        },
+      );
+      await tester.sendKeyEvent(LogicalKeyboardKey.escape);
+      await tester.pumpAndSettle();
+
+      // clicking on the more action button for the same workspace shouldn't do
+      // anything
+      await tester.openCollaborativeWorkspaceMenu();
+      await tester.hoverOnWidget(
+        workspaceItem,
+        onHover: () async {
+          final moreButton = find.byWidgetPredicate(
+            (w) => w is WorkspaceMoreActionList && w.workspace.name == name,
+          );
+          expect(moreButton, findsOneWidget);
+          await tester.tapButton(moreButton);
+          expect(find.text(LocaleKeys.button_rename.tr()), findsOneWidget);
 
           // click it again
           await tester.tapButton(moreButton);
 
           // nothing should happen
-          expect(
-            find.text(LocaleKeys.button_rename.tr()),
-            findsOneWidget,
-          );
+          expect(find.text(LocaleKeys.button_rename.tr()), findsOneWidget);
+        },
+      );
+      await tester.sendKeyEvent(LogicalKeyboardKey.escape);
+      await tester.pumpAndSettle();
+
+      // clicking on the more button of another workspace should close the menu
+      // for this one
+      await tester.openCollaborativeWorkspaceMenu();
+      final moreButton = find.byWidgetPredicate(
+        (w) => w is WorkspaceMoreActionList && w.workspace.name == name,
+      );
+      await tester.hoverOnWidget(
+        workspaceItem,
+        onHover: () async {
+          expect(moreButton, findsOneWidget);
+          await tester.tapButton(moreButton);
+          expect(find.text(LocaleKeys.button_rename.tr()), findsOneWidget);
+        },
+      );
+
+      final otherWorspaceItem = find.byWidgetPredicate(
+        (w) => w is WorkspaceMenuItem && w.workspace.name != name,
+      );
+      final otherMoreButton = find.byWidgetPredicate(
+        (w) => w is WorkspaceMoreActionList && w.workspace.name != name,
+      );
+      await tester.hoverOnWidget(
+        otherWorspaceItem,
+        onHover: () async {
+          expect(otherMoreButton, findsOneWidget);
+          await tester.tapButton(otherMoreButton);
+          expect(find.text(LocaleKeys.button_rename.tr()), findsOneWidget);
+
+          expect(moreButton, findsNothing);
         },
       );
     });
diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_actions.dart b/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_actions.dart
@@ -18,15 +18,23 @@ enum WorkspaceMoreAction {
   divider,
 }
 
-class WorkspaceMoreActionList extends StatelessWidget {
+class WorkspaceMoreActionList extends StatefulWidget {
   const WorkspaceMoreActionList({
     super.key,
     required this.workspace,
-    required this.isShowingMoreActions,
+    required this.popoverMutex,
   });
 
   final UserWorkspacePB workspace;
-  final ValueNotifier<bool> isShowingMoreActions;
+  final PopoverMutex popoverMutex;
+
+  @override
+  State<WorkspaceMoreActionList> createState() =>
+      _WorkspaceMoreActionListState();
+}
+
+class _WorkspaceMoreActionListState extends State<WorkspaceMoreActionList> {
+  bool isPopoverOpen = false;
 
   @override
   Widget build(BuildContext context) {
@@ -45,16 +53,22 @@ class WorkspaceMoreActionList extends StatelessWidget {
     return PopoverActionList<_WorkspaceMoreActionWrapper>(
       direction: PopoverDirection.bottomWithLeftAligned,
       actions: actions
-          .map((e) => _WorkspaceMoreActionWrapper(e, workspace))
+          .map(
+            (action) => _WorkspaceMoreActionWrapper(
+              action,
+              widget.workspace,
+              () => PopoverContainer.of(context).closeAll(),
+            ),
+          )
           .toList(),
+      mutex: widget.popoverMutex,
       constraints: const BoxConstraints(minWidth: 220),
       animationDuration: Durations.short3,
       slideDistance: 2,
       beginScaleFactor: 1.0,
       beginOpacity: 0.8,
-      onClosed: () {
-        isShowingMoreActions.value = false;
-      },
+      onClosed: () => isPopoverOpen = false,
+      asBarrier: true,
       buildChild: (controller) {
         return SizedBox.square(
           dimension: 24.0,
@@ -64,11 +78,10 @@ class WorkspaceMoreActionList extends StatelessWidget {
               FlowySvgs.workspace_three_dots_s,
             ),
             onTap: () {
-              if (!isShowingMoreActions.value) {
+              if (!isPopoverOpen) {
                 controller.show();
+                isPopoverOpen = true;
               }
-
-              isShowingMoreActions.value = true;
             },
           ),
         );
@@ -79,10 +92,15 @@ class WorkspaceMoreActionList extends StatelessWidget {
 }
 
 class _WorkspaceMoreActionWrapper extends CustomActionCell {
-  _WorkspaceMoreActionWrapper(this.inner, this.workspace);
+  _WorkspaceMoreActionWrapper(
+    this.inner,
+    this.workspace,
+    this.closeWorkspaceMenu,
+  );
 
   final WorkspaceMoreAction inner;
   final UserWorkspacePB workspace;
+  final VoidCallback closeWorkspaceMenu;
 
   @override
   Widget buildWithContext(
@@ -117,6 +135,7 @@ class _WorkspaceMoreActionWrapper extends CustomActionCell {
       margin: const EdgeInsets.all(6),
       onTap: () async {
         PopoverContainer.of(context).closeAll();
+        closeWorkspaceMenu();
 
         final workspaceBloc = context.read<UserWorkspaceBloc>();
         switch (inner) {
diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_menu.dart b/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_menu.dart
@@ -43,13 +43,7 @@ class WorkspacesMenu extends StatefulWidget {
 }
 
 class _WorkspacesMenuState extends State<WorkspacesMenu> {
-  final ValueNotifier<bool> isShowingMoreActions = ValueNotifier(false);
-
-  @override
-  void dispose() {
-    isShowingMoreActions.dispose();
-    super.dispose();
-  }
+  final popoverMutex = PopoverMutex();
 
   @override
   Widget build(BuildContext context) {
@@ -59,7 +53,7 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
       children: [
         // user email
         Padding(
-          padding: const EdgeInsets.symmetric(horizontal: 4.0),
+          padding: const EdgeInsets.only(left: 10.0, top: 6.0, right: 10.0),
           child: Row(
             children: [
               Expanded(
@@ -71,18 +65,21 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
                 ),
               ),
               const HSpace(4.0),
-              const _WorkspaceMoreButton(),
+              WorkspaceMoreButton(
+                popoverMutex: popoverMutex,
+              ),
               const HSpace(8.0),
             ],
           ),
         ),
         const Padding(
-          padding: EdgeInsets.symmetric(vertical: 8.0),
+          padding: EdgeInsets.symmetric(vertical: 8.0, horizontal: 6.0),
           child: Divider(height: 1.0),
         ),
         // workspace list
         Flexible(
           child: SingleChildScrollView(
+            padding: const EdgeInsets.symmetric(horizontal: 6.0),
             child: Column(
               mainAxisSize: MainAxisSize.min,
               children: [
@@ -93,7 +90,7 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
                     userProfile: widget.userProfile,
                     isSelected: workspace.workspaceId ==
                         widget.currentWorkspace.workspaceId,
-                    isShowingMoreActions: isShowingMoreActions,
+                    popoverMutex: popoverMutex,
                   ),
                   const VSpace(6.0),
                 ],
@@ -102,13 +99,19 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
           ),
         ),
         // add new workspace
-        const _CreateWorkspaceButton(),
-        const VSpace(6.0),
+        const Padding(
+          padding: EdgeInsets.symmetric(horizontal: 6.0),
+          child: _CreateWorkspaceButton(),
+        ),
 
         if (UniversalPlatform.isDesktop) ...[
-          const _ImportNotionButton(),
-          const VSpace(6.0),
+          const Padding(
+            padding: EdgeInsets.only(left: 6.0, top: 6.0, right: 6.0),
+            child: _ImportNotionButton(),
+          ),
         ],
+
+        const VSpace(6.0),
       ],
     );
   }
@@ -132,13 +135,13 @@ class WorkspaceMenuItem extends StatefulWidget {
     required this.workspace,
     required this.userProfile,
     required this.isSelected,
-    required this.isShowingMoreActions,
+    required this.popoverMutex,
   });
 
   final UserProfilePB userProfile;
   final UserWorkspacePB workspace;
   final bool isSelected;
-  final ValueNotifier<bool> isShowingMoreActions;
+  final PopoverMutex popoverMutex;
 
   @override
   State<WorkspaceMenuItem> createState() => _WorkspaceMenuItemState();
@@ -230,7 +233,7 @@ class _WorkspaceMenuItemState extends State<WorkspaceMenuItem> {
             },
             child: WorkspaceMoreActionList(
               workspace: widget.workspace,
-              isShowingMoreActions: widget.isShowingMoreActions,
+              popoverMutex: widget.popoverMutex,
             ),
           ),
         const HSpace(8.0),
@@ -394,40 +397,35 @@ class _ImportNotionButton extends StatelessWidget {
   Widget build(BuildContext context) {
     return SizedBox(
       height: 40,
-      child: Stack(
-        alignment: Alignment.centerRight,
-        children: [
-          FlowyButton(
-            key: importNotionButtonKey,
-            onTap: () {
-              _showImportNotinoDialog(context);
-            },
-            margin: const EdgeInsets.symmetric(horizontal: 4.0),
-            text: Row(
-              children: [
-                _buildLeftIcon(context),
-                const HSpace(8.0),
-                FlowyText.regular(
-                  LocaleKeys.workspace_importFromNotion.tr(),
-                ),
-              ],
+      child: FlowyButton(
+        key: importNotionButtonKey,
+        onTap: () {
+          _showImportNotinoDialog(context);
+        },
+        margin: const EdgeInsets.symmetric(horizontal: 4.0),
+        text: Row(
+          children: [
+            _buildLeftIcon(context),
+            const HSpace(8.0),
+            FlowyText.regular(
+              LocaleKeys.workspace_importFromNotion.tr(),
             ),
-          ),
-          FlowyTooltip(
-            message: LocaleKeys.workspace_learnMore.tr(),
-            preferBelow: true,
-            child: FlowyIconButton(
-              icon: const FlowySvg(
-                FlowySvgs.information_s,
-              ),
-              onPressed: () {
-                afLaunchUrlString(
-                  'https://docs.appflowy.io/docs/guides/import-from-notion',
-                );
-              },
+          ],
+        ),
+        rightIcon: FlowyTooltip(
+          message: LocaleKeys.workspace_learnMore.tr(),
+          preferBelow: true,
+          child: FlowyIconButton(
+            icon: const FlowySvg(
+              FlowySvgs.information_s,
             ),
+            onPressed: () {
+              afLaunchUrlString(
+                'https://docs.appflowy.io/docs/guides/import-from-notion',
+              );
+            },
           ),
-        ],
+        ),
       ),
     );
   }
@@ -478,14 +476,22 @@ class _ImportNotionButton extends StatelessWidget {
   }
 }
 
-class _WorkspaceMoreButton extends StatelessWidget {
-  const _WorkspaceMoreButton();
+@visibleForTesting
+class WorkspaceMoreButton extends StatelessWidget {
+  const WorkspaceMoreButton({
+    super.key,
+    required this.popoverMutex,
+  });
+
+  final PopoverMutex popoverMutex;
 
   @override
   Widget build(BuildContext context) {
     return AppFlowyPopover(
       direction: PopoverDirection.bottomWithLeftAligned,
       offset: const Offset(0, 6),
+      mutex: popoverMutex,
+      asBarrier: true,
       popupBuilder: (_) => FlowyButton(
         margin: const EdgeInsets.symmetric(horizontal: 6.0, vertical: 7.0),
         leftIcon: const FlowySvg(FlowySvgs.workspace_logout_s),
diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/sidebar_workspace.dart b/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/workspace/sidebar_workspace.dart
