fix: state of AppFlowyBoard is wrong after reordering the group

This commit is contained in:
appflowy 2022-10-07 14:42:40 +08:00
parent 73e81da356
commit fa3f9c21fc
14 changed files with 154 additions and 75 deletions

View File

@ -166,11 +166,11 @@ class BoardBloc extends Bloc<BoardEvent, BoardState> {
}
}
void _moveGroup(String fromColumnId, String toColumnId) {
void _moveGroup(String fromGroupId, String toGroupId) {
_rowService
.moveGroup(
fromGroupId: fromColumnId,
toGroupId: toColumnId,
fromGroupId: fromGroupId,
toGroupId: toGroupId,
)
.then((result) {
result.fold((l) => null, (r) => add(BoardEvent.didReceiveError(r)));

View File

@ -87,13 +87,13 @@ class BoardDataController {
onUpdatedGroup.call(changeset.updateGroups);
}
if (changeset.insertedGroups.isNotEmpty) {
onInsertedGroup.call(changeset.insertedGroups);
}
if (changeset.deletedGroups.isNotEmpty) {
onDeletedGroup.call(changeset.deletedGroups);
}
if (changeset.insertedGroups.isNotEmpty) {
onInsertedGroup.call(changeset.insertedGroups);
}
},
(e) => _onError?.call(e),
);

View File

@ -32,4 +32,8 @@ class Log {
'AppFlowyBoard: ❗️[Trace] - ${DateTime.now().second}=> $message');
}
}
static void error(String? message) {
debugPrint('AppFlowyBoard: ❌[Error] - ${DateTime.now().second}=> $message');
}
}

View File

