From cd245b5f0a3cc966758b725b0cfbca049e953400 Mon Sep 17 00:00:00 2001 From: Mathias Mogensen <42929161+Xazin@users.noreply.github.com> Date: Thu, 7 Mar 2024 14:04:10 +0100 Subject: [PATCH] fix: focus traversal in checklist popover (#4843) * fix: focus traversal in checklist popover * fix: dont trim input * chore: remove redundant state var * chore: remove late from controller --- .../desktop_grid_checklist_cell.dart | 8 ++-- .../cell_editor/checklist_cell_editor.dart | 42 +++++++++---------- .../appflowy_popover/lib/src/popover.dart | 10 ++++- .../src/flowy_overlay/appflowy_popover.dart | 6 +++ .../lib/style_widget/button.dart | 8 +++- 5 files changed, 48 insertions(+), 26 deletions(-) diff --git a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_grid/desktop_grid_checklist_cell.dart b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_grid/desktop_grid_checklist_cell.dart index 0db7329ffc..285b0217eb 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_grid/desktop_grid_checklist_cell.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell/desktop_grid/desktop_grid_checklist_cell.dart @@ -1,11 +1,12 @@ -import 'package:appflowy/plugins/database/grid/presentation/layout/sizes.dart'; -import 'package:appflowy/plugins/database/widgets/row/cells/cell_container.dart'; +import 'package:flutter/material.dart'; + import 'package:appflowy/plugins/database/application/cell/bloc/checklist_cell_bloc.dart'; +import 'package:appflowy/plugins/database/grid/presentation/layout/sizes.dart'; import 'package:appflowy/plugins/database/widgets/cell_editor/checklist_cell_editor.dart'; import 'package:appflowy/plugins/database/widgets/cell_editor/checklist_progress_bar.dart'; +import 'package:appflowy/plugins/database/widgets/row/cells/cell_container.dart'; import 'package:appflowy_popover/appflowy_popover.dart'; import 'package:flowy_infra_ui/flowy_infra_ui.dart'; -import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import '../editable_cell_skeleton/checklist.dart'; @@ -25,6 +26,7 @@ class DesktopGridChecklistCellSkin extends IEditableChecklistCellSkin { constraints: BoxConstraints.loose(const Size(360, 400)), direction: PopoverDirection.bottomWithLeftAligned, triggerActions: PopoverTriggerFlags.none, + skipTraversal: true, popupBuilder: (BuildContext popoverContext) { WidgetsBinding.instance.addPostFrameCallback((_) { cellContainerNotifier.isFocus = true; diff --git a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart index f985e26560..9299545f13 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/widgets/cell_editor/checklist_cell_editor.dart @@ -1,6 +1,9 @@ import 'dart:async'; import 'dart:io'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; + import 'package:appflowy/generated/flowy_svgs.g.dart'; import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/plugins/database/application/cell/cell_controller_builder.dart'; @@ -11,11 +14,10 @@ import 'package:easy_localization/easy_localization.dart'; import 'package:flowy_infra/size.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'; import '../../application/cell/bloc/checklist_cell_bloc.dart'; + import 'checklist_progress_bar.dart'; class ChecklistCellEditor extends StatefulWidget { @@ -345,12 +347,11 @@ class NewTaskItem extends StatefulWidget { } class _NewTaskItemState extends State { - late final TextEditingController _textEditingController; + final _textEditingController = TextEditingController(); @override void initState() { super.initState(); - _textEditingController = TextEditingController(); if (widget.focusNode.canRequestFocus) { widget.focusNode.requestFocus(); } @@ -385,15 +386,13 @@ class _NewTaskItemState extends State { hintText: LocaleKeys.grid_checklist_addNew.tr(), ), onSubmitted: (taskDescription) { - if (taskDescription.trim().isNotEmpty) { - context.read().add( - ChecklistCellEvent.createNewTask( - taskDescription.trim(), - ), - ); + if (taskDescription.isNotEmpty) { + context + .read() + .add(ChecklistCellEvent.createNewTask(taskDescription)); + _textEditingController.clear(); } widget.focusNode.requestFocus(); - _textEditingController.clear(); }, onChanged: (value) => setState(() {}), ), @@ -409,16 +408,17 @@ class _NewTaskItemState extends State { : Theme.of(context).colorScheme.primaryContainer, fontColor: Theme.of(context).colorScheme.onPrimary, padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 3), - onPressed: () { - final text = _textEditingController.text.trim(); - if (text.isNotEmpty) { - context.read().add( - ChecklistCellEvent.createNewTask(text), - ); - } - widget.focusNode.requestFocus(); - _textEditingController.clear(); - }, + onPressed: _textEditingController.text.isEmpty + ? null + : () { + context.read().add( + ChecklistCellEvent.createNewTask( + _textEditingController.text, + ), + ); + widget.focusNode.requestFocus(); + _textEditingController.clear(); + }, ), ], ), diff --git a/frontend/appflowy_flutter/packages/appflowy_popover/lib/src/popover.dart b/frontend/appflowy_flutter/packages/appflowy_popover/lib/src/popover.dart index 4c93b40e78..17eefc44f0 100644 --- a/frontend/appflowy_flutter/packages/appflowy_popover/lib/src/popover.dart +++ b/frontend/appflowy_flutter/packages/appflowy_popover/lib/src/popover.dart @@ -1,7 +1,8 @@ -import 'package:appflowy_popover/src/layout.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import 'package:appflowy_popover/src/layout.dart'; + import 'mask.dart'; import 'mutex.dart'; @@ -90,6 +91,8 @@ class Popover extends StatefulWidget { /// the conflict won't be resolve by using Listener, we want these two gestures exclusive. final PopoverClickHandler clickHandler; + final bool skipTraversal; + /// The content area of the popover. final Widget child; @@ -110,6 +113,7 @@ class Popover extends StatefulWidget { this.canClose, this.asBarrier = false, this.clickHandler = PopoverClickHandler.listener, + this.skipTraversal = false, }); @override @@ -158,6 +162,7 @@ class PopoverState extends State { popupBuilder: widget.popupBuilder, onClose: () => close(), onCloseAll: () => _removeRootOverlay(), + skipTraversal: widget.skipTraversal, ), ); @@ -263,6 +268,7 @@ class PopoverContainer extends StatefulWidget { final EdgeInsets windowPadding; final void Function() onClose; final void Function() onCloseAll; + final bool skipTraversal; const PopoverContainer({ super.key, @@ -273,6 +279,7 @@ class PopoverContainer extends StatefulWidget { required this.windowPadding, required this.onClose, required this.onCloseAll, + required this.skipTraversal, }); @override @@ -293,6 +300,7 @@ class PopoverContainerState extends State { Widget build(BuildContext context) { return Focus( autofocus: true, + skipTraversal: widget.skipTraversal, child: CustomSingleChildLayout( delegate: PopoverLayoutDelegate( direction: widget.direction, diff --git a/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/src/flowy_overlay/appflowy_popover.dart b/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/src/flowy_overlay/appflowy_popover.dart index aa94f0cc02..0a3ff0a119 100644 --- a/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/src/flowy_overlay/appflowy_popover.dart +++ b/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/src/flowy_overlay/appflowy_popover.dart @@ -26,6 +26,10 @@ class AppFlowyPopover extends StatelessWidget { /// the conflict won't be resolve by using Listener, we want these two gestures exclusive. final PopoverClickHandler clickHandler; + /// If true the popover will not participate in focus traversal. + /// + final bool skipTraversal; + const AppFlowyPopover({ super.key, required this.child, @@ -43,6 +47,7 @@ class AppFlowyPopover extends StatelessWidget { this.windowPadding = const EdgeInsets.all(8.0), this.decoration, this.clickHandler = PopoverClickHandler.listener, + this.skipTraversal = false, }); @override @@ -58,6 +63,7 @@ class AppFlowyPopover extends StatelessWidget { windowPadding: windowPadding, offset: offset, clickHandler: clickHandler, + skipTraversal: skipTraversal, popupBuilder: (context) { return _PopoverContainer( constraints: constraints, diff --git a/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/style_widget/button.dart b/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/style_widget/button.dart index 4f029c3699..dced70a6be 100644 --- a/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/style_widget/button.dart +++ b/frontend/appflowy_flutter/packages/flowy_infra_ui/lib/style_widget/button.dart @@ -1,12 +1,13 @@ import 'dart:io'; +import 'package:flutter/material.dart'; + import 'package:flowy_infra/size.dart'; import 'package:flowy_infra_ui/style_widget/hover.dart'; import 'package:flowy_infra_ui/style_widget/text.dart'; import 'package:flowy_infra_ui/widget/flowy_tooltip.dart'; import 'package:flowy_infra_ui/widget/ignore_parent_gesture.dart'; import 'package:flowy_infra_ui/widget/spacing.dart'; -import 'package:flutter/material.dart'; class FlowyButton extends StatelessWidget { final Widget text; @@ -213,6 +214,7 @@ class FlowyTextButton extends StatelessWidget { ); child = RawMaterialButton( + focusNode: FocusNode(skipTraversal: onPressed == null), hoverElevation: 0, highlightElevation: 0, shape: RoundedRectangleBorder(borderRadius: radius ?? Corners.s6Border), @@ -237,6 +239,10 @@ class FlowyTextButton extends StatelessWidget { ); } + if (onPressed == null) { + child = ExcludeFocus(child: child); + } + return child; } }