From d3ce59fe08f752ff6efac1d7b34fd31201bf42d9 Mon Sep 17 00:00:00 2001 From: Lucas Date: Fri, 9 May 2025 13:02:19 +0800 Subject: [PATCH] fix: v0.9.2 launch review issues (#7898) * fix: remove empty paragraphs after inserting file block * fix: chat page animation * fix: remove opacity from locked page icon * feat: open the space after opening the view * fix: chat page loading issue * fix: view title assertion * fix: cover title issue * fix: flutter analyze * fix: list state issue * fix: reverse animation list issue * fix: integration test --- frontend/appflowy_flutter/ios/Podfile.lock | 11 --- .../presentation/animated_chat_list.dart | 82 ++++++++++++------- .../animated_chat_list_reversed.dart | 2 +- .../file/file_selection_menu.dart | 20 +++-- .../editor_plugins/header/cover_title.dart | 8 +- .../home/desktop_home_screen.dart | 14 ++++ .../workspace/presentation/home/hotkeys.dart | 2 + .../menu/sidebar/space/sidebar_space.dart | 16 ++++ .../widgets/lock_page_action.dart | 5 +- .../presentation/widgets/view_title_bar.dart | 8 +- .../unit_test/editor/file_block_test.dart | 46 +++++++++++ 11 files changed, 156 insertions(+), 58 deletions(-) create mode 100644 frontend/appflowy_flutter/test/unit_test/editor/file_block_test.dart diff --git a/frontend/appflowy_flutter/ios/Podfile.lock b/frontend/appflowy_flutter/ios/Podfile.lock index 24510f46e7..625593982c 100644 --- a/frontend/appflowy_flutter/ios/Podfile.lock +++ b/frontend/appflowy_flutter/ios/Podfile.lock @@ -71,11 +71,6 @@ PODS: - SDWebImage (5.21.0): - SDWebImage/Core (= 5.21.0) - SDWebImage/Core (5.21.0) - - Sentry/HybridSDK (8.46.0) - - sentry_flutter (8.14.2): - - Flutter - - FlutterMacOS - - Sentry/HybridSDK (= 8.46.0) - share_plus (0.0.1): - Flutter - shared_preferences_foundation (0.0.1): @@ -112,7 +107,6 @@ DEPENDENCIES: - path_provider_foundation (from `.symlinks/plugins/path_provider_foundation/darwin`) - permission_handler_apple (from `.symlinks/plugins/permission_handler_apple/ios`) - saver_gallery (from `.symlinks/plugins/saver_gallery/ios`) - - sentry_flutter (from `.symlinks/plugins/sentry_flutter/ios`) - share_plus (from `.symlinks/plugins/share_plus/ios`) - shared_preferences_foundation (from `.symlinks/plugins/shared_preferences_foundation/darwin`) - sqflite_darwin (from `.symlinks/plugins/sqflite_darwin/darwin`) @@ -126,7 +120,6 @@ SPEC REPOS: - DKPhotoGallery - ReachabilitySwift - SDWebImage - - Sentry - SwiftyGif - Toast @@ -165,8 +158,6 @@ EXTERNAL SOURCES: :path: ".symlinks/plugins/permission_handler_apple/ios" saver_gallery: :path: ".symlinks/plugins/saver_gallery/ios" - sentry_flutter: - :path: ".symlinks/plugins/sentry_flutter/ios" share_plus: :path: ".symlinks/plugins/share_plus/ios" shared_preferences_foundation: @@ -202,8 +193,6 @@ SPEC CHECKSUMS: ReachabilitySwift: 32793e867593cfc1177f5d16491e3a197d2fccda saver_gallery: af2d0c762dafda254e0ad025ef0dabd6506cd490 SDWebImage: f84b0feeb08d2d11e6a9b843cb06d75ebf5b8868 - Sentry: da60d980b197a46db0b35ea12cb8f39af48d8854 - sentry_flutter: 27892878729f42701297c628eb90e7c6529f3684 share_plus: 50da8cb520a8f0f65671c6c6a99b3617ed10a58a shared_preferences_foundation: 9e1978ff2562383bd5676f64ec4e9aa8fa06a6f7 sqflite_darwin: 20b2a3a3b70e43edae938624ce550a3cbf66a3d0 diff --git a/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list.dart b/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list.dart index 355037d665..bfb29a55d4 100644 --- a/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list.dart +++ b/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list.dart @@ -3,7 +3,7 @@ import 'dart:async'; import 'dart:math'; -import 'package:appflowy_backend/log.dart'; +import 'package:appflowy/util/debounce.dart'; import 'package:diffutil_dart/diffutil.dart' as diffutil; import 'package:flowy_infra_ui/widget/spacing.dart'; import 'package:flutter/material.dart'; @@ -43,7 +43,6 @@ class ChatAnimatedList extends StatefulWidget { class ChatAnimatedListState extends State with SingleTickerProviderStateMixin { - final GlobalKey _listKey = GlobalKey(); late final ChatController _chatController = Provider.of( context, listen: false, @@ -69,6 +68,10 @@ class ChatAnimatedListState extends State int _lastUserMessageIndex = 0; bool _isScrollingToBottom = false; + final _loadPreviousMessagesDebounce = Debounce( + duration: const Duration(milliseconds: 200), + ); + @override void initState() { super.initState(); @@ -134,15 +137,19 @@ class ChatAnimatedListState extends State itemPositionsListener.itemPositions.addListener(() { _handleToggleScrollToBottom(); }); + + itemPositionsListener.itemPositions.addListener(() { + _handleLoadPreviousMessages(); + }); } @override void dispose() { - super.dispose(); - _scrollToBottomShowTimer?.cancel(); _scrollToBottomController.dispose(); _operationsSubscription.cancel(); + + super.dispose(); } @override @@ -150,13 +157,21 @@ class ChatAnimatedListState extends State final builders = context.watch(); final height = MediaQuery.of(context).size.height; - return Stack( + // A trick to avoid the first message being scrolled to the top + int initialScrollIndex = _chatController.messages.length; + double initialAlignment = 1.0; + if (_chatController.messages.length <= 2) { + initialScrollIndex = 0; + initialAlignment = 0.0; + } + + final Widget child = Stack( children: [ ScrollablePositionedList.builder( scrollOffsetController: scrollOffsetController, itemScrollController: itemScrollController, - initialScrollIndex: _chatController.messages.length, - initialAlignment: 1, + initialScrollIndex: initialScrollIndex, + initialAlignment: initialAlignment, scrollOffsetListener: scrollOffsetListener, itemPositionsListener: itemPositionsListener, physics: ClampingScrollPhysics(), @@ -165,7 +180,7 @@ class ChatAnimatedListState extends State itemCount: _chatController.messages.length + 1, itemBuilder: (context, index) { if (index == _chatController.messages.length) { - return VSpace(height - 400); + return VSpace(height - 360); } final message = _chatController.messages[index]; @@ -192,6 +207,8 @@ class ChatAnimatedListState extends State ), ], ); + + return child; } void _scrollLastMessageToTop(Message data) { @@ -202,7 +219,6 @@ class ChatAnimatedListState extends State if (_lastUserMessageIndex != lastUserMessageIndex) { // scroll the current message to the top - Log.info('scrolling the last message to the top'); itemScrollController.scrollTo( index: lastUserMessageIndex, duration: const Duration(milliseconds: 300), @@ -238,9 +254,15 @@ class ChatAnimatedListState extends State // get the max item final sortedItems = itemPositionsListener.itemPositions.value.toList() ..sort((a, b) => a.index.compareTo(b.index)); - final maxItem = sortedItems.last; + final maxItem = sortedItems.lastOrNull; - if (maxItem.index >= _chatController.messages.length - 1) { + if (maxItem == null) { + return; + } + + if (maxItem.index > _chatController.messages.length - 1 || + (maxItem.index == _chatController.messages.length - 1 && + maxItem.itemTrailingEdge <= 1.01)) { _scrollToBottomShowTimer?.cancel(); _scrollToBottomController.reverse(); return; @@ -254,33 +276,31 @@ class ChatAnimatedListState extends State }); } + void _handleLoadPreviousMessages() { + final sortedItems = itemPositionsListener.itemPositions.value.toList() + ..sort((a, b) => a.index.compareTo(b.index)); + final minItem = sortedItems.firstOrNull; + + if (minItem == null || minItem.index > 0 || minItem.itemLeadingEdge < 0) { + return; + } + + _loadPreviousMessagesDebounce.call( + () { + widget.onLoadPreviousMessages?.call(); + }, + ); + } + void _onInserted(final int position, final Message data) { if (position == _oldList.length) { _scrollLastMessageToTop(data); } } - void _onRemoved(final int position, final Message data) { - final visualPosition = max(_oldList.length - position - 1, 0); - _listKey.currentState!.removeItem( - visualPosition, - (context, animation) => widget.itemBuilder( - context, - animation, - data, - isRemoved: true, - ), - duration: widget.removeAnimationDuration, - ); - } + void _onRemoved(final int position, final Message data) {} - void _onChanged(int position, Message oldData, Message newData) { - _onRemoved(position, oldData); - _listKey.currentState!.insertItem( - max(_oldList.length - position - 1, 0), - duration: widget.insertAnimationDuration, - ); - } + void _onChanged(int position, Message oldData, Message newData) {} void _onDiffUpdate(diffutil.DataDiffUpdate update) { update.when( diff --git a/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list_reversed.dart b/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list_reversed.dart index c775a0d338..b425192c3e 100644 --- a/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list_reversed.dart +++ b/frontend/appflowy_flutter/lib/plugins/ai_chat/presentation/animated_chat_list_reversed.dart @@ -168,7 +168,7 @@ class ChatAnimatedListReversedState extends State slivers: [ SliverPadding( padding: EdgeInsets.only( - top: 400, + top: widget.bottomPadding ?? 0, ), ), SliverAnimatedList( diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/file/file_selection_menu.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/file/file_selection_menu.dart index 132a7b8e7e..1a935ed422 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/file/file_selection_menu.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/file/file_selection_menu.dart @@ -1,7 +1,6 @@ -import 'package:flutter/material.dart'; - import 'package:appflowy/plugins/document/presentation/editor_plugins/plugins.dart'; import 'package:appflowy_editor/appflowy_editor.dart'; +import 'package:flutter/material.dart'; extension InsertFile on EditorState { Future insertEmptyFileBlock(GlobalKey key) async { @@ -17,11 +16,18 @@ extension InsertFile on EditorState { } final file = fileNode(url: '')..extraInfos = {'global_key': key}; - final insertedPath = delta.isEmpty ? path : path.next; - final transaction = this.transaction - ..insertNode(insertedPath, file) - ..insertNode(insertedPath, paragraphNode()) - ..afterSelection = Selection.collapsed(Position(path: insertedPath.next)); + final transaction = this.transaction; + + // if current node is a paragraph and empty, replace it with the file block + if (delta.isEmpty && node.type == ParagraphBlockKeys.type) { + final insertedPath = path; + transaction.insertNode(insertedPath, file); + transaction.deleteNode(node); + } else { + // otherwise, insert the file block after the current node + final insertedPath = path.next; + transaction.insertNode(insertedPath, file); + } return apply(transaction); } diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/cover_title.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/cover_title.dart index 2c5062d408..d255ea0e0b 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/cover_title.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/cover_title.dart @@ -52,6 +52,8 @@ class _InnerCoverTitleState extends State<_InnerCoverTitle> { late final titleFocusNode = editorContext.coverTitleFocusNode; int lineCount = 1; + bool updatingViewName = false; + @override void initState() { super.initState(); @@ -87,7 +89,7 @@ class _InnerCoverTitleState extends State<_InnerCoverTitle> { final width = context.read().state.width; return BlocConsumer( listenWhen: (previous, current) => - previous.view.name != current.view.name, + previous.view.name != current.view.name && !updatingViewName, listener: _onListen, builder: (context, state) { final appearance = context.read().state; @@ -206,6 +208,8 @@ class _InnerCoverTitleState extends State<_InnerCoverTitle> { } void _onViewNameChanged() { + updatingViewName = true; + Debounce.debounce( 'update view name', const Duration(milliseconds: 250), @@ -222,6 +226,8 @@ class _InnerCoverTitleState extends State<_InnerCoverTitle> { context .read() ?.add(ViewInfoEvent.titleChanged(titleTextController.text)); + + updatingViewName = false; }, ); } diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/home/desktop_home_screen.dart b/frontend/appflowy_flutter/lib/workspace/presentation/home/desktop_home_screen.dart index 36dfb84586..90b02b2867 100644 --- a/frontend/appflowy_flutter/lib/workspace/presentation/home/desktop_home_screen.dart +++ b/frontend/appflowy_flutter/lib/workspace/presentation/home/desktop_home_screen.dart @@ -23,6 +23,7 @@ import 'package:appflowy_backend/log.dart'; import 'package:appflowy_backend/protobuf/flowy-folder/protobuf.dart'; import 'package:appflowy_backend/protobuf/flowy-user/protobuf.dart' show UserProfilePB; +import 'package:collection/collection.dart'; import 'package:flowy_infra_ui/style_widget/container.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; @@ -116,6 +117,9 @@ class DesktopHomeScreen extends StatelessWidget { TabsEvent.openPlugin(plugin: view.plugin()), ); } + + // switch to the space that contains the last opened view + _switchToSpace(view); } }, child: BlocBuilder( @@ -290,6 +294,16 @@ class DesktopHomeScreen extends StatelessWidget { ], ); } + + Future _switchToSpace(ViewPB view) async { + final ancestors = await ViewBackendService.getViewAncestors(view.id); + final space = ancestors.fold( + (ancestors) => + ancestors.items.firstWhereOrNull((ancestor) => ancestor.isSpace), + (error) => null, + ); + switchToSpaceIdNotifier.value = space; + } } class DesktopHomeScreenStackAdaptor extends HomeStackDelegate { diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/home/hotkeys.dart b/frontend/appflowy_flutter/lib/workspace/presentation/home/hotkeys.dart index c582ee292e..504d1491ff 100644 --- a/frontend/appflowy_flutter/lib/workspace/presentation/home/hotkeys.dart +++ b/frontend/appflowy_flutter/lib/workspace/presentation/home/hotkeys.dart @@ -8,6 +8,7 @@ import 'package:appflowy/workspace/application/sidebar/rename_view/rename_view_b import 'package:appflowy/workspace/application/tabs/tabs_bloc.dart'; import 'package:appflowy/workspace/presentation/home/menu/sidebar/shared/sidebar_setting.dart'; import 'package:appflowy_backend/log.dart'; +import 'package:appflowy_backend/protobuf/flowy-folder/view.pb.dart'; import 'package:appflowy_backend/protobuf/flowy-user/user_profile.pb.dart'; import 'package:flutter/material.dart'; import 'package:hotkey_manager/hotkey_manager.dart'; @@ -18,6 +19,7 @@ typedef KeyDownHandler = void Function(HotKey hotKey); ValueNotifier switchToTheNextSpace = ValueNotifier(0); ValueNotifier createNewPageNotifier = ValueNotifier(0); +ValueNotifier switchToSpaceIdNotifier = ValueNotifier(null); @visibleForTesting final zoomInKeyCodes = [KeyCode.equal, KeyCode.numpadAdd, KeyCode.add]; diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/space/sidebar_space.dart b/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/space/sidebar_space.dart index e4be64d5b9..bba4200ebc 100644 --- a/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/space/sidebar_space.dart +++ b/frontend/appflowy_flutter/lib/workspace/presentation/home/menu/sidebar/space/sidebar_space.dart @@ -73,7 +73,9 @@ class _SpaceState extends State<_Space> { @override void initState() { super.initState(); + switchToTheNextSpace.addListener(_switchToNextSpace); + switchToSpaceIdNotifier.addListener(_switchToSpace); } @override @@ -81,6 +83,7 @@ class _SpaceState extends State<_Space> { switchToTheNextSpace.removeListener(_switchToNextSpace); isHovered.dispose(); isExpandedNotifier.dispose(); + super.dispose(); } @@ -175,4 +178,17 @@ class _SpaceState extends State<_Space> { void _switchToNextSpace() { context.read().add(const SpaceEvent.switchToNextSpace()); } + + void _switchToSpace() { + if (!mounted || !context.mounted) { + return; + } + + final space = switchToSpaceIdNotifier.value; + if (space == null) { + return; + } + + context.read().add(SpaceEvent.open(space)); + } } diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/widgets/more_view_actions/widgets/lock_page_action.dart b/frontend/appflowy_flutter/lib/workspace/presentation/widgets/more_view_actions/widgets/lock_page_action.dart index 202919b639..6ee7b8566a 100644 --- a/frontend/appflowy_flutter/lib/workspace/presentation/widgets/more_view_actions/widgets/lock_page_action.dart +++ b/frontend/appflowy_flutter/lib/workspace/presentation/widgets/more_view_actions/widgets/lock_page_action.dart @@ -109,10 +109,7 @@ class LockPageButtonWrapper extends StatelessWidget { return FlowyTooltip( message: LocaleKeys.lockPage_lockedOperationTooltip.tr(), child: IgnorePointer( - child: Opacity( - opacity: 0.5, - child: child, - ), + child: child, ), ); } diff --git a/frontend/appflowy_flutter/lib/workspace/presentation/widgets/view_title_bar.dart b/frontend/appflowy_flutter/lib/workspace/presentation/widgets/view_title_bar.dart index 3be0973123..13799fbbcd 100644 --- a/frontend/appflowy_flutter/lib/workspace/presentation/widgets/view_title_bar.dart +++ b/frontend/appflowy_flutter/lib/workspace/presentation/widgets/view_title_bar.dart @@ -136,9 +136,11 @@ class ViewTitleBar extends StatelessWidget { ? ViewTitleBehavior.editable // only the last one is editable : ViewTitleBehavior.uneditable, // others are not editable onUpdated: () { - context - .read() - .add(const ViewTitleBarEvent.reload()); + if (context.mounted) { + context + .read() + .add(const ViewTitleBarEvent.reload()); + } }, ), ); diff --git a/frontend/appflowy_flutter/test/unit_test/editor/file_block_test.dart b/frontend/appflowy_flutter/test/unit_test/editor/file_block_test.dart new file mode 100644 index 0000000000..71bff330eb --- /dev/null +++ b/frontend/appflowy_flutter/test/unit_test/editor/file_block_test.dart @@ -0,0 +1,46 @@ +import 'package:appflowy/plugins/document/presentation/editor_plugins/plugins.dart'; +import 'package:appflowy_editor/appflowy_editor.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('FileBlock:', () { + test('insert file block in non-empty paragraph', () async { + final document = Document.blank() + ..insert( + [0], + [paragraphNode(text: 'Hello World')], + ); + final editorState = EditorState(document: document); + editorState.selection = Selection.collapsed(Position(path: [0])); + + // insert file block after the first line + await editorState.insertEmptyFileBlock(GlobalKey()); + + final afterDocument = editorState.document; + expect(afterDocument.root.children.length, 2); + expect(afterDocument.root.children[1].type, FileBlockKeys.type); + expect(afterDocument.root.children[0].type, ParagraphBlockKeys.type); + expect( + afterDocument.root.children[0].delta!.toPlainText(), + 'Hello World', + ); + }); + + test('insert file block in empty paragraph', () async { + final document = Document.blank() + ..insert( + [0], + [paragraphNode(text: '')], + ); + final editorState = EditorState(document: document); + editorState.selection = Selection.collapsed(Position(path: [0])); + + await editorState.insertEmptyFileBlock(GlobalKey()); + + final afterDocument = editorState.document; + expect(afterDocument.root.children.length, 1); + expect(afterDocument.root.children[0].type, FileBlockKeys.type); + }); + }); +}