From 80f034beee51a14718d7fb63a9f3f2d94ddf59b4 Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Sat, 22 Oct 2022 19:55:18 +0800 Subject: [PATCH] feat: alter some select option editor bloc events and add tests (#1264) * chore: separate select and unselect events * style: improve readability * chore: don't send empty payloads if we can help it * test: add select option text field test * test: complete bloc test for select option * test: delete all options between each test * fix: keep insert order * test: combine select and unselect tests * chore: remove duplicate wait Co-authored-by: appflowy --- .../cell/select_option_editor_bloc.dart | 59 +++--- .../cell/select_option_service.dart | 7 +- .../select_option_editor.dart | 12 +- .../cell/select_option_cell/text_field.dart | 19 +- .../grid_test/select_option_bloc_test.dart | 180 +++++++++++++++++- .../select_option_text_field_test.dart | 90 +++++++++ 6 files changed, 324 insertions(+), 43 deletions(-) create mode 100644 frontend/app_flowy/test/widget_test/select_option_text_field_test.dart diff --git a/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_editor_bloc.dart b/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_editor_bloc.dart index 931e370855..8b36560027 100644 --- a/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_editor_bloc.dart +++ b/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_editor_bloc.dart @@ -1,7 +1,6 @@ import 'dart:async'; import 'package:app_flowy/plugins/grid/application/cell/cell_service/cell_service.dart'; -import 'package:collection/collection.dart'; import 'package:dartz/dartz.dart'; import 'package:flowy_sdk/log.dart'; import 'package:flowy_sdk/protobuf/flowy-grid/select_type_option.pb.dart'; @@ -46,19 +45,29 @@ class SelectOptionCellEditorBloc )); }, deleteOption: (_DeleteOption value) { - _deleteOption(value.option); + _deleteOption([value.option]); + }, + deleteAllOptions: (_DeleteAllOptions value) { + if (state.allOptions.isNotEmpty) { + _deleteOption(state.allOptions); + } }, updateOption: (_UpdateOption value) { _updateOption(value.option); }, selectOption: (_SelectOption value) { - _onSelectOption(value.optionId); + _selectOptionService.select(optionIds: [value.optionId]); + }, + unSelectOption: (_UnSelectOption value) { + _selectOptionService.unSelect(optionIds: [value.optionId]); }, trySelectOption: (_TrySelectOption value) { _trySelectOption(value.optionName, emit); }, selectMultipleOptions: (_SelectMultipleOptions value) { - _selectMultipleOptions(value.optionNames); + if (value.optionNames.isNotEmpty) { + _selectMultipleOptions(value.optionNames); + } _filterOption(value.remainder, emit); }, filterOption: (_SelectOptionFilter value) { @@ -81,11 +90,8 @@ class SelectOptionCellEditorBloc result.fold((l) => {}, (err) => Log.error(err)); } - void _deleteOption(SelectOptionPB option) async { - final result = await _selectOptionService.delete( - option: option, - ); - + void _deleteOption(List options) async { + final result = await _selectOptionService.delete(options: options); result.fold((l) => null, (err) => Log.error(err)); } @@ -97,16 +103,6 @@ class SelectOptionCellEditorBloc result.fold((l) => null, (err) => Log.error(err)); } - void _onSelectOption(String optionId) { - final hasSelected = state.selectedOptions - .firstWhereOrNull((option) => option.id == optionId); - if (hasSelected != null) { - _selectOptionService.unSelect(optionIds: [optionId]); - } else { - _selectOptionService.select(optionIds: [optionId]); - } - } - void _trySelectOption( String optionName, Emitter emit) { SelectOptionPB? matchingOption; @@ -138,9 +134,19 @@ class SelectOptionCellEditorBloc } void _selectMultipleOptions(List optionNames) { - final optionIds = state.options - .where((e) => optionNames.contains(e.name)) - .map((e) => e.id); + // The options are unordered. So in order to keep the inserted [optionNames] + // order, it needs to get the option id in the [optionNames] order. + final lowerCaseNames = optionNames.map((e) => e.toLowerCase()); + final Map optionIdsMap = {}; + for (final option in state.options) { + optionIdsMap[option.name.toLowerCase()] = option.id; + } + + final optionIds = lowerCaseNames + .where((name) => optionIdsMap[name] != null) + .map((name) => optionIdsMap[name]!) + .toList(); + _selectOptionService.select(optionIds: optionIds); } @@ -162,8 +168,10 @@ class SelectOptionCellEditorBloc return; } return result.fold( - (data) => add(SelectOptionEditorEvent.didReceiveOptions( - data.options, data.selectOptions)), + (data) => add( + SelectOptionEditorEvent.didReceiveOptions( + data.options, data.selectOptions), + ), (err) { Log.error(err); return null; @@ -225,10 +233,13 @@ class SelectOptionEditorEvent with _$SelectOptionEditorEvent { _NewOption; const factory SelectOptionEditorEvent.selectOption(String optionId) = _SelectOption; + const factory SelectOptionEditorEvent.unSelectOption(String optionId) = + _UnSelectOption; const factory SelectOptionEditorEvent.updateOption(SelectOptionPB option) = _UpdateOption; const factory SelectOptionEditorEvent.deleteOption(SelectOptionPB option) = _DeleteOption; + const factory SelectOptionEditorEvent.deleteAllOptions() = _DeleteAllOptions; const factory SelectOptionEditorEvent.filterOption(String optionName) = _SelectOptionFilter; const factory SelectOptionEditorEvent.trySelectOption(String optionName) = diff --git a/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_service.dart b/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_service.dart index 22f3b02eb1..0a182fbe15 100644 --- a/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_service.dart +++ b/frontend/app_flowy/lib/plugins/grid/application/cell/select_option_service.dart @@ -45,11 +45,10 @@ class SelectOptionService { return GridEventUpdateSelectOption(payload).send(); } - Future> delete({ - required SelectOptionPB option, - }) { + Future> delete( + {required Iterable options}) { final payload = SelectOptionChangesetPayloadPB.create() - ..deleteOptions.add(option) + ..deleteOptions.addAll(options) ..cellIdentifier = _cellIdentifier(); return GridEventUpdateSelectOption(payload).send(); diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_editor.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_editor.dart index d6b08e507d..2674892cf8 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_editor.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_editor.dart @@ -261,9 +261,15 @@ class _SelectOptionCellState extends State<_SelectOptionCell> { child: SelectOptionTagCell( option: widget.option, onSelected: (option) { - context - .read() - .add(SelectOptionEditorEvent.selectOption(option.id)); + if (widget.isSelected) { + context + .read() + .add(SelectOptionEditorEvent.unSelectOption(option.id)); + } else { + context + .read() + .add(SelectOptionEditorEvent.selectOption(option.id)); + } }, children: [ if (widget.isSelected) diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart index 9fd16c8f5d..becc0b8cb0 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart @@ -96,7 +96,7 @@ class _SelectOptionTextFieldState extends State { } if (text.isNotEmpty) { - widget.onSubmitted(text); + widget.onSubmitted(text.trim()); focusNode.requestFocus(); } }, @@ -132,26 +132,25 @@ class _SelectOptionTextFieldState extends State { return; } - final trimmedText = text.trim(); + final trimmedText = text.trimLeft(); List splits = []; String currentString = ''; // split the string into tokens for (final char in trimmedText.split('')) { - if (!widget.textSeparators.contains(char)) { - currentString += char; + if (widget.textSeparators.contains(char)) { + if (currentString.isNotEmpty) { + splits.add(currentString.trim()); + } + currentString = ''; continue; } - if (currentString.isNotEmpty) { - splits.add(currentString); - } - currentString = ''; + currentString += char; } // add the remainder (might be '') splits.add(currentString); - final submittedOptions = - splits.sublist(0, splits.length - 1).map((e) => e.trim()).toList(); + final submittedOptions = splits.sublist(0, splits.length - 1).toList(); final remainder = splits.elementAt(splits.length - 1).trimLeft(); editingController.text = remainder; diff --git a/frontend/app_flowy/test/bloc_test/grid_test/select_option_bloc_test.dart b/frontend/app_flowy/test/bloc_test/grid_test/select_option_bloc_test.dart index bea51b4960..5aad5c9420 100644 --- a/frontend/app_flowy/test/bloc_test/grid_test/select_option_bloc_test.dart +++ b/frontend/app_flowy/test/bloc_test/grid_test/select_option_bloc_test.dart @@ -1,6 +1,9 @@ import 'package:app_flowy/plugins/grid/application/cell/cell_service/cell_service.dart'; import 'package:app_flowy/plugins/grid/application/cell/select_option_editor_bloc.dart'; +import 'package:app_flowy/plugins/grid/application/prelude.dart'; +import 'package:dartz/dartz.dart'; import 'package:flowy_sdk/protobuf/flowy-grid/field_entities.pb.dart'; +import 'package:flowy_sdk/protobuf/flowy-grid/select_type_option.pb.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:bloc_test/bloc_test.dart'; import 'util.dart'; @@ -18,6 +21,28 @@ void main() { await cellTest.makeCellController(FieldType.SingleSelect); }); + blocTest( + "delete options", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + bloc.add(const SelectOptionEditorEvent.newOption("A")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.newOption("B")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.newOption("C")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.deleteAllOptions()); + }, + wait: gridResponseDuration(), + verify: (bloc) { + assert(bloc.state.options.isEmpty); + }, + ); + blocTest( "create option", build: () { @@ -25,12 +50,163 @@ void main() { bloc.add(const SelectOptionEditorEvent.initial()); return bloc; }, - act: (bloc) => bloc.add(const SelectOptionEditorEvent.newOption("A")), + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("A")); + }, + wait: gridResponseDuration(), + verify: (bloc) { + expect(bloc.state.options.length, 1); + expect(bloc.state.options[0].name, "A"); + }, + ); + + blocTest( + "delete option", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("A")); + await Future.delayed(gridResponseDuration()); + bloc.add(SelectOptionEditorEvent.deleteOption(bloc.state.options[0])); + }, + wait: gridResponseDuration(), + verify: (bloc) { + assert(bloc.state.options.isEmpty); + }, + ); + + blocTest( + "update option", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("A")); + await Future.delayed(gridResponseDuration()); + SelectOptionPB optionUpdate = bloc.state.options[0] + ..color = SelectOptionColorPB.Aqua + ..name = "B"; + bloc.add(SelectOptionEditorEvent.updateOption(optionUpdate)); + }, wait: gridResponseDuration(), verify: (bloc) { assert(bloc.state.options.length == 1); - assert(bloc.state.options[0].name == "A"); + expect(bloc.state.options[0].color, SelectOptionColorPB.Aqua); + expect(bloc.state.options[0].name, "B"); + }, + ); + + blocTest( + "select/unselect option", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("A")); + await Future.delayed(gridResponseDuration()); + expect(bloc.state.selectedOptions.length, 1); + final optionId = bloc.state.options[0].id; + bloc.add(SelectOptionEditorEvent.unSelectOption(optionId)); + await Future.delayed(gridResponseDuration()); + assert(bloc.state.selectedOptions.isEmpty); + bloc.add(SelectOptionEditorEvent.selectOption(optionId)); + }, + wait: gridResponseDuration(), + verify: (bloc) { + assert(bloc.state.selectedOptions.length == 1); + expect(bloc.state.selectedOptions[0].name, "A"); + }, + ); + + blocTest( + "select an option or create one", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("A")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.trySelectOption("B")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.trySelectOption("A")); + }, + wait: gridResponseDuration(), + verify: (bloc) { + assert(bloc.state.selectedOptions.length == 1); + assert(bloc.state.options.length == 2); + expect(bloc.state.selectedOptions[0].name, "A"); + }, + ); + + blocTest( + "select multiple options", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("A")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.newOption("B")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.selectMultipleOptions( + ["A", "B", "C"], "x")); + }, + wait: gridResponseDuration(), + verify: (bloc) { + assert(bloc.state.selectedOptions.length == 1); + expect(bloc.state.selectedOptions[0].name, "A"); + expect(bloc.state.filter, const Some("x")); + }, + ); + + blocTest( + "filter options", + build: () { + final bloc = SelectOptionCellEditorBloc(cellController: cellController); + bloc.add(const SelectOptionEditorEvent.initial()); + return bloc; + }, + act: (bloc) async { + _removeFieldOptions(bloc); + bloc.add(const SelectOptionEditorEvent.newOption("abcd")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.newOption("aaaa")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.newOption("defg")); + await Future.delayed(gridResponseDuration()); + bloc.add(const SelectOptionEditorEvent.filterOption("a")); + }, + wait: gridResponseDuration(), + verify: (bloc) { + expect(bloc.state.options.length, 2); + expect(bloc.state.allOptions.length, 3); + expect(bloc.state.createOption, const Some("a")); + expect(bloc.state.filter, const Some("a")); }, ); }); } + +void _removeFieldOptions(SelectOptionCellEditorBloc bloc) async { + if (bloc.state.options.isNotEmpty) { + bloc.add(const SelectOptionEditorEvent.deleteAllOptions()); + await Future.delayed(gridResponseDuration()); + } +} diff --git a/frontend/app_flowy/test/widget_test/select_option_text_field_test.dart b/frontend/app_flowy/test/widget_test/select_option_text_field_test.dart new file mode 100644 index 0000000000..a8f0168f45 --- /dev/null +++ b/frontend/app_flowy/test/widget_test/select_option_text_field_test.dart @@ -0,0 +1,90 @@ +import 'dart:collection'; + +import 'package:app_flowy/plugins/grid/presentation/widgets/cell/select_option_cell/text_field.dart'; +import 'package:flowy_infra/theme.dart'; +import 'package:flowy_sdk/protobuf/flowy-grid/protobuf.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; +import 'package:textfield_tags/textfield_tags.dart'; + +import '../bloc_test/grid_test/util.dart'; + +void main() { + setUpAll(() { + AppFlowyGridTest.ensureInitialized(); + }); + + group('text_field.dart', () { + String submit = ''; + String remainder = ''; + List select = []; + + final textField = SelectOptionTextField( + options: const [], + selectedOptionMap: LinkedHashMap(), + distanceToText: 0.0, + tagController: TextfieldTagsController(), + onSubmitted: (text) => submit = text, + onPaste: (options, remaining) { + remainder = remaining; + select = options; + }, + newText: (_) {}, + textSeparators: const [','], + textController: TextEditingController(), + ); + + testWidgets('SelectOptionTextField callback outputs', + (WidgetTester tester) async { + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Provider.value( + value: AppTheme.fromType(ThemeType.light), + child: textField, + ), + ), + ), + ); + + // test that the input field exists + expect(find.byType(TextField), findsOneWidget); + + // simulate normal input + await tester.enterText(find.byType(TextField), 'abcd'); + expect(remainder, 'abcd'); + + await tester.enterText(find.byType(TextField), ' '); + expect(remainder, ''); + + // test submit functionality (aka pressing enter) + await tester.enterText(find.byType(TextField), 'an option'); + await tester.testTextInput.receiveAction(TextInputAction.done); + expect(submit, 'an option'); + + await tester.enterText(find.byType(TextField), ' another one '); + await tester.testTextInput.receiveAction(TextInputAction.done); + expect(submit, 'another one'); + + // test inputs containing commas + await tester.enterText(find.byType(TextField), ' abcd,'); + expect(remainder, ''); + expect(select, ['abcd']); + + await tester.enterText(find.byType(TextField), ',acd, aaaa '); + expect(remainder, 'aaaa '); + expect(select, ['acd']); + + await tester.enterText(find.byType(TextField), 'a a, bbbb , '); + expect(remainder, ''); + expect(select, ['a a', 'bbbb']); + + // test paste followed by submit + await tester.enterText(find.byType(TextField), 'aaa, bbb, c'); + await tester.testTextInput.receiveAction(TextInputAction.done); + expect(select, ['aaa', 'bbb']); + expect(submit, 'c'); + }); + }); +}