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
This commit is contained in:
Richard Shiue 2023-12-04 10:22:26 +08:00 committed by GitHub
parent 4ae679128f
commit 5fa441cbf5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 146 additions and 214 deletions

View File

@ -25,7 +25,7 @@
"preLaunchTask": "AF: Build Appflowy Core",
"env": {
"RUST_LOG": "trace",
"RUST_BACKTRACE": 1
"RUST_BACKTRACE": "1"
},
"cwd": "${workspaceRoot}/appflowy_flutter"
},

View File

@ -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;
}
}

View File

@ -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;
});
}

View File

@ -127,9 +127,12 @@ class _DesktopBoardContentState extends State<DesktopBoardContent> {
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<DesktopBoardContent> {
scrollController: scrollController,
controller: context.read<BoardBloc>().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<DesktopBoardContent> {
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),

View File

@ -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 [];

View File

@ -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;

View File

@ -61,6 +61,7 @@ class _DateCellState extends State<DateCardCell> {
state.dateStr,
fontSize: 11,
color: Theme.of(context).hintColor,
overflow: TextOverflow.ellipsis,
),
),
);

View File

@ -85,7 +85,11 @@ class _DateCellState extends GridCellState<GridDateCell> {
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(

View File

@ -67,28 +67,26 @@ class _NumberCellState extends GridEditableTextCell<GridNumberCell> {
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(),
),
),
);

View File

@ -77,65 +77,58 @@ class _GridTextCellState extends GridEditableTextCell<GridTextCell> {
_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<TextCellBloc, TextCellState>(
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<TextCellBloc, TextCellState>(
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(),
),
),
],
),
),
);

View File

@ -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<GridURLCellAccessoryType> 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<GridURLCell> createState() => _GridURLCellState();
@ -65,13 +63,11 @@ class GridURLCell extends GridCellWidget {
GridCellAccessoryBuildContext buildContext,
) get accessoryBuilder => (buildContext) {
final List<GridCellAccessoryBuilder> 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<GridURLCell> {
_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<GridURLCell> {
@override
Future<void> 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<GridURLCell> {
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<StatefulWidget> 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<State<_CopyURLAccessory>>;

View File

@ -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,

View File

@ -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<usize>) {
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);
}
}

View File

@ -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 {

View File

@ -101,10 +101,7 @@ impl GroupCustomize for SingleSelectGroupController {
) -> FlowyResult<(Option<TypeOptionData>, Option<InsertedGroupPB>)> {
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)?;

View File

@ -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);
}