@ -11,11 +11,11 @@ import 'reorder_phantom/phantom_controller.dart';
import '../rendering/board_overlay.dart';
class AppFlowyBoardScrollController {
AppFlowyBoardState? _groupState;
AppFlowyBoardState? _boardState;
void scrollToBottom(String groupId,
{void Function(BuildContext)? completed}) {
_groupState?.reorderFlexActionMap[groupId]?.scrollToBottom(completed);
_boardState?.reorderFlexActionMap[groupId]?.scrollToBottom(completed);
}
}
@ -92,11 +92,7 @@ class AppFlowyBoard extends StatelessWidget {
///
final AppFlowyBoardScrollController? boardScrollController;
final AppFlowyBoardState _groupState = AppFlowyBoardState();
late final BoardPhantomController _phantomController;
AppFlowyBoard({
const AppFlowyBoard({
required this.controller,
required this.cardBuilder,
this.background,
@ -107,12 +103,7 @@ class AppFlowyBoard extends StatelessWidget {
this.groupConstraints = const BoxConstraints(maxWidth: 200),
this.config = const AppFlowyBoardConfig(),
Key? key,
}) : super(key: key) {
_phantomController = BoardPhantomController(
delegate: controller,
groupsState: _groupState,
);
}
}) : super(key: key);
@override
Widget build(BuildContext context) {
@ -120,8 +111,14 @@ class AppFlowyBoard extends StatelessWidget {
value: controller,
child: Consumer<AppFlowyBoardController>(
builder: (context, notifier, child) {
final boardState = AppFlowyBoardState();
BoardPhantomController phantomController = BoardPhantomController(
delegate: controller,
groupsState: boardState,
);
if (boardScrollController != null) {
boardScrollController!._groupState = _groupState;
boardScrollController!._boardState = boardState;
}
return _AppFlowyBoardContent(
@ -129,14 +126,14 @@ class AppFlowyBoard extends StatelessWidget {
dataController: controller,
scrollController: scrollController,
scrollManager: boardScrollController,
groupState: _groupState,
boardState: boardState,
background: background,
delegate: _phantomController,
delegate: phantomController,
groupConstraints: groupConstraints,
cardBuilder: cardBuilder,
footerBuilder: footerBuilder,
headerBuilder: headerBuilder,
phantomController: _phantomController,
phantomController: phantomController,
onReorder: controller.moveGroup,
);
},
@ -154,7 +151,7 @@ class _AppFlowyBoardContent extends StatefulWidget {
final ReorderFlexConfig reorderFlexConfig;
final BoxConstraints groupConstraints;
final AppFlowyBoardScrollController? scrollManager;
final AppFlowyBoardState groupState;
final AppFlowyBoardState boardState;
final AppFlowyBoardCardBuilder cardBuilder;
final AppFlowyBoardHeaderBuilder? headerBuilder;
final AppFlowyBoardFooterBuilder? footerBuilder;
@ -167,7 +164,7 @@ class _AppFlowyBoardContent extends StatefulWidget {
required this.delegate,
required this.dataController,
required this.scrollManager,
required this.groupState,
required this.boardState,
this.scrollController,
this.background,
required this.groupConstraints,
@ -199,7 +196,7 @@ class _AppFlowyBoardContentState extends State<_AppFlowyBoardContent> {
reorderFlexId: widget.dataController.identifier,
acceptedReorderFlexId: widget.dataController.groupIds,
delegate: widget.delegate,
columnsState: widget.groupState,
columnsState: widget.boardState,
);
final reorderFlex = ReorderFlex(
@ -253,7 +250,7 @@ class _AppFlowyBoardContentState extends State<_AppFlowyBoardContent> {
);
final reorderFlexAction = ReorderFlexActionImpl();
widget.groupState.reorderFlexActionMap[columnData.id] =
widget.boardState.reorderFlexActionMap[columnData.id] =
reorderFlexAction;
return ChangeNotifierProvider.value(
@ -274,8 +271,8 @@ class _AppFlowyBoardContentState extends State<_AppFlowyBoardContent> {
onReorder: widget.dataController.moveGroupItem,
cornerRadius: widget.config.cornerRadius,
backgroundColor: widget.config.groupBackgroundColor,
dragStateStorage: widget.groupState,
dragTargetKeys: widget.groupState,
dragStateStorage: widget.boardState,
dragTargetKeys: widget.boardState,
reorderFlexAction: reorderFlexAction,
);

View File

@ -138,7 +138,11 @@ class AppFlowyBoardController extends ChangeNotifier
/// groups or get ready to reinitialize the [AppFlowyBoard].
void clear() {
_groupDatas.clear();
for (final group in _groupControllers.values) {
group.dispose();
}
_groupControllers.clear();
notifyListeners();
}
@ -223,6 +227,8 @@ class AppFlowyBoardController extends ChangeNotifier
final fromGroupController = getGroupController(fromGroupId)!;
final toGroupController = getGroupController(toGroupId)!;
final fromGroupItem = fromGroupController.removeAt(fromGroupIndex);
if (fromGroupItem == null) return;
if (toGroupController.items.length > toGroupIndex) {
assert(toGroupController.items[toGroupIndex] is PhantomGroupItem);
@ -283,7 +289,9 @@ class AppFlowyBoardController extends ChangeNotifier
Log.trace(
'[$BoardPhantomController] update $groupId:$index to $groupId:$newIndex');
final item = groupController.removeAt(index, notify: false);
groupController.insert(newIndex, item, notify: false);
if (item != null) {
groupController.insert(newIndex, item, notify: false);
}
}
}
}

View File

@ -156,9 +156,9 @@ class _AppFlowyBoardGroupState extends State<AppFlowyBoardGroup> {
widget.onDragStarted?.call(index);
},
onReorder: ((fromIndex, toIndex) {
if (widget.phantomController.isFromGroup(widget.groupId)) {
if (widget.phantomController.shouldReorder(widget.groupId)) {
widget.onReorder(widget.groupId, fromIndex, toIndex);
widget.phantomController.transformIndex(fromIndex, toIndex);
widget.phantomController.updateIndex(fromIndex, toIndex);
}
}),
onDragEnded: () {

View File

@ -52,8 +52,17 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin {
/// * [notify] the default value of [notify] is true, it will notify the
/// listener. Set to false if you do not want to notify the listeners.
///
AppFlowyGroupItem removeAt(int index, {bool notify = true}) {
assert(index >= 0);
AppFlowyGroupItem? removeAt(int index, {bool notify = true}) {
if (groupData._items.length <= index) {
Log.error(
'Fatal error, index is out of bounds. Index: $index, len: ${groupData._items.length}');
return null;
}
if (index < 0) {
Log.error('Invalid index:$index');
return null;
}
Log.debug('[$AppFlowyGroupController] $groupData remove item at $index');
final item = groupData._items.removeAt(index);
@ -73,12 +82,17 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin {
/// Move the item from [fromIndex] to [toIndex]. It will do nothing if the
/// [fromIndex] equal to the [toIndex].
bool move(int fromIndex, int toIndex) {
assert(fromIndex >= 0);
assert(toIndex >= 0);
if (groupData._items.length < fromIndex) {
Log.error(
'Out of bounds error. index: $fromIndex should not greater than ${groupData._items.length}');
return false;
}
if (fromIndex == toIndex) {
return false;
}
Log.debug(
'[$AppFlowyGroupController] $groupData move item from $fromIndex to $toIndex');
final item = groupData._items.removeAt(fromIndex);
@ -126,7 +140,7 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin {
Log.debug('[$AppFlowyGroupController] $groupData add $newItem');
} else {
if (index >= groupData._items.length) {
Log.warn(
Log.error(
'[$AppFlowyGroupController] unexpected items length, index should less than the count of the items. Index: $index, items count: ${items.length}');
return;
}

View File

@ -46,15 +46,23 @@ class BoardPhantomController extends OverlapDragTargetDelegate
required this.groupsState,
});
bool isFromGroup(String groupId) {
/// Determines whether the group should perform reorder
///
/// Returns `true` if the fromGroupId and toGroupId of the phantomRecord
/// equal to the passed in groupId.
///
/// Returns `true` if the phantomRecord is null
///
bool shouldReorder(String groupId) {
if (phantomRecord != null) {
return phantomRecord!.fromGroupId == groupId;
return phantomRecord!.toGroupId == groupId &&
phantomRecord!.fromGroupId == groupId;
} else {
return true;
}
}
void transformIndex(int fromIndex, int toIndex) {
void updateIndex(int fromIndex, int toIndex) {
if (phantomRecord == null) {
return;
}
@ -69,7 +77,6 @@ class BoardPhantomController extends OverlapDragTargetDelegate
/// Remove the phantom in the group when the group is end dragging.
void groupEndDragging(String groupId) {
phantomState.setGroupIsDragging(groupId, false);
if (phantomRecord == null) return;
final fromGroupId = phantomRecord!.fromGroupId;
@ -246,10 +253,6 @@ class PhantomRecord {
});
void updateFromGroupIndex(int index) {
if (fromGroupIndex == index) {
return;
}
fromGroupIndex = index;
}

View File

@ -623,7 +623,7 @@ impl GridRevisionEditor {
self.view_manager
.move_group_row(row_rev, to_group_id, to_row_id.clone(), |row_changeset| {
wrap_future(async move {
tracing::trace!("Move group row cause row data changed: {:?}", row_changeset);
tracing::trace!("Row data changed: {:?}", row_changeset);
let cell_changesets = row_changeset
.cell_by_field_id
.into_iter()

View File

@ -79,7 +79,7 @@ impl GridViewRevisionEditor {
Ok(json_str)
}
pub(crate) async fn will_create_row(&self, row_rev: &mut RowRevision, params: &CreateRowParams) {
pub(crate) async fn will_create_view_row(&self, row_rev: &mut RowRevision, params: &CreateRowParams) {
if params.group_id.is_none() {
return;
}
@ -92,7 +92,7 @@ impl GridViewRevisionEditor {
.await;
}
pub(crate) async fn did_create_row(&self, row_pb: &RowPB, params: &CreateRowParams) {
pub(crate) async fn did_create_view_row(&self, row_pb: &RowPB, params: &CreateRowParams) {
// Send the group notification if the current view has groups
match params.group_id.as_ref() {
None => {}
@ -115,7 +115,7 @@ impl GridViewRevisionEditor {
}
#[tracing::instrument(level = "trace", skip_all)]
pub(crate) async fn did_delete_row(&self, row_rev: &RowRevision) {
pub(crate) async fn did_delete_view_row(&self, row_rev: &RowRevision) {
// Send the group notification if the current view has groups;
let changesets = self
.mut_group_controller(|group_controller, field_rev| group_controller.did_delete_row(row_rev, &field_rev))
@ -129,7 +129,7 @@ impl GridViewRevisionEditor {
}
}
pub(crate) async fn did_update_row(&self, row_rev: &RowRevision) {
pub(crate) async fn did_update_view_row(&self, row_rev: &RowRevision) {
let changesets = self
.mut_group_controller(|group_controller, field_rev| group_controller.did_update_row(row_rev, &field_rev))
.await;
@ -141,7 +141,7 @@ impl GridViewRevisionEditor {
}
}
pub(crate) async fn move_group_row(
pub(crate) async fn move_view_group_row(
&self,
row_rev: &RowRevision,
row_changeset: &mut RowChangeset,
@ -167,14 +167,14 @@ impl GridViewRevisionEditor {
}
/// Only call once after grid view editor initialized
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) async fn load_groups(&self) -> FlowyResult<Vec<GroupPB>> {
pub(crate) async fn load_view_groups(&self) -> FlowyResult<Vec<GroupPB>> {
let groups = self.group_controller.read().await.groups();
tracing::trace!("Number of groups: {}", groups.len());
Ok(groups.into_iter().map(GroupPB::from).collect())
}
#[tracing::instrument(level = "trace", skip(self), err)]
pub(crate) async fn move_group(&self, params: MoveGroupParams) -> FlowyResult<()> {
pub(crate) async fn move_view_group(&self, params: MoveGroupParams) -> FlowyResult<()> {
let _ = self
.group_controller
.write()
@ -206,13 +206,13 @@ impl GridViewRevisionEditor {
self.group_controller.read().await.field_id().to_owned()
}
pub(crate) async fn get_setting(&self) -> GridSettingPB {
pub(crate) async fn get_view_setting(&self) -> GridSettingPB {
let field_revs = self.field_delegate.get_field_revs().await;
let grid_setting = make_grid_setting(&*self.pad.read().await, &field_revs);
grid_setting
}
pub(crate) async fn get_filters(&self) -> Vec<GridFilterConfigurationPB> {
pub(crate) async fn get_view_filters(&self) -> Vec<GridFilterConfigurationPB> {
let field_revs = self.field_delegate.get_field_revs().await;
match self.pad.read().await.get_all_filters(&field_revs) {
None => vec![],
@ -245,7 +245,7 @@ impl GridViewRevisionEditor {
Ok(())
}
pub(crate) async fn delete_group(&self, params: DeleteGroupParams) -> FlowyResult<()> {
pub(crate) async fn delete_view_group(&self, params: DeleteGroupParams) -> FlowyResult<()> {
self.modify(|pad| {
let changeset = pad.delete_filter(&params.field_id, &params.field_type_rev, &params.group_id)?;
Ok(changeset)
@ -253,7 +253,7 @@ impl GridViewRevisionEditor {
.await
}
pub(crate) async fn insert_filter(&self, params: InsertFilterParams) -> FlowyResult<()> {
pub(crate) async fn insert_view_filter(&self, params: InsertFilterParams) -> FlowyResult<()> {
self.modify(|pad| {
let filter_rev = FilterConfigurationRevision {
id: gen_grid_filter_id(),
@ -267,7 +267,7 @@ impl GridViewRevisionEditor {
.await
}
pub(crate) async fn delete_filter(&self, delete_filter: DeleteFilterParams) -> FlowyResult<()> {
pub(crate) async fn delete_view_filter(&self, delete_filter: DeleteFilterParams) -> FlowyResult<()> {
self.modify(|pad| {
let changeset = pad.delete_filter(
&delete_filter.field_id,
@ -324,7 +324,7 @@ impl GridViewRevisionEditor {
}
async fn notify_did_update_setting(&self) {
let setting = self.get_setting().await;
let setting = self.get_view_setting().await;
send_dart_notification(&self.view_id, GridNotification::DidUpdateGridSetting)
.payload(setting)
.send();

View File

@ -65,14 +65,14 @@ impl GridViewManager {
/// When the row was created, we may need to modify the [RowRevision] according to the [CreateRowParams].
pub(crate) async fn will_create_row(&self, row_rev: &mut RowRevision, params: &CreateRowParams) {
for view_editor in self.view_editors.iter() {
view_editor.will_create_row(row_rev, params).await;
view_editor.will_create_view_row(row_rev, params).await;
}
}
/// Notify the view that the row was created. For the moment, the view is just sending notifications.
pub(crate) async fn did_create_row(&self, row_pb: &RowPB, params: &CreateRowParams) {
for view_editor in self.view_editors.iter() {
view_editor.did_create_row(row_pb, params).await;
view_editor.did_create_view_row(row_pb, params).await;
}
}
@ -84,7 +84,7 @@ impl GridViewManager {
}
Some(row_rev) => {
for view_editor in self.view_editors.iter() {
view_editor.did_update_row(&row_rev).await;
view_editor.did_update_view_row(&row_rev).await;
}
}
}
@ -102,33 +102,33 @@ impl GridViewManager {
pub(crate) async fn did_delete_row(&self, row_rev: Arc<RowRevision>) {
for view_editor in self.view_editors.iter() {
view_editor.did_delete_row(&row_rev).await;
view_editor.did_delete_view_row(&row_rev).await;
}
}
pub(crate) async fn get_setting(&self) -> FlowyResult<GridSettingPB> {
let view_editor = self.get_default_view_editor().await?;
Ok(view_editor.get_setting().await)
Ok(view_editor.get_view_setting().await)
}
pub(crate) async fn get_filters(&self) -> FlowyResult<Vec<GridFilterConfigurationPB>> {
let view_editor = self.get_default_view_editor().await?;
Ok(view_editor.get_filters().await)
Ok(view_editor.get_view_filters().await)
}
pub(crate) async fn insert_or_update_filter(&self, params: InsertFilterParams) -> FlowyResult<()> {
let view_editor = self.get_default_view_editor().await?;
view_editor.insert_filter(params).await
view_editor.insert_view_filter(params).await
}
pub(crate) async fn delete_filter(&self, params: DeleteFilterParams) -> FlowyResult<()> {
let view_editor = self.get_default_view_editor().await?;
view_editor.delete_filter(params).await
view_editor.delete_view_filter(params).await
}
pub(crate) async fn load_groups(&self) -> FlowyResult<RepeatedGridGroupPB> {
let view_editor = self.get_default_view_editor().await?;
let groups = view_editor.load_groups().await?;
let groups = view_editor.load_view_groups().await?;
Ok(RepeatedGridGroupPB { items: groups })
}
@ -139,12 +139,12 @@ impl GridViewManager {
pub(crate) async fn delete_group(&self, params: DeleteGroupParams) -> FlowyResult<()> {
let view_editor = self.get_default_view_editor().await?;
view_editor.delete_group(params).await
view_editor.delete_view_group(params).await
}
pub(crate) async fn move_group(&self, params: MoveGroupParams) -> FlowyResult<()> {
let view_editor = self.get_default_view_editor().await?;
let _ = view_editor.move_group(params).await?;
let _ = view_editor.move_view_group(params).await?;
Ok(())
}
@ -161,7 +161,7 @@ impl GridViewManager {
let mut row_changeset = RowChangeset::new(row_rev.id.clone());
let view_editor = self.get_default_view_editor().await?;
let group_changesets = view_editor
.move_group_row(&row_rev, &mut row_changeset, &to_group_id, to_row_id.clone())
.move_view_group_row(&row_rev, &mut row_changeset, &to_group_id, to_row_id.clone())
.await;
if !row_changeset.is_empty() {

View File

@ -119,12 +119,20 @@ where
self.mut_configuration(|configuration| {
let from_index = configuration.groups.iter().position(|group| group.id == from_id);
let to_index = configuration.groups.iter().position(|group| group.id == to_id);
tracing::info!("Configuration groups: {:?} ", configuration.groups);
if let (Some(from), Some(to)) = &(from_index, to_index) {
tracing::trace!("Move group from index:{:?} to index:{:?}", from_index, to_index);
let group = configuration.groups.remove(*from);
configuration.groups.insert(*to, group);
}
tracing::debug!(
"Group order: {:?} ",
configuration
.groups
.iter()
.map(|group| group.name.clone())
.collect::<Vec<String>>()
.join(",")
);
from_index.is_some() && to_index.is_some()
})?;

View File

@ -80,9 +80,9 @@ pub fn move_group_row(group: &mut Group, context: &mut MoveGroupRowContext) -> O
};
// Remove the row in which group contains it
if from_index.is_some() {
if let Some(from_index) = &from_index {
changeset.deleted_rows.push(row_rev.id.clone());
tracing::debug!("Group:{} remove row:{}", group.id, row_rev.id);
tracing::debug!("Group:{} remove {} at {}", group.id, row_rev.id, from_index);
group.remove_row(&row_rev.id);
}
@ -97,10 +97,11 @@ pub fn move_group_row(group: &mut Group, context: &mut MoveGroupRowContext) -> O
}
Some(to_index) => {
if to_index < group.number_of_row() {
tracing::debug!("Group:{} insert row:{} at {} ", group.id, row_rev.id, to_index);
tracing::debug!("Group:{} insert {} at {} ", group.id, row_rev.id, to_index);
inserted_row.index = Some(to_index as i32);
group.insert_row(to_index, row_pb);
} else {
tracing::warn!("Mote to index: {} is out of bounds", to_index);
tracing::debug!("Group:{} append row:{}", group.id, row_rev.id);
group.add_row(row_pb);
}

View File

@ -350,6 +350,36 @@ async fn group_move_from_default_group_test() {
#[tokio::test]
async fn group_move_group_test() {
let mut test = GridGroupTest::new().await;
let group_0 = test.group_at_index(0).await;
let group_1 = test.group_at_index(1).await;
let scripts = vec![
MoveGroup {
from_group_index: 0,
to_group_index: 1,
},
AssertGroupRowCount {
group_index: 0,
row_count: 2,
},
AssertGroup {
group_index: 0,
expected_group: group_1,
},
AssertGroupRowCount {
group_index: 1,
row_count: 2,
},
AssertGroup {
group_index: 1,
expected_group: group_0,
},
];
test.run_scripts(scripts).await;
}
#[tokio::test]
async fn group_move_group_row_after_move_group_test() {
let mut test = GridGroupTest::new().await;
let group_0 = test.group_at_index(0).await;
let group_1 = test.group_at_index(1).await;
@ -366,6 +396,20 @@ async fn group_move_group_test() {
group_index: 1,
expected_group: group_0,
},
MoveRow {
from_group_index: 0,
from_row_index: 0,
to_group_index: 1,
to_row_index: 0,
},
AssertGroupRowCount {
group_index: 0,
row_count: 1,
},
AssertGroupRowCount {
group_index: 1,
row_count: 3,
},
];
test.run_scripts(scripts).await;
}