From ea670b6ae6f20d4dd58f0cbfb2f06babff18da87 Mon Sep 17 00:00:00 2001 From: Ahad Patel <69256193+Ahad-patel@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:32:05 +0530 Subject: [PATCH] fix: grid new row getting cut (#6475) * fix: Grid new row getting cut * chore: add test case for testing text in veiwport * refac: clean up code --------- Co-authored-by: Mathias Mogensen Co-authored-by: Lucas.Xu --- .../desktop/database/database_field_test.dart | 29 +++- .../database/grid/presentation/grid_page.dart | 127 ++++++++++-------- .../widgets/footer/grid_footer.dart | 3 +- 3 files changed, 100 insertions(+), 59 deletions(-) diff --git a/frontend/appflowy_flutter/integration_test/desktop/database/database_field_test.dart b/frontend/appflowy_flutter/integration_test/desktop/database/database_field_test.dart index 8cfe024008..b2871ffd31 100644 --- a/frontend/appflowy_flutter/integration_test/desktop/database/database_field_test.dart +++ b/frontend/appflowy_flutter/integration_test/desktop/database/database_field_test.dart @@ -1,11 +1,12 @@ +import 'package:flutter/material.dart'; + import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/plugins/database/grid/presentation/grid_page.dart'; import 'package:appflowy/plugins/database/widgets/field/type_option_editor/select/select_option.dart'; import 'package:appflowy/util/field_type_extension.dart'; -import 'package:appflowy_backend/protobuf/flowy-database2/field_entities.pbenum.dart'; +import 'package:appflowy_backend/protobuf/flowy-database2/protobuf.dart'; import 'package:appflowy_backend/protobuf/flowy-folder/protobuf.dart'; import 'package:easy_localization/easy_localization.dart'; -import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; @@ -316,6 +317,30 @@ void main() { ); }); + testWidgets('text in viewport while typing', (tester) async { + await tester.initializeAppFlowy(); + await tester.tapAnonymousSignInButton(); + + await tester.createNewPageWithNameUnderParent(layout: ViewLayoutPB.Grid); + + await tester.changeCalculateAtIndex(0, CalculationType.Count); + + // add very large text with 200 lines + final largeText = List.generate( + 200, + (index) => 'Line ${index + 1}', + ).join('\n'); + + await tester.editCell( + rowIndex: 2, + fieldType: FieldType.RichText, + input: largeText, + ); + + // checks if last line is in view port + tester.expectToSeeText('Line 200'); + }); + // Disable this test because it fails on CI randomly // testWidgets('last modified and created at field type options', // (tester) async { diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/grid_page.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/grid_page.dart index 954277a49f..a2ae293187 100755 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/grid_page.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/grid_page.dart @@ -1,6 +1,5 @@ import 'dart:math'; -import 'package:flowy_infra/theme_extension.dart'; import 'package:flutter/material.dart'; import 'package:appflowy/generated/locale_keys.g.dart'; @@ -18,6 +17,7 @@ import 'package:appflowy/workspace/presentation/widgets/dialogs.dart'; import 'package:appflowy_backend/log.dart'; import 'package:appflowy_backend/protobuf/flowy-folder/view.pb.dart'; import 'package:easy_localization/easy_localization.dart'; +import 'package:flowy_infra/theme_extension.dart'; import 'package:flowy_infra_ui/flowy_infra_ui.dart'; import 'package:flowy_infra_ui/style_widget/scrolling/styled_scrollview.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; @@ -292,21 +292,41 @@ class _GridRows extends StatefulWidget { class _GridRowsState extends State<_GridRows> { bool showFloatingCalculations = false; + bool isAtBottom = false; @override void initState() { super.initState(); _evaluateFloatingCalculations(); + widget.scrollController.verticalController.addListener(_onScrollChanged); + } + + void _onScrollChanged() { + final controller = widget.scrollController.verticalController; + final isAtBottom = controller.position.atEdge && controller.offset > 0 || + controller.offset >= controller.position.maxScrollExtent - 1; + if (isAtBottom != this.isAtBottom) { + setState(() => this.isAtBottom = isAtBottom); + } + } + + @override + void dispose() { + widget.scrollController.verticalController.removeListener(_onScrollChanged); + super.dispose(); } void _evaluateFloatingCalculations() { WidgetsBinding.instance.addPostFrameCallback((_) { if (mounted) { setState(() { + final verticalController = widget.scrollController.verticalController; // maxScrollExtent is 0.0 if scrolling is not possible - showFloatingCalculations = widget.scrollController.verticalController - .position.maxScrollExtent > - 0; + showFloatingCalculations = + verticalController.position.maxScrollExtent > 0; + + isAtBottom = verticalController.position.atEdge && + verticalController.offset > 0; }); } }); @@ -356,11 +376,11 @@ class _GridRowsState extends State<_GridRows> { ) { // 1. GridRowBottomBar // 2. GridCalculationsRow - // 3. Footer Padding - final itemCount = state.rowInfos.length + 3; - return Stack( + final itemCount = + state.rowInfos.length + (showFloatingCalculations ? 1 : 2); + return Column( children: [ - Positioned.fill( + Expanded( child: ReorderableListView.builder( /// This is a workaround related to /// https://github.com/flutter/flutter/issues/25652 @@ -408,38 +428,31 @@ class _GridRowsState extends State<_GridRows> { }, itemCount: itemCount, itemBuilder: (context, index) { - if (index < state.rowInfos.length) { - return _renderRow( - context, - state.rowInfos[index].rowId, - index: index, - ); - } - if (index == state.rowInfos.length) { return const GridRowBottomBar(key: Key('grid_footer')); } - if (index == state.rowInfos.length + 1) { - if (showFloatingCalculations) { - return const SizedBox( - key: Key('calculations_bottom_padding'), - height: 36, - ); - } else { - return GridCalculationsRow( - key: const Key('grid_calculations'), - viewId: widget.viewId, - ); - } + if (index == state.rowInfos.length + 1 && + !showFloatingCalculations) { + return GridCalculationsRow( + key: const Key('grid_calculations'), + viewId: widget.viewId, + ); } - return const SizedBox(key: Key('footer_padding'), height: 10); + return _renderRow( + context, + state.rowInfos[index].rowId, + index: index, + ); }, ), ), if (showFloatingCalculations) ...[ - _PositionedCalculationsRow(viewId: widget.viewId), + _PositionedCalculationsRow( + viewId: widget.viewId, + isAtBottom: isAtBottom, + ), ], ], ); @@ -542,10 +555,16 @@ class _WrapScrollView extends StatelessWidget { class _PositionedCalculationsRow extends StatefulWidget { const _PositionedCalculationsRow({ required this.viewId, + this.isAtBottom = false, }); final String viewId; + /// We don't need to show the top border if the scroll offset + /// is at the bottom of the ScrollView. + /// + final bool isAtBottom; + @override State<_PositionedCalculationsRow> createState() => _PositionedCalculationsRowState(); @@ -555,32 +574,28 @@ class _PositionedCalculationsRowState extends State<_PositionedCalculationsRow> { @override Widget build(BuildContext context) { - return Positioned( - bottom: 0, - left: 0, - right: 0, - child: Container( - margin: EdgeInsets.only( - left: - context.read().horizontalPadding, - ), - padding: const EdgeInsets.only(bottom: 10), - decoration: BoxDecoration( - color: Theme.of(context).canvasColor, - border: Border( - top: BorderSide( - color: AFThemeExtension.of(context).borderColor, - ), - ), - ), - child: SizedBox( - height: 36, - width: double.infinity, - child: GridCalculationsRow( - key: const Key('floating_grid_calculations'), - viewId: widget.viewId, - includeDefaultInsets: false, - ), + return Container( + margin: EdgeInsets.only( + left: context.read().horizontalPadding, + ), + padding: const EdgeInsets.only(bottom: 10), + decoration: BoxDecoration( + color: Theme.of(context).canvasColor, + border: widget.isAtBottom + ? null + : Border( + top: BorderSide( + color: AFThemeExtension.of(context).borderColor, + ), + ), + ), + child: SizedBox( + height: 36, + width: double.infinity, + child: GridCalculationsRow( + key: const Key('floating_grid_calculations'), + viewId: widget.viewId, + includeDefaultInsets: false, ), ), ); diff --git a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/footer/grid_footer.dart b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/footer/grid_footer.dart index 65b713bd35..219b82e027 100755 --- a/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/footer/grid_footer.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/grid/presentation/widgets/footer/grid_footer.dart @@ -1,3 +1,5 @@ +import 'package:flutter/material.dart'; + import 'package:appflowy/generated/flowy_svgs.g.dart'; import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/plugins/database/grid/application/grid_bloc.dart'; @@ -7,7 +9,6 @@ import 'package:easy_localization/easy_localization.dart'; import 'package:flowy_infra/theme_extension.dart'; import 'package:flowy_infra_ui/style_widget/button.dart'; import 'package:flowy_infra_ui/style_widget/text.dart'; -import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; class GridAddRowButton extends StatelessWidget {