chore: checklist ux flow redesign (#3418)

* chore: ux flow redesign * chore: remove unused imports * fix: allow creation of tasks of the same name * chore: apply code suggestions from Mathias
2024-08-30 18:12:39 +00:00 · 2023-09-22 09:33:24 +08:00 · 2023-09-22 09:33:24 +08:00 · 3c65a96b04
commit 3c65a96b04
parent 6ba7fc0317
8 changed files with 131 additions and 174 deletions
--- a/frontend/appflowy_flutter/integration_test/database_cell_test.dart
+++ b/frontend/appflowy_flutter/integration_test/database_cell_test.dart
@ -456,9 +456,6 @@ void main() {
    // assert that the checklist editor is shown
    tester.assertChecklistEditorVisible(visible: true);

-    // assert that new task editor is shown
-    tester.assertNewCheckListTaskEditorVisible(visible: true);
-
    // create a new task with enter
    await tester.createNewChecklistTask(name: "task 0", enter: true);

@ -481,7 +478,6 @@ void main() {

    // dismiss new task editor
    await tester.dismissCellEditor();
-    tester.assertNewCheckListTaskEditorVisible(visible: false);

    // dismiss checklist cell editor
    await tester.dismissCellEditor();
@ -489,11 +485,8 @@ void main() {
    // assert that progress bar is shown in grid at 0%
    tester.assertChecklistCellInGrid(rowIndex: 0, percent: 0);

-    // start editing the first checklist cell again, click on new task button
+    // start editing the first checklist cell again
    await tester.tapChecklistCellInGrid(rowIndex: 0);
-    tester.assertNewCheckListTaskEditorVisible(visible: false);
-    await tester.tapChecklistNewTaskButton();
-    tester.assertNewCheckListTaskEditorVisible(visible: true);

    // create another task with the create button
    await tester.createNewChecklistTask(name: "task 2", button: true);
@ -540,9 +533,6 @@ void main() {
    // delete the remaining task
    await tester.deleteChecklistTask(index: 0);

-    // assert that the new task editor is shown
-    tester.assertNewCheckListTaskEditorVisible(visible: true);
-
    // dismiss the cell editor
    await tester.dismissCellEditor();

--- a/frontend/appflowy_flutter/integration_test/util/database_test_op.dart
+++ b/frontend/appflowy_flutter/integration_test/util/database_test_op.dart
@ -46,7 +46,6 @@ import 'package:appflowy/plugins/database_view/widgets/row/cells/date_cell/date_
 import 'package:appflowy/plugins/database_view/widgets/row/cells/select_option_cell/extension.dart';
 import 'package:appflowy/plugins/database_view/widgets/row/cells/select_option_cell/select_option_editor.dart';
 import 'package:appflowy/plugins/database_view/widgets/row/cells/select_option_cell/text_field.dart';
-import 'package:appflowy/plugins/database_view/widgets/row/cells/timestamp_cell/timestamp_cell.dart';
 import 'package:appflowy/plugins/database_view/widgets/row/row_action.dart';
 import 'package:appflowy/plugins/database_view/widgets/row/row_banner.dart';
 import 'package:appflowy/plugins/database_view/widgets/row/row_detail.dart';
@ -494,15 +493,6 @@ extension AppFlowyDatabaseTest on WidgetTester {
    }
  }

-  void assertNewCheckListTaskEditorVisible({required bool visible}) {
-    final editor = find.byType(NewTaskItem);
-    if (visible) {
-      expect(editor, findsOneWidget);
-    } else {
-      expect(editor, findsNothing);
-    }
-  }
-
  Future<void> createNewChecklistTask({
    required String name,
    enter = false,
@ -515,10 +505,10 @@ extension AppFlowyDatabaseTest on WidgetTester {
    );

    await enterText(textField, name);
-    await pumpAndSettle(const Duration(milliseconds: 300));
+    await pumpAndSettle();
    if (enter) {
      await testTextInput.receiveAction(TextInputAction.done);
-      await pumpAndSettle(const Duration(milliseconds: 300));
+      await pumpAndSettle();
    } else {
      await tapButton(
        find.descendant(
@ -555,11 +545,7 @@ extension AppFlowyDatabaseTest on WidgetTester {

    await enterText(textField, name);
    await testTextInput.receiveAction(TextInputAction.done);
-    await pumpAndSettle(const Duration(milliseconds: 300));
-  }
-
-  Future<void> tapChecklistNewTaskButton() async {
-    await tapButton(find.byType(ChecklistNewTaskButton));
+    await pumpAndSettle();
  }

  Future<void> checkChecklistTask({required int index}) async {
--- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cell_builder.dart
+++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cell_builder.dart
@ -67,6 +67,7 @@ class GridCellBuilder {
      case FieldType.Checklist:
        return GridChecklistCell(
          cellControllerBuilder: cellControllerBuilder,
+          style: style,
          key: key,
        );
      case FieldType.Number:
--- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/cells.dart
+++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/cells.dart
@ -4,4 +4,5 @@ export 'date_cell/date_cell.dart';
 export 'number_cell/number_cell.dart';
 export 'select_option_cell/select_option_cell.dart';
 export 'text_cell/text_cell.dart';
+export 'timestamp_cell/timestamp_cell.dart';
 export 'url_cell/url_cell.dart';
--- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/checklist_cell/checklist_cell.dart
+++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/checklist_cell/checklist_cell.dart
@ -10,10 +10,26 @@ import 'checklist_cell_bloc.dart';
 import 'checklist_cell_editor.dart';
 import 'checklist_progress_bar.dart';

+class ChecklistCellStyle extends GridCellStyle {
+  String placeholder;
+  EdgeInsets? cellPadding;
+
+  ChecklistCellStyle({
+    required this.placeholder,
+    this.cellPadding,
+  });
+}
+
 class GridChecklistCell extends GridCellWidget {
  final CellControllerBuilder cellControllerBuilder;
-  GridChecklistCell({required this.cellControllerBuilder, Key? key})
-      : super(key: key);
+  late final ChecklistCellStyle? cellStyle;
+  GridChecklistCell({
+    required this.cellControllerBuilder,
+    GridCellStyle? style,
+    super.key,
+  }) {
+    cellStyle = style as ChecklistCellStyle?;
+  }

  @override
  GridCellState<GridChecklistCell> createState() => GridChecklistCellState();
@ -53,15 +69,22 @@ class GridChecklistCellState extends GridCellState<GridChecklistCell> {
          );
        },
        onClose: () => widget.onCellFocus.value = false,
-        child: Padding(
-          padding: GridSize.cellContentInsets,
-          child: BlocBuilder<ChecklistCardCellBloc, ChecklistCellState>(
-            builder: (context, state) {
-              if (state.allOptions.isEmpty) {
-                return const SizedBox.shrink();
-              }
-              return ChecklistProgressBar(percent: state.percent);
-            },
+        child: Align(
+          alignment: Alignment.centerLeft,
+          child: Padding(
+            padding:
+                widget.cellStyle?.cellPadding ?? GridSize.cellContentInsets,
+            child: BlocBuilder<ChecklistCardCellBloc, ChecklistCellState>(
+              builder: (context, state) {
+                if (state.allOptions.isEmpty) {
+                  return FlowyText.medium(
+                    widget.cellStyle?.placeholder ?? "",
+                    color: Theme.of(context).hintColor,
+                  );
+                }
+                return ChecklistProgressBar(percent: state.percent);
+              },
+            ),
          ),
        ),
      ),
--- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/checklist_cell/checklist_cell_editor.dart
+++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/checklist_cell/checklist_cell_editor.dart
@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:appflowy/generated/flowy_svgs.g.dart';
 import 'package:appflowy/generated/locale_keys.g.dart';
 import 'package:appflowy/plugins/database_view/application/cell/cell_controller_builder.dart';
@ -30,13 +32,19 @@ class _GridChecklistCellEditorState extends State<GridChecklistCellEditor> {
  /// Focus node for the new task text field
  late final FocusNode newTaskFocusNode;

-  /// A flag that determines whether the new task text field is visible
-  bool _isAddingNewTask = false;
-
  @override
  void initState() {
    super.initState();
-    newTaskFocusNode = FocusNode();
+    newTaskFocusNode = FocusNode(
+      onKey: (node, event) {
+        if (event is RawKeyDownEvent &&
+            event.logicalKey == LogicalKeyboardKey.escape) {
+          node.unfocus();
+          return KeyEventResult.handled;
+        }
+        return KeyEventResult.ignored;
+      },
+    );
    _bloc = ChecklistCellEditorBloc(cellController: widget.cellController)
      ..add(const ChecklistCellEditorEvent.initial());
  }
@ -48,59 +56,35 @@ class _GridChecklistCellEditorState extends State<GridChecklistCellEditor> {
      child: BlocConsumer<ChecklistCellEditorBloc, ChecklistCellEditorState>(
        listener: (context, state) {
          if (state.allOptions.isEmpty) {
-            setState(() => _isAddingNewTask = true);
+            newTaskFocusNode.requestFocus();
          }
        },
        builder: (context, state) {
-          return Focus(
-            onKey: (node, event) {
-              // don't hide new task text field if there are no tasks at all
-              if (state.allOptions.isNotEmpty &&
-                  event is RawKeyDownEvent &&
-                  event.logicalKey == LogicalKeyboardKey.escape) {
-                setState(() {
-                  _isAddingNewTask = false;
-                });
-                return KeyEventResult.handled;
-              }
-              return KeyEventResult.ignored;
-            },
-            child: CustomScrollView(
-              shrinkWrap: true,
-              physics: StyledScrollPhysics(),
-              slivers: [
-                SliverToBoxAdapter(
-                  child: AnimatedSwitcher(
-                    duration: const Duration(milliseconds: 300),
-                    child: state.allOptions.isEmpty
-                        ? const SizedBox.shrink()
-                        : Padding(
-                            padding: const EdgeInsets.fromLTRB(16, 16, 16, 4),
-                            child: ChecklistProgressBar(
-                              percent: state.percent,
-                            ),
-                          ),
-                  ),
-                ),
-                ChecklistItemList(
-                  options: state.allOptions,
-                  newTaskFocusNode: newTaskFocusNode,
-                  isAddingNewTask: _isAddingNewTask,
-                  onUpdateTask: () => setState(() {
-                    _isAddingNewTask = true;
-                    newTaskFocusNode.requestFocus();
-                  }),
-                ),
-                const SliverToBoxAdapter(
-                  child: TypeOptionSeparator(spacing: 0.0),
-                ),
-                SliverToBoxAdapter(
-                  child: ChecklistNewTaskButton(
-                    onTap: () => setState(() => _isAddingNewTask = true),
-                  ),
-                ),
-              ],
-            ),
+          return Column(
+            mainAxisSize: MainAxisSize.min,
+            children: [
+              AnimatedSwitcher(
+                duration: const Duration(milliseconds: 300),
+                child: state.allOptions.isEmpty
+                    ? const SizedBox.shrink()
+                    : Padding(
+                        padding: const EdgeInsets.fromLTRB(16, 16, 16, 4),
+                        child: ChecklistProgressBar(
+                          percent: state.percent,
+                        ),
+                      ),
+              ),
+              ChecklistItemList(
+                options: state.allOptions,
+                onUpdateTask: () => newTaskFocusNode.requestFocus(),
+              ),
+              if (state.allOptions.isNotEmpty)
+                const TypeOptionSeparator(spacing: 0.0),
+              Padding(
+                padding: const EdgeInsets.symmetric(vertical: 8),
+                child: NewTaskItem(focusNode: newTaskFocusNode),
+              ),
+            ],
          );
        },
      ),
@ -118,16 +102,12 @@ class _GridChecklistCellEditorState extends State<GridChecklistCellEditor> {
 /// a new task if `isAddingNewTask` is true
 class ChecklistItemList extends StatefulWidget {
  final List<ChecklistSelectOption> options;
-  final FocusNode newTaskFocusNode;
-  final bool isAddingNewTask;
  final VoidCallback onUpdateTask;

  const ChecklistItemList({
    super.key,
    required this.options,
    required this.onUpdateTask,
-    required this.isAddingNewTask,
-    required this.newTaskFocusNode,
  });

  @override
@ -137,32 +117,28 @@ class ChecklistItemList extends StatefulWidget {
 class _ChecklistItemListState extends State<ChecklistItemList> {
  @override
  Widget build(BuildContext context) {
-    final itemList = [
-      const VSpace(6.0),
-      ...widget.options.mapIndexed(
-        (index, option) => Padding(
-          padding: const EdgeInsets.symmetric(vertical: 2),
-          child: ChecklistItem(
+    if (widget.options.isEmpty) {
+      return const SizedBox.shrink();
+    }
+
+    final itemList = widget.options
+        .mapIndexed(
+          (index, option) => ChecklistItem(
            option: option,
            onSubmitted:
                index == widget.options.length - 1 ? widget.onUpdateTask : null,
            key: ValueKey(option.data.id),
-            // only allow calling the callback for the last task in the list
          ),
-        ),
-      ),
-      AnimatedSwitcher(
-        duration: const Duration(milliseconds: 300),
-        child: widget.isAddingNewTask
-            ? NewTaskItem(focusNode: widget.newTaskFocusNode)
-            : const SizedBox.shrink(),
-      ),
-      const VSpace(6.0),
-    ];
-    return SliverList(
-      delegate: SliverChildBuilderDelegate(
-        (BuildContext context, int index) => itemList[index],
-        childCount: itemList.length,
+        )
+        .toList();
+
+    return Flexible(
+      child: ListView.separated(
+        itemBuilder: (context, index) => itemList[index],
+        separatorBuilder: (context, index) => const VSpace(4),
+        itemCount: itemList.length,
+        shrinkWrap: true,
+        padding: const EdgeInsets.symmetric(vertical: 8.0),
      ),
    );
  }
@ -187,6 +163,7 @@ class _ChecklistItemState extends State<ChecklistItem> {
  late final TextEditingController _textController;
  late final FocusNode _focusNode;
  bool _isHovered = false;
+  Timer? _debounceOnChanged;

  @override
  void initState() {
@ -249,13 +226,9 @@ class _ChecklistItemState extends State<ChecklistItem> {
                    ),
                    hintText: LocaleKeys.grid_checklist_taskHint.tr(),
                  ),
-                  onSubmitted: (taskDescription) {
-                    context.read<ChecklistCellEditorBloc>().add(
-                          ChecklistCellEditorEvent.updateTaskName(
-                            widget.option.data,
-                            taskDescription,
-                          ),
-                        );
+                  onChanged: _debounceOnChangedText,
+                  onSubmitted: (description) {
+                    _submitUpdateTaskDescription(description);
                    widget.onSubmitted?.call();
                  },
                ),
@ -276,6 +249,22 @@ class _ChecklistItemState extends State<ChecklistItem> {
      ),
    );
  }
+
+  void _debounceOnChangedText(String text) {
+    _debounceOnChanged?.cancel();
+    _debounceOnChanged = Timer(const Duration(milliseconds: 300), () {
+      _submitUpdateTaskDescription(text);
+    });
+  }
+
+  void _submitUpdateTaskDescription(String description) {
+    context.read<ChecklistCellEditorBloc>().add(
+          ChecklistCellEditorEvent.updateTaskName(
+            widget.option.data,
+            description,
+          ),
+        );
+  }
 }

 /// Creates a new task after entering the description and pressing enter.
@ -304,19 +293,12 @@ class _NewTaskItemState extends State<NewTaskItem> {
  @override
  Widget build(BuildContext context) {
    return Container(
-      padding: const EdgeInsets.symmetric(horizontal: 8.0),
+      padding: const EdgeInsets.symmetric(horizontal: 8),
      constraints: BoxConstraints(minHeight: GridSize.popoverItemHeight),
      child: Row(
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
-          const FlowyIconButton(
-            width: 32,
-            icon: FlowySvg(
-              FlowySvgs.uncheck_s,
-              blendMode: BlendMode.dst,
-            ),
-            hoverColor: Colors.transparent,
-          ),
+          const HSpace(8),
          Expanded(
            child: TextField(
              focusNode: widget.focusNode,
@ -330,7 +312,7 @@ class _NewTaskItemState extends State<NewTaskItem> {
                  vertical: 6.0,
                  horizontal: 2.0,
                ),
-                hintText: LocaleKeys.grid_checklist_taskHint.tr(),
+                hintText: LocaleKeys.grid_checklist_addNew.tr(),
              ),
              onSubmitted: (taskDescription) {
                if (taskDescription.trim().isNotEmpty) {
@ -340,15 +322,21 @@ class _NewTaskItemState extends State<NewTaskItem> {
                        ),
                      );
                }
+                widget.focusNode.requestFocus();
                _textEditingController.clear();
              },
+              onChanged: (value) => setState(() {}),
            ),
          ),
          FlowyTextButton(
            LocaleKeys.grid_checklist_submitNewTask.tr(),
            fontSize: 11,
-            fillColor: Theme.of(context).colorScheme.primary,
-            hoverColor: Theme.of(context).colorScheme.primaryContainer,
+            fillColor: _textEditingController.text.isEmpty
+                ? Theme.of(context).disabledColor
+                : Theme.of(context).colorScheme.primary,
+            hoverColor: _textEditingController.text.isEmpty
+                ? Theme.of(context).disabledColor
+                : Theme.of(context).colorScheme.primaryContainer,
            fontColor: Theme.of(context).colorScheme.onPrimary,
            padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 3),
            onPressed: () {
@ -359,6 +347,7 @@ class _NewTaskItemState extends State<NewTaskItem> {
                      ),
                    );
              }
+              widget.focusNode.requestFocus();
              _textEditingController.clear();
            },
          ),
@ -367,25 +356,3 @@ class _NewTaskItemState extends State<NewTaskItem> {
    );
  }
 }
-
-@visibleForTesting
-class ChecklistNewTaskButton extends StatelessWidget {
-  final VoidCallback onTap;
-  const ChecklistNewTaskButton({super.key, required this.onTap});
-
-  @override
-  Widget build(BuildContext context) {
-    return Padding(
-      padding: const EdgeInsets.symmetric(horizontal: 8.0, vertical: 8.0),
-      child: SizedBox(
-        height: 30,
-        child: FlowyButton(
-          text: FlowyText.medium(LocaleKeys.grid_checklist_addNew.tr()),
-          margin: const EdgeInsets.all(6),
-          leftIcon: const FlowySvg(FlowySvgs.add_s),
-          onTap: onTap,
-        ),
-      ),
-    );
-  }
-}
--- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/row_property.dart
+++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/row_property.dart
@ -6,6 +6,7 @@ import 'package:appflowy/plugins/database_view/application/field/type_option/typ
 import 'package:appflowy/plugins/database_view/grid/application/row/row_detail_bloc.dart';
 import 'package:appflowy/plugins/database_view/grid/presentation/widgets/header/field_cell.dart';
 import 'package:appflowy/plugins/database_view/grid/presentation/widgets/header/field_editor.dart';
+import 'package:appflowy/plugins/database_view/widgets/row/cells/cells.dart';
 import 'package:appflowy/workspace/presentation/widgets/dialogs.dart';
 import 'package:appflowy_backend/log.dart';
 import 'package:appflowy_backend/protobuf/flowy-database2/field_entities.pb.dart';
@ -19,13 +20,6 @@ import 'package:flutter_bloc/flutter_bloc.dart';

 import 'accessory/cell_accessory.dart';
 import 'cell_builder.dart';
-import 'cells/checkbox_cell/checkbox_cell.dart';
-import 'cells/date_cell/date_cell.dart';
-import 'cells/number_cell/number_cell.dart';
-import 'cells/select_option_cell/select_option_cell.dart';
-import 'cells/text_cell/text_cell.dart';
-import 'cells/timestamp_cell/timestamp_cell.dart';
-import 'cells/url_cell/url_cell.dart';

 /// Display the row properties in a list. Only use this widget in the
 /// [RowDetailPage].
@ -251,8 +245,9 @@ GridCellStyle? _customCellStyle(FieldType fieldType) {
        cellPadding: const EdgeInsets.symmetric(horizontal: 8, vertical: 5),
      );
    case FieldType.Checklist:
-      return SelectOptionCellStyle(
+      return ChecklistCellStyle(
        placeholder: LocaleKeys.grid_row_textPlaceholder.tr(),
+        cellPadding: const EdgeInsets.symmetric(horizontal: 8, vertical: 6),
      );
    case FieldType.Number:
      return GridNumberCellStyle(
--- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist.rs
+++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist.rs
@ -87,7 +87,7 @@ impl CellDataChangeset for ChecklistTypeOption {
 #[inline]
 fn update_cell_data_with_changeset(
  cell_data: &mut ChecklistCellData,
-  mut changeset: ChecklistCellChangeset,
+  changeset: ChecklistCellChangeset,
 ) {
  // Delete the options
  cell_data
@ -98,12 +98,6 @@ fn update_cell_data_with_changeset(
    .retain(|option_id| !changeset.delete_option_ids.contains(option_id));

  // Insert new options
-  changeset.insert_options.retain(|option_name| {
-    !cell_data
-      .options
-      .iter()
-      .any(|option| option.name == *option_name)
-  });
  changeset
    .insert_options
    .into_iter()