From 5fa441cbf5a8462b0f0602d76d702b0e947dfa1a Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Mon, 4 Dec 2023 10:22:26 +0800 Subject: [PATCH] fix: launch review issues (#4075) * chore: add a tooltip for fields in row detail page * fix: grouping by field makes cell contents disappear * chore: code cleanup * chore: env var values in launch.json should be string * fix: group orders not being saved * test: fix test * chore: more code cleanup * fix: field settings not found * chore: ellide cell text * fix: alignment issues in row detail page --- frontend/.vscode/launch.json | 2 +- .../application/cell/cell_service.dart | 5 +- .../application/field/field_controller.dart | 7 ++ .../board/presentation/board_page.dart | 14 +-- .../presentation/widgets/row/mobile_row.dart | 2 +- .../grid/presentation/widgets/row/row.dart | 2 +- .../widgets/card/cells/date_card_cell.dart | 1 + .../row/cells/date_cell/date_cell.dart | 6 +- .../row/cells/number_cell/number_cell.dart | 40 +++--- .../row/cells/text_cell/text_cell.dart | 103 +++++++-------- .../widgets/row/cells/url_cell/url_cell.dart | 119 +++++------------- .../widgets/row/row_property.dart | 22 +++- .../select_type_option.rs | 15 +-- .../src/services/group/configuration.rs | 15 +-- .../single_select_controller.rs | 5 +- .../tests/database/group_test/test.rs | 2 +- 16 files changed, 146 insertions(+), 214 deletions(-) diff --git a/frontend/.vscode/launch.json b/frontend/.vscode/launch.json index c21e232410..83b2e889e3 100644 --- a/frontend/.vscode/launch.json +++ b/frontend/.vscode/launch.json @@ -25,7 +25,7 @@ "preLaunchTask": "AF: Build Appflowy Core", "env": { "RUST_LOG": "trace", - "RUST_BACKTRACE": 1 + "RUST_BACKTRACE": "1" }, "cwd": "${workspaceRoot}/appflowy_flutter" }, diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/application/cell/cell_service.dart b/frontend/appflowy_flutter/lib/plugins/database_view/application/cell/cell_service.dart index f73d4be332..6ea4fe6cdb 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/application/cell/cell_service.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/application/cell/cell_service.dart @@ -79,8 +79,7 @@ class DatabaseCellContext with _$DatabaseCellContext { /// It will be visible when the field is not hidden or when hidden fields /// should be shown. bool isVisible({bool showHiddenFields = false}) { - return fieldInfo.visibility != null && - (showHiddenFields || - fieldInfo.visibility != FieldVisibility.AlwaysHidden); + return showHiddenFields || + fieldInfo.visibility != FieldVisibility.AlwaysHidden; } } diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/application/field/field_controller.dart b/frontend/appflowy_flutter/lib/plugins/database_view/application/field/field_controller.dart index 4c660dc25c..4cdeb789a8 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/application/field/field_controller.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/application/field/field_controller.dart @@ -382,6 +382,13 @@ class FieldController { final updatedFieldInfo = fieldInfo.copyWith(fieldSettings: fieldSettings); + final index = _fieldSettings + .indexWhere((element) => element.fieldId == fieldInfo.id); + if (index != -1) { + _fieldSettings.removeAt(index); + } + _fieldSettings.add(fieldSettings); + return updatedFieldInfo; }); } diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/board/presentation/board_page.dart b/frontend/appflowy_flutter/lib/plugins/database_view/board/presentation/board_page.dart index fc30408ea6..3a3c0ced4c 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/board/presentation/board_page.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/board/presentation/board_page.dart @@ -127,9 +127,12 @@ class _DesktopBoardContentState extends State { late final AppFlowyBoardScrollController scrollManager; final config = const AppFlowyBoardConfig( - groupBackgroundColor: Color(0xffF7F8FC), + groupMargin: EdgeInsets.symmetric(horizontal: 4), + groupBodyPadding: EdgeInsets.symmetric(horizontal: 4), + groupFooterPadding: EdgeInsets.fromLTRB(4, 14, 4, 4), groupHeaderPadding: EdgeInsets.symmetric(horizontal: 8), cardMargin: EdgeInsets.symmetric(horizontal: 4, vertical: 3), + stretchGroupHeight: false, ); @override @@ -169,12 +172,7 @@ class _DesktopBoardContentState extends State { scrollController: scrollController, controller: context.read().boardController, groupConstraints: const BoxConstraints.tightFor(width: 256), - config: const AppFlowyBoardConfig( - groupMargin: EdgeInsets.symmetric(horizontal: 4), - groupBodyPadding: EdgeInsets.symmetric(horizontal: 4), - groupFooterPadding: EdgeInsets.fromLTRB(4, 14, 4, 4), - stretchGroupHeight: false, - ), + config: config, leading: HiddenGroupsColumn(margin: config.groupHeaderPadding), trailing: showCreateGroupButton ? BoardTrailing(scrollController: scrollController) @@ -248,7 +246,7 @@ class _DesktopBoardContentState extends State { final isEditing = boardBloc.state.isEditingRow && boardBloc.state.editingRow?.row.id == groupItem.row.id; - final groupItemId = groupItem.row.id + groupData.group.groupId; + final groupItemId = "${groupData.group.groupId}${groupItem.row.id}"; return AppFlowyGroupCard( key: ValueKey(groupItemId), diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/mobile_row.dart b/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/mobile_row.dart index 640964fbd5..cb283a6434 100755 --- a/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/mobile_row.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/mobile_row.dart @@ -153,7 +153,7 @@ class RowContent extends StatelessWidget { final GridCellWidget child = builder.build(cellId); return MobileCellContainer( - width: cellId.fieldInfo.fieldSettings?.width.toDouble() ?? 140, + width: cellId.fieldInfo.fieldSettings!.width.toDouble(), isPrimary: cellId.fieldInfo.field.isPrimary, accessoryBuilder: (buildContext) { return []; diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/row.dart b/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/row.dart index 711a111646..24fc59fa84 100755 --- a/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/row.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/grid/presentation/widgets/row/row.dart @@ -268,7 +268,7 @@ class RowContent extends StatelessWidget { (cellId) { final GridCellWidget child = builder.build(cellId); return CellContainer( - width: cellId.fieldInfo.fieldSettings?.width.toDouble() ?? 140, + width: cellId.fieldInfo.fieldSettings!.width.toDouble(), isPrimary: cellId.fieldInfo.field.isPrimary, accessoryBuilder: (buildContext) { final builder = child.accessoryBuilder; diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/card/cells/date_card_cell.dart b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/card/cells/date_card_cell.dart index afbc6b4a8c..d2e1d9c4d7 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/card/cells/date_card_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/card/cells/date_card_cell.dart @@ -61,6 +61,7 @@ class _DateCellState extends State { state.dateStr, fontSize: 11, color: Theme.of(context).hintColor, + overflow: TextOverflow.ellipsis, ), ), ); diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/date_cell/date_cell.dart b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/date_cell/date_cell.dart index 984b66e586..198b0e6ea2 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/date_cell/date_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/date_cell/date_cell.dart @@ -85,7 +85,11 @@ class _DateCellState extends GridCellState { child: Container( alignment: alignment, padding: padding, - child: FlowyText.medium(text, color: color), + child: FlowyText.medium( + text, + color: color, + overflow: TextOverflow.ellipsis, + ), ), popupBuilder: (BuildContext popoverContent) { return DateCellEditor( diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/number_cell/number_cell.dart b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/number_cell/number_cell.dart index 8fe810831a..b8aa6d7795 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/number_cell/number_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/number_cell/number_cell.dart @@ -67,28 +67,26 @@ class _NumberCellState extends GridEditableTextCell { listener: (context, state) => _controller.text = state.cellContent, ), ], - child: Padding( - padding: GridSize.cellContentInsets, - child: TextField( - controller: _controller, - focusNode: focusNode, - onEditingComplete: () => focusNode.unfocus(), - onSubmitted: (_) => focusNode.unfocus(), - maxLines: null, - style: Theme.of(context).textTheme.bodyMedium, - textInputAction: TextInputAction.done, - decoration: InputDecoration( - contentPadding: EdgeInsets.zero, - border: InputBorder.none, - focusedBorder: InputBorder.none, - enabledBorder: InputBorder.none, - errorBorder: InputBorder.none, - disabledBorder: InputBorder.none, - hintText: widget.cellStyle.placeholder, - isDense: true, - ), - onTapOutside: (_) => focusNode.unfocus(), + child: TextField( + controller: _controller, + focusNode: focusNode, + onEditingComplete: () => focusNode.unfocus(), + onSubmitted: (_) => focusNode.unfocus(), + maxLines: null, + style: Theme.of(context).textTheme.bodyMedium, + textInputAction: TextInputAction.done, + decoration: InputDecoration( + contentPadding: + widget.cellStyle.cellPadding ?? GridSize.cellContentInsets, + border: InputBorder.none, + focusedBorder: InputBorder.none, + enabledBorder: InputBorder.none, + errorBorder: InputBorder.none, + disabledBorder: InputBorder.none, + hintText: widget.cellStyle.placeholder, + isDense: true, ), + onTapOutside: (_) => focusNode.unfocus(), ), ), ); diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/text_cell/text_cell.dart b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/text_cell/text_cell.dart index 255915bd6d..f6a1cf147b 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/text_cell/text_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/text_cell/text_cell.dart @@ -77,65 +77,58 @@ class _GridTextCellState extends GridEditableTextCell { _controller.text = state.content; } }, - child: Padding( - padding: widget.cellStyle.cellPadding ?? - EdgeInsets.only( - left: GridSize.cellContentInsets.left, - right: GridSize.cellContentInsets.right, - ), - child: Row( - children: [ - if (widget.cellStyle.showEmoji) - // Only build the emoji when it changes - BlocBuilder( - buildWhen: (p, c) => p.emoji != c.emoji, - builder: (context, state) => Center( - child: FlowyText( - state.emoji, - fontSize: widget.cellStyle.emojiFontSize, - ), + child: Row( + children: [ + if (widget.cellStyle.showEmoji) ...[ + // Only build the emoji when it changes + BlocBuilder( + buildWhen: (p, c) => p.emoji != c.emoji, + builder: (context, state) => Center( + child: FlowyText( + state.emoji, + fontSize: widget.cellStyle.emojiFontSize, ), ), - HSpace(widget.cellStyle.emojiHPadding), - Expanded( - child: widget.cellStyle.useRoundedBorder - ? FlowyTextField( - controller: _controller, - textStyle: widget.cellStyle.textStyle ?? - Theme.of(context).textTheme.bodyMedium, - focusNode: focusNode, - autoFocus: widget.cellStyle.autofocus, - hintText: widget.cellStyle.placeholder, - onChanged: (text) => _cellBloc.add( - TextCellEvent.updateText(text), - ), - debounceDuration: const Duration(milliseconds: 300), - ) - : TextField( - controller: _controller, - focusNode: focusNode, - maxLines: null, - style: widget.cellStyle.textStyle ?? - Theme.of(context).textTheme.bodyMedium, - autofocus: widget.cellStyle.autofocus, - decoration: InputDecoration( - contentPadding: EdgeInsets.only( - top: GridSize.cellContentInsets.top, - bottom: GridSize.cellContentInsets.bottom, - ), - border: InputBorder.none, - focusedBorder: InputBorder.none, - enabledBorder: InputBorder.none, - errorBorder: InputBorder.none, - disabledBorder: InputBorder.none, - hintText: widget.cellStyle.placeholder, - isDense: true, - ), - onTapOutside: (_) => focusNode.unfocus(), - ), ), + HSpace(widget.cellStyle.emojiHPadding), ], - ), + Expanded( + child: widget.cellStyle.useRoundedBorder + ? FlowyTextField( + controller: _controller, + textStyle: widget.cellStyle.textStyle ?? + Theme.of(context).textTheme.bodyMedium, + focusNode: focusNode, + autoFocus: widget.cellStyle.autofocus, + hintText: widget.cellStyle.placeholder, + onChanged: (text) => _cellBloc.add( + TextCellEvent.updateText(text), + ), + debounceDuration: const Duration(milliseconds: 300), + ) + : TextField( + controller: _controller, + focusNode: focusNode, + maxLines: null, + style: widget.cellStyle.textStyle ?? + Theme.of(context).textTheme.bodyMedium, + autofocus: widget.cellStyle.autofocus, + decoration: InputDecoration( + contentPadding: widget.cellStyle.cellPadding ?? + GridSize.cellContentInsets, + border: InputBorder.none, + focusedBorder: InputBorder.none, + enabledBorder: InputBorder.none, + errorBorder: InputBorder.none, + disabledBorder: InputBorder.none, + hintText: widget.cellStyle.placeholder, + isDense: true, + isCollapsed: true, + ), + onTapOutside: (_) => focusNode.unfocus(), + ), + ), + ], ), ), ); diff --git a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/url_cell/url_cell.dart b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/url_cell/url_cell.dart index 41aa3b7cfc..6058c2ff1a 100644 --- a/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/url_cell/url_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database_view/widgets/row/cells/url_cell/url_cell.dart @@ -4,11 +4,8 @@ import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/plugins/database_view/application/cell/cell_controller.dart'; import 'package:appflowy/plugins/database_view/application/cell/cell_controller_builder.dart'; import 'package:appflowy/workspace/presentation/home/toast.dart'; -import 'package:appflowy_popover/appflowy_popover.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:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; @@ -16,19 +13,20 @@ import 'package:url_launcher/url_launcher_string.dart'; import '../../../../grid/presentation/layout/sizes.dart'; import '../../accessory/cell_accessory.dart'; import '../../cell_builder.dart'; -import 'cell_editor.dart'; import 'url_cell_bloc.dart'; class GridURLCellStyle extends GridCellStyle { String? placeholder; TextStyle? textStyle; bool? autofocus; + EdgeInsets? cellPadding; List accessoryTypes; GridURLCellStyle({ this.placeholder, this.accessoryTypes = const [], + this.cellPadding, }); } @@ -48,14 +46,14 @@ class GridURLCell extends GridCellWidget { if (style != null) { cellStyle = (style as GridURLCellStyle); } else { - cellStyle = null; + cellStyle = GridURLCellStyle(); } } /// Use final URLCellDataNotifier _cellDataNotifier; final CellControllerBuilder cellControllerBuilder; - late final GridURLCellStyle? cellStyle; + late final GridURLCellStyle cellStyle; @override GridCellState createState() => _GridURLCellState(); @@ -65,13 +63,11 @@ class GridURLCell extends GridCellWidget { GridCellAccessoryBuildContext buildContext, ) get accessoryBuilder => (buildContext) { final List accessories = []; - if (cellStyle != null) { - accessories.addAll( - cellStyle!.accessoryTypes.map((ty) { - return _accessoryFromType(ty, buildContext); - }), - ); - } + accessories.addAll( + cellStyle.accessoryTypes.map((ty) { + return _accessoryFromType(ty, buildContext); + }), + ); // If the accessories is empty then the default accessory will be GridURLCellAccessoryType.visitURL if (accessories.isEmpty) { @@ -143,39 +139,31 @@ class _GridURLCellState extends GridEditableTextCell { _controller.text = state.content; }, builder: (context, state) { - final style = widget.cellStyle?.textStyle ?? + final style = widget.cellStyle.textStyle ?? Theme.of(context).textTheme.bodyMedium!; widget._cellDataNotifier.value = state.content; - return Padding( - padding: EdgeInsets.only( - left: GridSize.cellContentInsets.left, - right: GridSize.cellContentInsets.right, + return TextField( + controller: _controller, + focusNode: focusNode, + maxLines: null, + style: style.copyWith( + color: Theme.of(context).colorScheme.primary, + decoration: TextDecoration.underline, ), - child: TextField( - controller: _controller, - focusNode: focusNode, - maxLines: null, - style: style.copyWith( - color: Theme.of(context).colorScheme.primary, - decoration: TextDecoration.underline, - ), - autofocus: false, - decoration: InputDecoration( - contentPadding: EdgeInsets.only( - top: GridSize.cellContentInsets.top, - bottom: GridSize.cellContentInsets.bottom, - ), - border: InputBorder.none, - focusedBorder: InputBorder.none, - enabledBorder: InputBorder.none, - errorBorder: InputBorder.none, - disabledBorder: InputBorder.none, - hintText: widget.cellStyle?.placeholder, - hintStyle: style.copyWith(color: Theme.of(context).hintColor), - isDense: true, - ), - onTapOutside: (_) => focusNode.unfocus(), + autofocus: false, + decoration: InputDecoration( + contentPadding: + widget.cellStyle.cellPadding ?? GridSize.cellContentInsets, + border: InputBorder.none, + focusedBorder: InputBorder.none, + enabledBorder: InputBorder.none, + errorBorder: InputBorder.none, + disabledBorder: InputBorder.none, + hintText: widget.cellStyle.placeholder, + hintStyle: style.copyWith(color: Theme.of(context).hintColor), + isDense: true, ), + onTapOutside: (_) => focusNode.unfocus(), ); }, ), @@ -184,7 +172,7 @@ class _GridURLCellState extends GridEditableTextCell { @override Future focusChanged() async { - _cellBloc.add(URLCellEvent.updateURL(_controller.text)); + _cellBloc.add(URLCellEvent.updateURL(_controller.text.trim())); return super.focusChanged(); } @@ -192,51 +180,6 @@ class _GridURLCellState extends GridEditableTextCell { String? onCopy() => _cellBloc.state.content; } -class _EditURLAccessory extends StatefulWidget { - const _EditURLAccessory({ - required this.cellControllerBuilder, - required this.anchorContext, - }); - - final CellControllerBuilder cellControllerBuilder; - final BuildContext anchorContext; - - @override - State createState() => _EditURLAccessoryState(); -} - -class _EditURLAccessoryState extends State<_EditURLAccessory> - with GridCellAccessoryState { - final popoverController = PopoverController(); - - @override - Widget build(BuildContext context) { - return AppFlowyPopover( - margin: EdgeInsets.zero, - constraints: BoxConstraints.loose(const Size(300, 160)), - controller: popoverController, - direction: PopoverDirection.bottomWithLeftAligned, - offset: const Offset(0, 8), - child: FlowySvg( - FlowySvgs.edit_s, - color: AFThemeExtension.of(context).textColor, - ), - popupBuilder: (BuildContext popoverContext) { - return URLEditorPopover( - cellController: - widget.cellControllerBuilder.build() as URLCellController, - onExit: () => popoverController.close(), - ); - }, - ); - } - - @override - void onTap() { - popoverController.show(); - } -} - typedef CopyURLCellAccessoryBuilder = GridCellAccessoryBuilder>; diff --git 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 index f737e33e23..7295643859 100644 --- 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 @@ -207,12 +207,18 @@ class _PropertyCellState extends State<_PropertyCell> { child: SizedBox( width: 160, height: 30, - child: FieldCellButton( - field: widget.cellContext.fieldInfo.field, - onTap: () => _popoverController.show(), - radius: BorderRadius.circular(6), - margin: - const EdgeInsets.symmetric(horizontal: 4, vertical: 6), + child: Tooltip( + waitDuration: const Duration(seconds: 1), + preferBelow: false, + verticalOffset: 15, + message: widget.cellContext.fieldInfo.field.name, + child: FieldCellButton( + field: widget.cellContext.fieldInfo.field, + onTap: () => _popoverController.show(), + radius: BorderRadius.circular(6), + margin: + const EdgeInsets.symmetric(horizontal: 4, vertical: 6), + ), ), ), ), @@ -266,10 +272,13 @@ GridCellStyle? customCellStyle(FieldType fieldType) { case FieldType.Number: return GridNumberCellStyle( placeholder: LocaleKeys.grid_row_textPlaceholder.tr(), + cellPadding: const EdgeInsets.symmetric(horizontal: 8, vertical: 10), ); case FieldType.RichText: return GridTextCellStyle( + cellPadding: const EdgeInsets.symmetric(horizontal: 8, vertical: 10), placeholder: LocaleKeys.grid_row_textPlaceholder.tr(), + showEmoji: false, ); case FieldType.SingleSelect: return SelectOptionCellStyle( @@ -280,6 +289,7 @@ GridCellStyle? customCellStyle(FieldType fieldType) { case FieldType.URL: return GridURLCellStyle( placeholder: LocaleKeys.grid_row_textPlaceholder.tr(), + cellPadding: const EdgeInsets.symmetric(horizontal: 8, vertical: 10), accessoryTypes: [ GridURLCellAccessoryType.copyURL, GridURLCellAccessoryType.visitURL, diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs index dcc3c87a2d..1e6aa781a1 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs @@ -25,27 +25,16 @@ pub trait SelectTypeOptionSharedAction: Send + Sync { /// If the option already exists, it will be updated. /// If the option does not exist, it will be inserted at the beginning. fn insert_option(&mut self, new_option: SelectOption) { - self.insert_option_at_index(new_option, None); - } - - fn insert_option_at_index(&mut self, new_option: SelectOption, new_index: Option) { let options = self.mut_options(); - let safe_new_index = new_index.map(|index| { - if index > options.len() { - options.len() - } else { - index - } - }); if let Some(index) = options .iter() .position(|option| option.id == new_option.id || option.name == new_option.name) { options.remove(index); - options.insert(safe_new_index.unwrap_or(index), new_option); + options.insert(index, new_option); } else { - options.insert(safe_new_index.unwrap_or(0), new_option); + options.insert(0, new_option); } } diff --git a/frontend/rust-lib/flowy-database2/src/services/group/configuration.rs b/frontend/rust-lib/flowy-database2/src/services/group/configuration.rs index 3b387940bd..06bd6101b2 100644 --- a/frontend/rust-lib/flowy-database2/src/services/group/configuration.rs +++ b/frontend/rust-lib/flowy-database2/src/services/group/configuration.rs @@ -477,12 +477,6 @@ fn merge_groups( // The group is ordered in old groups. Add them before adding the new groups for old in old_groups { - if let Some(index) = new_group_map.get_index_of(&old.id) { - let right = new_group_map.split_off(index); - merge_result.all_groups.extend(new_group_map.into_values()); - new_group_map = right; - } - if let Some(new) = new_group_map.shift_remove(&old.id) { merge_result.all_groups.push(new.clone()); } else { @@ -491,11 +485,10 @@ fn merge_groups( } // Find out the new groups - let new_groups = new_group_map.into_values(); - for (_, group) in new_groups.into_iter().enumerate() { - merge_result.all_groups.push(group.clone()); - merge_result.new_groups.push(group); - } + merge_result + .all_groups + .extend(new_group_map.values().cloned()); + merge_result.new_groups.extend(new_group_map.into_values()); // The `No status` group index is initialized to 0 if let Some(no_status_group) = no_status_group { diff --git a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/single_select_controller.rs b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/single_select_controller.rs index 0129715e3a..6986ad0e83 100644 --- a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/single_select_controller.rs +++ b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/single_select_controller.rs @@ -101,10 +101,7 @@ impl GroupCustomize for SingleSelectGroupController { ) -> FlowyResult<(Option, Option)> { let mut new_type_option = self.type_option.clone(); let new_select_option = self.type_option.create_option(&name); - new_type_option.insert_option_at_index( - new_select_option.clone(), - Some(new_type_option.options.len()), - ); + new_type_option.insert_option(new_select_option.clone()); let new_group = Group::new(new_select_option.id, new_select_option.name); let inserted_group_pb = self.context.add_new_group(new_group)?; diff --git a/frontend/rust-lib/flowy-database2/tests/database/group_test/test.rs b/frontend/rust-lib/flowy-database2/tests/database/group_test/test.rs index 119986b04b..cfdd8a1c8c 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/group_test/test.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/group_test/test.rs @@ -462,7 +462,7 @@ async fn group_insert_single_select_option_test() { AssertGroupCount(5), ]; test.run_scripts(scripts).await; - let new_group = test.group_at_index(1).await; + let new_group = test.group_at_index(4).await; assert_eq!(new_group.group_name, new_option_name); }