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

* 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 <lucas.xu@appflowy.io> --------- Co-authored-by: Lucas <lucas.xu@appflowy.io>
2025-04-24 06:37:14 -04:00 · 2024-12-30 17:55:40 +08:00 · 2024-12-30 17:55:40 +08:00 · 8826e479eb
commit 8826e479eb
parent 8e4fe3d559
4 changed files with 155 additions and 69 deletions
--- 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);
        },
      );
    });
--- 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) {
--- 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);
+      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(),
+            ),
+          ],
+        ),
+        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',
+              );
            },
-            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',
-                );
-              },
-            ),
-          ),
-        ],
+        ),
      ),
    );
  }
@ -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),
--- 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
@ -207,6 +207,7 @@ class _SidebarSwitchWorkspaceButtonState
      direction: PopoverDirection.bottomWithCenterAligned,
      offset: const Offset(0, 5),
      constraints: const BoxConstraints(maxWidth: 300, maxHeight: 600),
+      margin: EdgeInsets.zero,
      animationDuration: Durations.short3,
      beginScaleFactor: 1.0,
      beginOpacity: 0.8,