Skip to content

Commit 48940ef

Browse files
appflowyLucasXu0
authored andcommitted
fix: update the cell content if input is not valid data (#1652)
Co-authored-by: nathan <[email protected]>
1 parent a4a2a5f commit 48940ef

File tree

7 files changed

+98
-58
lines changed

7 files changed

+98
-58
lines changed

frontend/app_flowy/lib/plugins/grid/application/cell/cell_service/cell_controller.dart

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ class IGridCellController<T, D> extends Equatable {
133133
final IGridCellDataPersistence<D> _cellDataPersistence;
134134

135135
CellListener? _cellListener;
136-
ValueNotifier<T?>? _cellDataNotifier;
136+
CellDataNotifier<T?>? _cellDataNotifier;
137137

138138
bool isListening = false;
139139
VoidCallback? _onFieldChangedFn;
@@ -170,8 +170,20 @@ class IGridCellController<T, D> extends Equatable {
170170

171171
FieldType get fieldType => cellId.fieldInfo.fieldType;
172172

173+
/// Listen on the cell content or field changes
174+
///
175+
/// An optional [listenWhenOnCellChanged] can be implemented for more
176+
/// granular control over when [listener] is called.
177+
/// [listenWhenOnCellChanged] will be invoked on each [onCellChanged]
178+
/// get called.
179+
/// [listenWhenOnCellChanged] takes the previous `value` and current
180+
/// `value` and must return a [bool] which determines whether or not
181+
/// the [onCellChanged] function will be invoked.
182+
/// [onCellChanged] is optional and if omitted, it will default to `true`.
183+
///
173184
VoidCallback? startListening({
174185
required void Function(T?) onCellChanged,
186+
bool Function(T? oldValue, T? newValue)? listenWhenOnCellChanged,
175187
VoidCallback? onCellFieldChanged,
176188
}) {
177189
if (isListening) {
@@ -180,7 +192,10 @@ class IGridCellController<T, D> extends Equatable {
180192
}
181193
isListening = true;
182194

183-
_cellDataNotifier = ValueNotifier(_cellsCache.get(_cacheKey));
195+
_cellDataNotifier = CellDataNotifier(
196+
value: _cellsCache.get(_cacheKey),
197+
listenWhen: listenWhenOnCellChanged,
198+
);
184199
_cellListener =
185200
CellListener(rowId: cellId.rowId, fieldId: cellId.fieldInfo.id);
186201

@@ -255,24 +270,21 @@ class IGridCellController<T, D> extends Equatable {
255270
/// You can set [deduplicate] to true (default is false) to reduce the save operation.
256271
/// It's useful when you call this method when user editing the [TextField].
257272
/// The default debounce interval is 300 milliseconds.
258-
void saveCellData(D data,
259-
{bool deduplicate = false,
260-
void Function(Option<FlowyError>)? resultCallback}) async {
273+
void saveCellData(
274+
D data, {
275+
bool deduplicate = false,
276+
void Function(Option<FlowyError>)? onFinish,
277+
}) async {
278+
_loadDataOperation?.cancel();
261279
if (deduplicate) {
262-
_loadDataOperation?.cancel();
263-
264280
_saveDataOperation?.cancel();
265281
_saveDataOperation = Timer(const Duration(milliseconds: 300), () async {
266282
final result = await _cellDataPersistence.save(data);
267-
if (resultCallback != null) {
268-
resultCallback(result);
269-
}
283+
onFinish?.call(result);
270284
});
271285
} else {
272286
final result = await _cellDataPersistence.save(data);
273-
if (resultCallback != null) {
274-
resultCallback(result);
275-
}
287+
onFinish?.call(result);
276288
}
277289
}
278290

@@ -302,6 +314,7 @@ class IGridCellController<T, D> extends Equatable {
302314
await _cellListener?.stop();
303315
_loadDataOperation?.cancel();
304316
_saveDataOperation?.cancel();
317+
_cellDataNotifier?.dispose();
305318
_cellDataNotifier = null;
306319

307320
if (_onFieldChangedFn != null) {
@@ -343,3 +356,23 @@ class GridCellFieldNotifierImpl extends IGridCellFieldNotifier {
343356
);
344357
}
345358
}
359+
360+
class CellDataNotifier<T> extends ChangeNotifier {
361+
T _value;
362+
bool Function(T? oldValue, T? newValue)? listenWhen;
363+
CellDataNotifier({required T value, this.listenWhen}) : _value = value;
364+
365+
set value(T newValue) {
366+
if (listenWhen?.call(_value, newValue) ?? false) {
367+
_value = newValue;
368+
notifyListeners();
369+
} else {
370+
if (_value != newValue) {
371+
_value = newValue;
372+
notifyListeners();
373+
}
374+
}
375+
}
376+
377+
T get value => _value;
378+
}

frontend/app_flowy/lib/plugins/grid/application/cell/date_cal_bloc.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class DateCalBloc extends Bloc<DateCalEvent, DateCalState> {
102102
}
103103
}
104104

105-
cellController.saveCellData(newCalData, resultCallback: (result) {
105+
cellController.saveCellData(newCalData, onFinish: (result) {
106106
result.fold(
107107
() => updateCalData(Some(newCalData), none()),
108108
(err) {
Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import 'package:flowy_sdk/protobuf/flowy-error/errors.pb.dart';
1+
import 'package:flowy_sdk/log.dart';
22
import 'package:flutter_bloc/flutter_bloc.dart';
33
import 'package:freezed_annotation/freezed_annotation.dart';
44
import 'dart:async';
5-
import 'package:dartz/dartz.dart';
65
import 'cell_service/cell_service.dart';
76

87
part 'number_cell_bloc.freezed.dart';
@@ -20,20 +19,19 @@ class NumberCellBloc extends Bloc<NumberCellEvent, NumberCellState> {
2019
initial: () {
2120
_startListening();
2221
},
23-
didReceiveCellUpdate: (content) {
24-
emit(state.copyWith(content: content));
22+
didReceiveCellUpdate: (cellContent) {
23+
emit(state.copyWith(cellContent: cellContent ?? ""));
2524
},
2625
updateCell: (text) {
27-
cellController.saveCellData(text, resultCallback: (result) {
28-
result.fold(
29-
() => null,
30-
(err) {
31-
if (!isClosed) {
32-
add(NumberCellEvent.didReceiveCellUpdate(right(err)));
33-
}
34-
},
35-
);
36-
});
26+
if (state.cellContent != text) {
27+
emit(state.copyWith(cellContent: text));
28+
cellController.saveCellData(text, onFinish: (result) {
29+
result.fold(
30+
() {},
31+
(err) => Log.error(err),
32+
);
33+
});
34+
}
3735
},
3836
);
3937
},
@@ -51,34 +49,42 @@ class NumberCellBloc extends Bloc<NumberCellEvent, NumberCellState> {
5149
}
5250

5351
void _startListening() {
54-
_onCellChangedFn = cellController.startListening(
55-
onCellChanged: ((cellContent) {
56-
if (!isClosed) {
57-
add(NumberCellEvent.didReceiveCellUpdate(left(cellContent ?? "")));
58-
}
59-
}),
60-
);
52+
_onCellChangedFn =
53+
cellController.startListening(onCellChanged: ((cellContent) {
54+
if (!isClosed) {
55+
add(NumberCellEvent.didReceiveCellUpdate(cellContent));
56+
}
57+
}), listenWhenOnCellChanged: (oldValue, newValue) {
58+
// If the new value is not the same as the content, which means the
59+
// backend formatted the content that user enter. For example:
60+
//
61+
// state.cellContent: "abc"
62+
// oldValue: ""
63+
// newValue: ""
64+
// The oldValue is the same as newValue. So the [onCellChanged] won't
65+
// get called. So just return true to refresh the cell content
66+
return true;
67+
});
6168
}
6269
}
6370

6471
@freezed
6572
class NumberCellEvent with _$NumberCellEvent {
6673
const factory NumberCellEvent.initial() = _Initial;
6774
const factory NumberCellEvent.updateCell(String text) = _UpdateCell;
68-
const factory NumberCellEvent.didReceiveCellUpdate(
69-
Either<String, FlowyError> cellContent) = _DidReceiveCellUpdate;
75+
const factory NumberCellEvent.didReceiveCellUpdate(String? cellContent) =
76+
_DidReceiveCellUpdate;
7077
}
7178

7279
@freezed
7380
class NumberCellState with _$NumberCellState {
7481
const factory NumberCellState({
75-
required Either<String, FlowyError> content,
82+
required String cellContent,
7683
}) = _NumberCellState;
7784

7885
factory NumberCellState.initial(GridCellController context) {
79-
final cellContent = context.getCellData() ?? "";
8086
return NumberCellState(
81-
content: left(cellContent),
87+
cellContent: context.getCellData() ?? "",
8288
);
8389
}
8490
}

frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/number_cell.dart

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ class _NumberCellState extends GridFocusNodeCellState<GridNumberCell> {
2929
final cellController = widget.cellControllerBuilder.build();
3030
_cellBloc = getIt<NumberCellBloc>(param1: cellController)
3131
..add(const NumberCellEvent.initial());
32-
_controller =
33-
TextEditingController(text: contentFromState(_cellBloc.state));
32+
_controller = TextEditingController(text: _cellBloc.state.cellContent);
3433
super.initState();
3534
}
3635

@@ -41,9 +40,8 @@ class _NumberCellState extends GridFocusNodeCellState<GridNumberCell> {
4140
child: MultiBlocListener(
4241
listeners: [
4342
BlocListener<NumberCellBloc, NumberCellState>(
44-
listenWhen: (p, c) => p.content != c.content,
45-
listener: (context, state) =>
46-
_controller.text = contentFromState(state),
43+
listenWhen: (p, c) => p.cellContent != c.cellContent,
44+
listener: (context, state) => _controller.text = state.cellContent,
4745
),
4846
],
4947
child: Padding(
@@ -80,20 +78,16 @@ class _NumberCellState extends GridFocusNodeCellState<GridNumberCell> {
8078
_delayOperation?.cancel();
8179
_delayOperation = Timer(const Duration(milliseconds: 30), () {
8280
if (_cellBloc.isClosed == false &&
83-
_controller.text != contentFromState(_cellBloc.state)) {
81+
_controller.text != _cellBloc.state.cellContent) {
8482
_cellBloc.add(NumberCellEvent.updateCell(_controller.text));
8583
}
8684
});
8785
}
8886
}
8987

90-
String contentFromState(NumberCellState state) {
91-
return state.content.fold((l) => l, (r) => "");
92-
}
93-
9488
@override
9589
String? onCopy() {
96-
return _cellBloc.state.content.fold((content) => content, (r) => null);
90+
return _cellBloc.state.cellContent;
9791
}
9892

9993
@override

frontend/rust-lib/flowy-grid/src/services/field/type_options/number_type_option/number_type_option_entities.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::services::cell::{CellBytesCustomParser, CellProtobufBlobParser, Decod
22
use crate::services::field::number_currency::Currency;
33
use crate::services::field::{strip_currency_symbol, NumberFormat, STRIP_SYMBOL};
44
use bytes::Bytes;
5-
use flowy_error::{FlowyError, FlowyResult};
5+
use flowy_error::FlowyResult;
66
use rust_decimal::Decimal;
77
use rusty_money::Money;
88
use std::str::FromStr;
@@ -40,7 +40,8 @@ impl NumberCellData {
4040
if num_str.chars().all(char::is_numeric) {
4141
Self::from_format_str(&num_str, sign_positive, format)
4242
} else {
43-
Err(FlowyError::invalid_data().context("Should only contain numbers"))
43+
// returns empty string if it can be formatted
44+
Ok(Self::default())
4445
}
4546
}
4647
},

frontend/rust-lib/flowy-grid/src/services/field/type_options/type_option_cell.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ impl CellDataCacheKey {
6060
}
6161
hasher.write(field_rev.id.as_bytes());
6262
hasher.write_u8(decoded_field_type as u8);
63-
hasher.write(cell_str.as_bytes());
63+
cell_str.hash(&mut hasher);
6464
Self(hasher.finish())
6565
}
6666
}
@@ -125,8 +125,14 @@ where
125125
}
126126
}
127127

128-
let cell_data = self.decode_cell_str(cell_str, decoded_field_type, field_rev)?;
128+
let cell_data = self.decode_cell_str(cell_str.clone(), decoded_field_type, field_rev)?;
129129
if let Some(cell_data_cache) = self.cell_data_cache.as_ref() {
130+
tracing::trace!(
131+
"Cell cache update: field_type:{}, cell_str: {}, cell_data: {:?}",
132+
decoded_field_type,
133+
cell_str,
134+
cell_data
135+
);
130136
cell_data_cache.write().insert(key.as_ref(), cell_data.clone());
131137
}
132138
Ok(cell_data)
@@ -140,12 +146,13 @@ where
140146
) {
141147
if let Some(cell_data_cache) = self.cell_data_cache.as_ref() {
142148
let field_type: FieldType = field_rev.ty.into();
149+
let key = CellDataCacheKey::new(field_rev, field_type.clone(), cell_str);
143150
tracing::trace!(
144-
"Update cell cache field_type: {}, cell_data: {:?}",
151+
"Cell cache update: field_type:{}, cell_str: {}, cell_data: {:?}",
145152
field_type,
153+
cell_str,
146154
cell_data
147155
);
148-
let key = CellDataCacheKey::new(field_rev, field_type, cell_str);
149156
cell_data_cache.write().insert(key.as_ref(), cell_data);
150157
}
151158
}

frontend/rust-lib/flowy-user/src/services/database.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ impl UserDB {
1717
}
1818
}
1919

20-
#[tracing::instrument(level = "trace", skip(self))]
2120
fn open_user_db_if_need(&self, user_id: &str) -> Result<Arc<ConnectionPool>, FlowyError> {
2221
if user_id.is_empty() {
2322
return Err(ErrorCode::UserIdIsEmpty.into());

0 commit comments

Comments
 (0)