Skip to content

Commit

Permalink
fix: kanban UX bugs (#5227)
Browse files Browse the repository at this point in the history
* chore: improve title editing behavior

* chore: fix editable text field

* chore: fix autoscroll
  • Loading branch information
richardshiue authored Apr 30, 2024
1 parent 33802fa commit f354437
Show file tree
Hide file tree
Showing 22 changed files with 180 additions and 150 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const uint8_t *sync_event(const uint8_t *input, uintptr_t len);

int32_t set_stream_port(int64_t port);

int32_t set_log_stream_port(int64_t port);

void link_me_please(void);

void rust_log(int64_t level, const char *data);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import 'package:appflowy/generated/flowy_svgs.g.dart';
import 'package:appflowy/plugins/database/board/presentation/widgets/board_column_header.dart';
import 'package:appflowy/plugins/database/widgets/card/container/card_container.dart';
import 'package:appflowy/plugins/database/widgets/card/card.dart';
import 'package:appflowy_backend/protobuf/flowy-folder/view.pb.dart';
import 'package:appflowy_board/appflowy_board.dart';
import 'package:flutter/material.dart';
Expand All @@ -22,13 +22,15 @@ void main() {

await tester.createNewPageWithNameUnderParent(layout: ViewLayoutPB.Board);

final findFirstCard = find.descendant(
of: find.byType(AppFlowyGroupCard),
matching: find.byType(Text),
);
final firstCard = find.byType(RowCard).first;

Text firstCardText = tester.firstWidget(findFirstCard);
expect(firstCardText.data, defaultFirstCardName);
expect(
find.descendant(
of: firstCard,
matching: find.text(defaultFirstCardName),
),
findsOneWidget,
);

await tester.tap(
find
Expand All @@ -45,7 +47,7 @@ void main() {
const newCardName = 'Card 4';
await tester.enterText(
find.descendant(
of: find.byType(RowCardContainer),
of: firstCard,
matching: find.byType(TextField),
),
newCardName,
Expand All @@ -55,8 +57,13 @@ void main() {
await tester.tap(find.byType(AppFlowyBoard));
await tester.pumpAndSettle();

firstCardText = tester.firstWidget(findFirstCard);
expect(firstCardText.data, newCardName);
expect(
find.descendant(
of: find.byType(RowCard).first,
matching: find.text(newCardName),
),
findsOneWidget,
);
});

testWidgets('from footer', (tester) async {
Expand All @@ -65,13 +72,15 @@ void main() {

await tester.createNewPageWithNameUnderParent(layout: ViewLayoutPB.Board);

final findLastCard = find.descendant(
of: find.byType(AppFlowyGroupCard),
matching: find.byType(Text),
);
final lastCard = find.byType(RowCard).last;

Text? lastCardText = tester.widgetList(findLastCard).last as Text;
expect(lastCardText.data, defaultLastCardName);
expect(
find.descendant(
of: lastCard,
matching: find.text(defaultLastCardName),
),
findsOneWidget,
);

await tester.tap(
find
Expand All @@ -81,12 +90,11 @@ void main() {
)
.at(1),
);
await tester.pumpAndSettle();

const newCardName = 'Card 4';
await tester.enterText(
find.descendant(
of: find.byType(RowCardContainer),
of: lastCard,
matching: find.byType(TextField),
),
newCardName,
Expand All @@ -96,8 +104,13 @@ void main() {
await tester.tap(find.byType(AppFlowyBoard));
await tester.pumpAndSettle();

lastCardText = tester.widgetList(findLastCard).last as Text;
expect(lastCardText.data, newCardName);
expect(
find.descendant(
of: find.byType(RowCard).last,
matching: find.text(newCardName),
),
findsOneWidget,
);
});
});
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import 'package:appflowy/plugins/database/widgets/card/card.dart';
import 'package:appflowy/plugins/database/widgets/cell_editor/extension.dart';
import 'package:appflowy/plugins/database/widgets/row/row_property.dart';
import 'package:appflowy_backend/protobuf/flowy-folder/view.pb.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
import 'package:appflowy_board/appflowy_board.dart';

import '../../shared/util.dart';

Expand All @@ -20,7 +20,7 @@ void main() {
await tester.createNewPageWithNameUnderParent(layout: ViewLayoutPB.Board);
final card1 = find.ancestor(
of: find.text(card1Name),
matching: find.byType(AppFlowyGroupCard),
matching: find.byType(RowCard),
);
final doingGroup = find.text('Doing');
final doingGroupCenter = tester.getCenter(doingGroup);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class CellController<T, D> {
Timer? _loadDataOperation;
Timer? _saveDataOperation;

Completer? _completer;

RowId get rowId => _cellContext.rowId;
String get fieldId => _cellContext.fieldId;
FieldInfo get fieldInfo => _fieldController.getField(_cellContext.fieldId)!;
Expand Down Expand Up @@ -192,13 +194,15 @@ class CellController<T, D> {
_loadDataOperation?.cancel();
if (debounce) {
_saveDataOperation?.cancel();
_completer = Completer();
_saveDataOperation = Timer(const Duration(milliseconds: 300), () async {
final result = await _cellDataPersistence.save(
viewId: viewId,
cellContext: _cellContext,
data: data,
);
onFinish?.call(result);
_completer?.complete();
});
} else {
final result = await _cellDataPersistence.save(
Expand Down Expand Up @@ -241,6 +245,7 @@ class CellController<T, D> {
);

_loadDataOperation?.cancel();
await _completer?.future;
_saveDataOperation?.cancel();
_cellDataNotifier?.dispose();
_cellDataNotifier = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class FieldCellState with _$FieldCellState {
factory FieldCellState.initial(FieldInfo fieldInfo) => FieldCellState(
fieldInfo: fieldInfo,
isResizing: false,
width: fieldInfo.fieldSettings!.width.toDouble(),
width: fieldInfo.width!.toDouble(),
resizeStart: 0,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,12 @@ class BoardPage extends StatelessWidget {
)..add(const BoardEvent.initial()),
child: BlocBuilder<BoardBloc, BoardState>(
buildWhen: (p, c) => p.loadingState != c.loadingState,
builder: (context, state) => state.loadingState.map(
loading: (_) => const Center(
builder: (context, state) => state.loadingState.when(
loading: () => const Center(
child: CircularProgressIndicator.adaptive(),
),
finish: (result) => result.successOrFail.fold(
idle: () => const SizedBox.shrink(),
finish: (result) => result.fold(
(_) => PlatformExtension.isMobile
? const MobileBoardContent()
: DesktopBoardContent(onEditStateChanged: onEditStateChanged),
Expand All @@ -121,7 +122,6 @@ class BoardPage extends StatelessWidget {
howToFix: LocaleKeys.errorDialog_howToFixFallback.tr(),
),
),
idle: (_) => const SizedBox.shrink(),
),
),
);
Expand Down Expand Up @@ -154,6 +154,10 @@ class _DesktopBoardContentState extends State<DesktopBoardContent> {
stretchGroupHeight: false,
);

late final cellBuilder = CardCellBuilder(
databaseController: context.read<BoardBloc>().databaseController,
);

@override
void dispose() {
scrollController.dispose();
Expand All @@ -164,7 +168,6 @@ class _DesktopBoardContentState extends State<DesktopBoardContent> {
Widget build(BuildContext context) {
return BlocListener<BoardBloc, BoardState>(
listener: (context, state) {
_handleEditStateChanged(state, context);
widget.onEditStateChanged?.call();
},
child: BlocBuilder<BoardBloc, BoardState>(
Expand All @@ -182,7 +185,7 @@ class _DesktopBoardContentState extends State<DesktopBoardContent> {
leading: HiddenGroupsColumn(margin: config.groupHeaderPadding),
trailing: showCreateGroupButton
? BoardTrailing(scrollController: scrollController)
: null,
: const HSpace(40),
headerBuilder: (_, groupData) => BlocProvider<BoardBloc>.value(
value: context.read<BoardBloc>(),
child: BoardColumnHeader(
Expand All @@ -203,16 +206,6 @@ class _DesktopBoardContentState extends State<DesktopBoardContent> {
);
}

void _handleEditStateChanged(BoardState state, BuildContext context) {
if (state.isEditingRow && state.editingRow != null) {
WidgetsBinding.instance.addPostFrameCallback((_) {
if (state.editingRow!.index == null) {
scrollManager.scrollToBottom(state.editingRow!.group.groupId);
}
});
}
}

Widget _buildFooter(BuildContext context, AppFlowyGroupData columnData) {
return Padding(
padding: config.groupFooterPadding,
Expand Down Expand Up @@ -257,14 +250,13 @@ class _DesktopBoardContentState extends State<DesktopBoardContent> {
final databaseController = boardBloc.databaseController;
final viewId = boardBloc.viewId;

final cellBuilder = CardCellBuilder(databaseController: databaseController);
final isEditing = boardBloc.state.isEditingRow &&
boardBloc.state.editingRow?.row.id == groupItem.row.id;

final groupItemId = "${groupData.group.groupId}${groupItem.row.id}";
final rowMeta = rowInfo?.rowMeta ?? groupItem.row;

return AppFlowyGroupCard(
return Container(
key: ValueKey(groupItemId),
margin: config.cardMargin,
decoration: _makeBoxDecoration(context),
Expand Down Expand Up @@ -412,52 +404,50 @@ class _BoardTrailingState extends State<BoardTrailing> {
}
});

return Padding(
padding: const EdgeInsets.only(left: 8.0, top: 12),
child: Align(
alignment: AlignmentDirectional.topStart,
child: AnimatedSwitcher(
duration: const Duration(milliseconds: 300),
child: isEditing
? SizedBox(
width: 256,
child: Padding(
padding: const EdgeInsets.all(8.0),
child: TextField(
controller: _textController,
focusNode: _focusNode,
decoration: InputDecoration(
suffixIcon: Padding(
padding: const EdgeInsets.only(left: 4, bottom: 8.0),
child: FlowyIconButton(
icon: const FlowySvg(FlowySvgs.close_filled_m),
hoverColor: Colors.transparent,
onPressed: () => _textController.clear(),
),
return Container(
padding: const EdgeInsets.only(left: 8.0, top: 12, right: 40),
alignment: AlignmentDirectional.topStart,
child: AnimatedSwitcher(
duration: const Duration(milliseconds: 300),
child: isEditing
? SizedBox(
width: 256,
child: Padding(
padding: const EdgeInsets.all(8.0),
child: TextField(
controller: _textController,
focusNode: _focusNode,
decoration: InputDecoration(
suffixIcon: Padding(
padding: const EdgeInsets.only(left: 4, bottom: 8.0),
child: FlowyIconButton(
icon: const FlowySvg(FlowySvgs.close_filled_m),
hoverColor: Colors.transparent,
onPressed: () => _textController.clear(),
),
suffixIconConstraints:
BoxConstraints.loose(const Size(20, 24)),
border: const UnderlineInputBorder(),
contentPadding: const EdgeInsets.fromLTRB(8, 4, 8, 8),
isDense: true,
),
style: Theme.of(context).textTheme.bodySmall,
onSubmitted: (groupName) => context
.read<BoardBloc>()
.add(BoardEvent.createGroup(groupName)),
suffixIconConstraints:
BoxConstraints.loose(const Size(20, 24)),
border: const UnderlineInputBorder(),
contentPadding: const EdgeInsets.fromLTRB(8, 4, 8, 8),
isDense: true,
),
),
)
: FlowyTooltip(
message: LocaleKeys.board_column_createNewColumn.tr(),
child: FlowyIconButton(
width: 26,
icon: const FlowySvg(FlowySvgs.add_s),
iconColorOnHover: Theme.of(context).colorScheme.onSurface,
onPressed: () => setState(() => isEditing = true),
style: Theme.of(context).textTheme.bodySmall,
onSubmitted: (groupName) => context
.read<BoardBloc>()
.add(BoardEvent.createGroup(groupName)),
),
),
),
)
: FlowyTooltip(
message: LocaleKeys.board_column_createNewColumn.tr(),
child: FlowyIconButton(
width: 26,
icon: const FlowySvg(FlowySvgs.add_s),
iconColorOnHover: Theme.of(context).colorScheme.onSurface,
onPressed: () => setState(() => isEditing = true),
),
),
),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,22 @@ class HiddenGroupsColumn extends StatelessWidget {
? SizedBox(
height: 50,
child: Padding(
padding: const EdgeInsets.only(left: 40, right: 8),
padding: const EdgeInsets.only(left: 80, right: 8),
child: Center(
child: _collapseExpandIcon(context, isCollapsed),
),
),
)
: SizedBox(
width: 234,
width: 274,
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
SizedBox(
height: 50,
child: Padding(
padding: EdgeInsets.only(
left: 40 + margin.left,
left: 80 + margin.left,
right: margin.right + 4,
),
child: Row(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,13 @@ class _EventCardState extends State<EventCard> {
),
);
},
child: Container(
padding: widget.padding,
decoration: decoration,
child: card,
child: Material(
color: Colors.transparent,
child: Container(
padding: widget.padding,
decoration: decoration,
child: card,
),
),
);

Expand Down
Loading

0 comments on commit f354437

Please sign in to comment.