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>
This commit is contained in:
Richard Shiue 2024-12-30 17:55:40 +08:00 committed by GitHub
parent e04c1bdaec
commit 92722d0922
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 155 additions and 69 deletions

View File

@ -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_actions.dart';
import 'package:appflowy/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_menu.dart'; import 'package:appflowy/workspace/presentation/home/menu/sidebar/workspace/_sidebar_workspace_menu.dart';
import 'package:easy_localization/easy_localization.dart'; import 'package:easy_localization/easy_localization.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart'; import 'package:integration_test/integration_test.dart';
@ -102,8 +103,7 @@ void main() {
expect(memberCount, findsNWidgets(2)); expect(memberCount, findsNWidgets(2));
}); });
testWidgets('only display one menu item in the workspace menu', testWidgets('workspace menu popover behavior test', (tester) async {
(tester) async {
// only run the test when the feature flag is on // only run the test when the feature flag is on
if (!FeatureFlag.collaborativeWorkspace.isOn) { if (!FeatureFlag.collaborativeWorkspace.isOn) {
return; return;
@ -128,6 +128,8 @@ void main() {
final workspaceItem = find.byWidgetPredicate( final workspaceItem = find.byWidgetPredicate(
(w) => w is WorkspaceMenuItem && w.workspace.name == name, (w) => w is WorkspaceMenuItem && w.workspace.name == name,
); );
// the workspace menu shouldn't conflict with logout
await tester.hoverOnWidget( await tester.hoverOnWidget(
workspaceItem, workspaceItem,
onHover: () async { onHover: () async {
@ -136,15 +138,73 @@ void main() {
); );
expect(moreButton, findsOneWidget); expect(moreButton, findsOneWidget);
await tester.tapButton(moreButton); 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 // click it again
await tester.tapButton(moreButton); await tester.tapButton(moreButton);
// nothing should happen // nothing should happen
expect( expect(find.text(LocaleKeys.button_rename.tr()), findsOneWidget);
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);
}, },
); );
}); });

View File

@ -18,15 +18,23 @@ enum WorkspaceMoreAction {
divider, divider,
} }
class WorkspaceMoreActionList extends StatelessWidget { class WorkspaceMoreActionList extends StatefulWidget {
const WorkspaceMoreActionList({ const WorkspaceMoreActionList({
super.key, super.key,
required this.workspace, required this.workspace,
required this.isShowingMoreActions, required this.popoverMutex,
}); });
final UserWorkspacePB workspace; final UserWorkspacePB workspace;
final ValueNotifier<bool> isShowingMoreActions; final PopoverMutex popoverMutex;
@override
State<WorkspaceMoreActionList> createState() =>
_WorkspaceMoreActionListState();
}
class _WorkspaceMoreActionListState extends State<WorkspaceMoreActionList> {
bool isPopoverOpen = false;
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
@ -45,16 +53,22 @@ class WorkspaceMoreActionList extends StatelessWidget {
return PopoverActionList<_WorkspaceMoreActionWrapper>( return PopoverActionList<_WorkspaceMoreActionWrapper>(
direction: PopoverDirection.bottomWithLeftAligned, direction: PopoverDirection.bottomWithLeftAligned,
actions: actions actions: actions
.map((e) => _WorkspaceMoreActionWrapper(e, workspace)) .map(
(action) => _WorkspaceMoreActionWrapper(
action,
widget.workspace,
() => PopoverContainer.of(context).closeAll(),
),
)
.toList(), .toList(),
mutex: widget.popoverMutex,
constraints: const BoxConstraints(minWidth: 220), constraints: const BoxConstraints(minWidth: 220),
animationDuration: Durations.short3, animationDuration: Durations.short3,
slideDistance: 2, slideDistance: 2,
beginScaleFactor: 1.0, beginScaleFactor: 1.0,
beginOpacity: 0.8, beginOpacity: 0.8,
onClosed: () { onClosed: () => isPopoverOpen = false,
isShowingMoreActions.value = false; asBarrier: true,
},
buildChild: (controller) { buildChild: (controller) {
return SizedBox.square( return SizedBox.square(
dimension: 24.0, dimension: 24.0,
@ -64,11 +78,10 @@ class WorkspaceMoreActionList extends StatelessWidget {
FlowySvgs.workspace_three_dots_s, FlowySvgs.workspace_three_dots_s,
), ),
onTap: () { onTap: () {
if (!isShowingMoreActions.value) { if (!isPopoverOpen) {
controller.show(); controller.show();
isPopoverOpen = true;
} }
isShowingMoreActions.value = true;
}, },
), ),
); );
@ -79,10 +92,15 @@ class WorkspaceMoreActionList extends StatelessWidget {
} }
class _WorkspaceMoreActionWrapper extends CustomActionCell { class _WorkspaceMoreActionWrapper extends CustomActionCell {
_WorkspaceMoreActionWrapper(this.inner, this.workspace); _WorkspaceMoreActionWrapper(
this.inner,
this.workspace,
this.closeWorkspaceMenu,
);
final WorkspaceMoreAction inner; final WorkspaceMoreAction inner;
final UserWorkspacePB workspace; final UserWorkspacePB workspace;
final VoidCallback closeWorkspaceMenu;
@override @override
Widget buildWithContext( Widget buildWithContext(
@ -117,6 +135,7 @@ class _WorkspaceMoreActionWrapper extends CustomActionCell {
margin: const EdgeInsets.all(6), margin: const EdgeInsets.all(6),
onTap: () async { onTap: () async {
PopoverContainer.of(context).closeAll(); PopoverContainer.of(context).closeAll();
closeWorkspaceMenu();
final workspaceBloc = context.read<UserWorkspaceBloc>(); final workspaceBloc = context.read<UserWorkspaceBloc>();
switch (inner) { switch (inner) {

View File

@ -43,13 +43,7 @@ class WorkspacesMenu extends StatefulWidget {
} }
class _WorkspacesMenuState extends State<WorkspacesMenu> { class _WorkspacesMenuState extends State<WorkspacesMenu> {
final ValueNotifier<bool> isShowingMoreActions = ValueNotifier(false); final popoverMutex = PopoverMutex();
@override
void dispose() {
isShowingMoreActions.dispose();
super.dispose();
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
@ -59,7 +53,7 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
children: [ children: [
// user email // user email
Padding( Padding(
padding: const EdgeInsets.symmetric(horizontal: 4.0), padding: const EdgeInsets.only(left: 10.0, top: 6.0, right: 10.0),
child: Row( child: Row(
children: [ children: [
Expanded( Expanded(
@ -71,18 +65,21 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
), ),
), ),
const HSpace(4.0), const HSpace(4.0),
const _WorkspaceMoreButton(), WorkspaceMoreButton(
popoverMutex: popoverMutex,
),
const HSpace(8.0), const HSpace(8.0),
], ],
), ),
), ),
const Padding( const Padding(
padding: EdgeInsets.symmetric(vertical: 8.0), padding: EdgeInsets.symmetric(vertical: 8.0, horizontal: 6.0),
child: Divider(height: 1.0), child: Divider(height: 1.0),
), ),
// workspace list // workspace list
Flexible( Flexible(
child: SingleChildScrollView( child: SingleChildScrollView(
padding: const EdgeInsets.symmetric(horizontal: 6.0),
child: Column( child: Column(
mainAxisSize: MainAxisSize.min, mainAxisSize: MainAxisSize.min,
children: [ children: [
@ -93,7 +90,7 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
userProfile: widget.userProfile, userProfile: widget.userProfile,
isSelected: workspace.workspaceId == isSelected: workspace.workspaceId ==
widget.currentWorkspace.workspaceId, widget.currentWorkspace.workspaceId,
isShowingMoreActions: isShowingMoreActions, popoverMutex: popoverMutex,
), ),
const VSpace(6.0), const VSpace(6.0),
], ],
@ -102,13 +99,19 @@ class _WorkspacesMenuState extends State<WorkspacesMenu> {
), ),
), ),
// add new workspace // add new workspace
const _CreateWorkspaceButton(), const Padding(
const VSpace(6.0), padding: EdgeInsets.symmetric(horizontal: 6.0),
child: _CreateWorkspaceButton(),
),
if (UniversalPlatform.isDesktop) ...[ if (UniversalPlatform.isDesktop) ...[
const _ImportNotionButton(), const Padding(
const VSpace(6.0), 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.workspace,
required this.userProfile, required this.userProfile,
required this.isSelected, required this.isSelected,
required this.isShowingMoreActions, required this.popoverMutex,
}); });
final UserProfilePB userProfile; final UserProfilePB userProfile;
final UserWorkspacePB workspace; final UserWorkspacePB workspace;
final bool isSelected; final bool isSelected;
final ValueNotifier<bool> isShowingMoreActions; final PopoverMutex popoverMutex;
@override @override
State<WorkspaceMenuItem> createState() => _WorkspaceMenuItemState(); State<WorkspaceMenuItem> createState() => _WorkspaceMenuItemState();
@ -230,7 +233,7 @@ class _WorkspaceMenuItemState extends State<WorkspaceMenuItem> {
}, },
child: WorkspaceMoreActionList( child: WorkspaceMoreActionList(
workspace: widget.workspace, workspace: widget.workspace,
isShowingMoreActions: widget.isShowingMoreActions, popoverMutex: widget.popoverMutex,
), ),
), ),
const HSpace(8.0), const HSpace(8.0),
@ -394,10 +397,7 @@ class _ImportNotionButton extends StatelessWidget {
Widget build(BuildContext context) { Widget build(BuildContext context) {
return SizedBox( return SizedBox(
height: 40, height: 40,
child: Stack( child: FlowyButton(
alignment: Alignment.centerRight,
children: [
FlowyButton(
key: importNotionButtonKey, key: importNotionButtonKey,
onTap: () { onTap: () {
_showImportNotinoDialog(context); _showImportNotinoDialog(context);
@ -412,8 +412,7 @@ class _ImportNotionButton extends StatelessWidget {
), ),
], ],
), ),
), rightIcon: FlowyTooltip(
FlowyTooltip(
message: LocaleKeys.workspace_learnMore.tr(), message: LocaleKeys.workspace_learnMore.tr(),
preferBelow: true, preferBelow: true,
child: FlowyIconButton( child: FlowyIconButton(
@ -427,7 +426,6 @@ class _ImportNotionButton extends StatelessWidget {
}, },
), ),
), ),
],
), ),
); );
} }
@ -478,14 +476,22 @@ class _ImportNotionButton extends StatelessWidget {
} }
} }
class _WorkspaceMoreButton extends StatelessWidget { @visibleForTesting
const _WorkspaceMoreButton(); class WorkspaceMoreButton extends StatelessWidget {
const WorkspaceMoreButton({
super.key,
required this.popoverMutex,
});
final PopoverMutex popoverMutex;
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
return AppFlowyPopover( return AppFlowyPopover(
direction: PopoverDirection.bottomWithLeftAligned, direction: PopoverDirection.bottomWithLeftAligned,
offset: const Offset(0, 6), offset: const Offset(0, 6),
mutex: popoverMutex,
asBarrier: true,
popupBuilder: (_) => FlowyButton( popupBuilder: (_) => FlowyButton(
margin: const EdgeInsets.symmetric(horizontal: 6.0, vertical: 7.0), margin: const EdgeInsets.symmetric(horizontal: 6.0, vertical: 7.0),
leftIcon: const FlowySvg(FlowySvgs.workspace_logout_s), leftIcon: const FlowySvg(FlowySvgs.workspace_logout_s),

View File

@ -207,6 +207,7 @@ class _SidebarSwitchWorkspaceButtonState
direction: PopoverDirection.bottomWithCenterAligned, direction: PopoverDirection.bottomWithCenterAligned,
offset: const Offset(0, 5), offset: const Offset(0, 5),
constraints: const BoxConstraints(maxWidth: 300, maxHeight: 600), constraints: const BoxConstraints(maxWidth: 300, maxHeight: 600),
margin: EdgeInsets.zero,
animationDuration: Durations.short3, animationDuration: Durations.short3,
beginScaleFactor: 1.0, beginScaleFactor: 1.0,
beginOpacity: 0.8, beginOpacity: 0.8,