From 243a781b6cf905f44c1a803a803f31ba624e0e81 Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Fri, 27 Jan 2023 10:35:17 +0800 Subject: [PATCH] chore: improve grid focus and hover event handling (#1735) * chore: improve primary cell accessory behavior * fix: focus border disappearing * chore: port to GridCellState * chore: fix typo * chore: connect popover controller * chore: final --- .../card/board_checklist_cell.dart | 7 +- .../widgets/cell/cell_accessory.dart | 20 ++--- .../widgets/cell/cell_container.dart | 42 +++++++---- .../cell/checklist_cell/checklist_cell.dart | 75 +++++++------------ .../checklist_cell/checklist_cell_editor.dart | 4 +- ...s_bar.dart => checklist_progress_bar.dart} | 16 ++-- .../select_option_cell.dart | 63 ++++++++-------- .../presentation/widgets/row/grid_row.dart | 4 +- 8 files changed, 109 insertions(+), 122 deletions(-) rename frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/{checklist_prograss_bar.dart => checklist_progress_bar.dart} (84%) diff --git a/frontend/app_flowy/lib/plugins/board/presentation/card/board_checklist_cell.dart b/frontend/app_flowy/lib/plugins/board/presentation/card/board_checklist_cell.dart index 1acbb07f33..1f7af63b3e 100644 --- a/frontend/app_flowy/lib/plugins/board/presentation/card/board_checklist_cell.dart +++ b/frontend/app_flowy/lib/plugins/board/presentation/card/board_checklist_cell.dart @@ -1,6 +1,6 @@ import 'package:app_flowy/plugins/grid/application/cell/cell_service/cell_service.dart'; import 'package:app_flowy/plugins/grid/application/cell/checklist_cell_bloc.dart'; -import 'package:app_flowy/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell.dart'; +import 'package:app_flowy/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_progress_bar.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; @@ -29,7 +29,10 @@ class _BoardChecklistCellState extends State { Widget build(BuildContext context) { return BlocProvider.value( value: _cellBloc, - child: const ChecklistProgressBar(), + child: BlocBuilder( + builder: (context, state) => + ChecklistProgressBar(percent: state.percent), + ), ); } } diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_accessory.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_accessory.dart index 33d446e359..de4b685b64 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_accessory.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_accessory.dart @@ -67,18 +67,14 @@ class _PrimaryCellAccessoryState extends State with GridCellAccessoryState { @override Widget build(BuildContext context) { - if (widget.isCellEditing) { - return const SizedBox(); - } else { - return Tooltip( - message: LocaleKeys.tooltip_openAsPage.tr(), - textStyle: AFThemeExtension.of(context).caption.textColor(Colors.white), - child: svgWidget( - "grid/expander", - color: Theme.of(context).colorScheme.primary, - ), - ); - } + return Tooltip( + message: LocaleKeys.tooltip_openAsPage.tr(), + textStyle: AFThemeExtension.of(context).caption.textColor(Colors.white), + child: svgWidget( + "grid/expander", + color: Theme.of(context).colorScheme.primary, + ), + ); } @override diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_container.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_container.dart index a5e645c039..d3fdb1aec6 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_container.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/cell_container.dart @@ -12,20 +12,23 @@ class CellContainer extends StatelessWidget { final GridCellWidget child; final AccessoryBuilder? accessoryBuilder; final double width; - final RegionStateNotifier rowStateNotifier; + final bool isPrimary; + final CellContainerNotifier cellContainerNotifier; + const CellContainer({ Key? key, required this.child, required this.width, - required this.rowStateNotifier, + required this.isPrimary, + required this.cellContainerNotifier, this.accessoryBuilder, }) : super(key: key); @override Widget build(BuildContext context) { - return ChangeNotifierProvider<_CellContainerNotifier>( - create: (_) => _CellContainerNotifier(child), - child: Selector<_CellContainerNotifier, bool>( + return ChangeNotifierProvider.value( + value: cellContainerNotifier, + child: Selector( selector: (context, notifier) => notifier.isFocus, builder: (privderContext, isFocus, _) { Widget container = Center(child: GridCellShortcuts(child: child)); @@ -41,6 +44,7 @@ class CellContainer extends StatelessWidget { if (accessories.isNotEmpty) { container = _GridCellEnterRegion( accessories: accessories, + isPrimary: isPrimary, child: container, ); } @@ -81,17 +85,23 @@ class CellContainer extends StatelessWidget { class _GridCellEnterRegion extends StatelessWidget { final Widget child; final List accessories; - const _GridCellEnterRegion( - {required this.child, required this.accessories, Key? key}) - : super(key: key); + final bool isPrimary; + const _GridCellEnterRegion({ + required this.child, + required this.accessories, + required this.isPrimary, + Key? key, + }) : super(key: key); @override Widget build(BuildContext context) { - return Selector2( - selector: (context, regionNotifier, cellNotifier) => !cellNotifier.isFocus && (cellNotifier.onEnter), - builder: (context, onEnter, _) { + return Selector2( + selector: (context, regionNotifier, cellNotifier) => + !cellNotifier.isFocus && + (cellNotifier.onEnter || regionNotifier.onEnter && isPrimary), + builder: (context, showAccessory, _) { List children = [child]; - if (onEnter) { + if (showAccessory) { children.add( CellAccessoryContainer(accessories: accessories).positioned( right: GridSize.cellContentInsets.right, @@ -102,10 +112,10 @@ class _GridCellEnterRegion extends StatelessWidget { return MouseRegion( cursor: SystemMouseCursors.click, onEnter: (p) => - Provider.of<_CellContainerNotifier>(context, listen: false) + Provider.of(context, listen: false) .onEnter = true, onExit: (p) => - Provider.of<_CellContainerNotifier>(context, listen: false) + Provider.of(context, listen: false) .onEnter = false, child: Stack( alignment: AlignmentDirectional.center, @@ -118,13 +128,13 @@ class _GridCellEnterRegion extends StatelessWidget { } } -class _CellContainerNotifier extends ChangeNotifier { +class CellContainerNotifier extends ChangeNotifier { final CellEditable cellEditable; VoidCallback? _onCellFocusListener; bool _isFocus = false; bool _onEnter = false; - _CellContainerNotifier(this.cellEditable) { + CellContainerNotifier(this.cellEditable) { _onCellFocusListener = () => isFocus = cellEditable.onCellFocus.value; cellEditable.onCellFocus.addListener(_onCellFocusListener!); } diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell.dart index 47d6342f46..0c9548974c 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell.dart @@ -5,9 +5,10 @@ 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 '../cell_builder.dart'; import 'checklist_cell_editor.dart'; -import 'checklist_prograss_bar.dart'; +import 'checklist_progress_bar.dart'; class GridChecklistCell extends GridCellWidget { final GridCellControllerBuilder cellControllerBuilder; @@ -15,12 +16,12 @@ class GridChecklistCell extends GridCellWidget { : super(key: key); @override - GridChecklistCellState createState() => GridChecklistCellState(); + GridCellState createState() => GridChecklistCellState(); } -class GridChecklistCellState extends State { - late PopoverController _popover; +class GridChecklistCellState extends GridCellState { late ChecklistCellBloc _cellBloc; + late final PopoverController _popover; @override void initState() { @@ -36,52 +37,32 @@ class GridChecklistCellState extends State { Widget build(BuildContext context) { return BlocProvider.value( value: _cellBloc, - child: Stack( - alignment: AlignmentDirectional.center, - fit: StackFit.expand, - children: [ - _wrapPopover(const ChecklistProgressBar()), - InkWell(onTap: () => _popover.show()), - ], + child: AppFlowyPopover( + controller: _popover, + constraints: BoxConstraints.loose(const Size(260, 400)), + direction: PopoverDirection.bottomWithLeftAligned, + triggerActions: PopoverTriggerFlags.none, + popupBuilder: (BuildContext context) { + WidgetsBinding.instance.addPostFrameCallback((_) { + widget.onCellEditing.value = true; + }); + return GridChecklistCellEditor( + cellController: widget.cellControllerBuilder.build() + as GridChecklistCellController, + ); + }, + onClose: () => widget.onCellEditing.value = false, + child: Padding( + padding: GridSize.cellContentInsets, + child: BlocBuilder( + builder: (context, state) => + ChecklistProgressBar(percent: state.percent), + ), + ), ), ); } - Widget _wrapPopover(Widget child) { - return AppFlowyPopover( - controller: _popover, - constraints: BoxConstraints.loose(const Size(260, 400)), - direction: PopoverDirection.bottomWithLeftAligned, - triggerActions: PopoverTriggerFlags.none, - popupBuilder: (BuildContext context) { - WidgetsBinding.instance.addPostFrameCallback((_) { - widget.onCellEditing.value = true; - }); - return GridChecklistCellEditor( - cellController: widget.cellControllerBuilder.build() - as GridChecklistCellController, - ); - }, - onClose: () => widget.onCellEditing.value = false, - child: Padding( - padding: GridSize.cellContentInsets, - child: child, - ), - ); - } -} - -class ChecklistProgressBar extends StatelessWidget { - const ChecklistProgressBar({Key? key}) : super(key: key); - @override - Widget build(BuildContext context) { - return BlocBuilder( - builder: (context, state) { - return ChecklistPrograssBar( - percent: state.percent, - ); - }, - ); - } + void requestBeginFocus() => _popover.show(); } diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell_editor.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell_editor.dart index 3c0810f3b5..281f6a954c 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell_editor.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_cell_editor.dart @@ -1,7 +1,7 @@ import 'package:app_flowy/plugins/grid/application/cell/cell_service/cell_service.dart'; import 'package:app_flowy/plugins/grid/application/cell/checklist_cell_editor_bloc.dart'; import 'package:app_flowy/plugins/grid/presentation/layout/sizes.dart'; -import 'package:app_flowy/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_prograss_bar.dart'; +import 'package:app_flowy/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_progress_bar.dart'; import 'package:app_flowy/plugins/grid/presentation/widgets/header/type_option/select_option_editor.dart'; import 'package:appflowy_popover/appflowy_popover.dart'; import 'package:flowy_infra/image.dart'; @@ -48,7 +48,7 @@ class _GridChecklistCellEditorState extends State { child: BlocBuilder( builder: (context, state) { final List slivers = [ - const SliverChecklistPrograssBar(), + const SliverChecklistProgressBar(), SliverToBoxAdapter( child: Padding( padding: GridSize.typeOptionContentInsets, diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_prograss_bar.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_progress_bar.dart similarity index 84% rename from frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_prograss_bar.dart rename to frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_progress_bar.dart index 8f6965a8ef..317d9dacff 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_prograss_bar.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/checklist_cell/checklist_progress_bar.dart @@ -8,9 +8,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:percent_indicator/percent_indicator.dart'; -class ChecklistPrograssBar extends StatelessWidget { +class ChecklistProgressBar extends StatelessWidget { final double percent; - const ChecklistPrograssBar({required this.percent, Key? key}) + const ChecklistProgressBar({required this.percent, Key? key}) : super(key: key); @override @@ -26,21 +26,21 @@ class ChecklistPrograssBar extends StatelessWidget { } } -class SliverChecklistPrograssBar extends StatelessWidget { - const SliverChecklistPrograssBar({Key? key}) : super(key: key); +class SliverChecklistProgressBar extends StatelessWidget { + const SliverChecklistProgressBar({Key? key}) : super(key: key); @override Widget build(BuildContext context) { return SliverPersistentHeader( pinned: true, - delegate: _SliverChecklistPrograssBarDelegate(), + delegate: _SliverChecklistProgressBarDelegate(), ); } } -class _SliverChecklistPrograssBarDelegate +class _SliverChecklistProgressBarDelegate extends SliverPersistentHeaderDelegate { - _SliverChecklistPrograssBarDelegate(); + _SliverChecklistProgressBarDelegate(); double fixHeight = 60; @@ -71,7 +71,7 @@ class _SliverChecklistPrograssBarDelegate ), Padding( padding: const EdgeInsets.only(top: 6.0), - child: ChecklistPrograssBar(percent: state.percent), + child: ChecklistProgressBar(percent: state.percent), ), ], ), diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_cell.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_cell.dart index 80d2eb426a..e30618dfd4 100644 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_cell.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/cell/select_option_cell/select_option_cell.dart @@ -39,11 +39,12 @@ class GridSingleSelectCell extends GridCellWidget { } @override - State createState() => _SingleSelectCellState(); + GridCellState createState() => _SingleSelectCellState(); } -class _SingleSelectCellState extends State { +class _SingleSelectCellState extends GridCellState { late SelectOptionCellBloc _cellBloc; + late final PopoverController _popover; @override void initState() { @@ -51,6 +52,7 @@ class _SingleSelectCellState extends State { widget.cellControllerBuilder.build() as GridSelectOptionCellController; _cellBloc = getIt(param1: cellController) ..add(const SelectOptionCellEvent.initial()); + _popover = PopoverController(); super.initState(); } @@ -61,10 +63,12 @@ class _SingleSelectCellState extends State { child: BlocBuilder( builder: (context, state) { return SelectOptionWrap( - selectOptions: state.selectedOptions, - cellStyle: widget.cellStyle, - onFocus: (value) => widget.onCellEditing.value = value, - cellControllerBuilder: widget.cellControllerBuilder); + selectOptions: state.selectedOptions, + cellStyle: widget.cellStyle, + onCellEditing: widget.onCellEditing, + popoverController: _popover, + cellControllerBuilder: widget.cellControllerBuilder, + ); }, ), ); @@ -75,6 +79,9 @@ class _SingleSelectCellState extends State { _cellBloc.close(); super.dispose(); } + + @override + void requestBeginFocus() => _popover.show(); } //---------------------------------------------------------------- @@ -95,11 +102,12 @@ class GridMultiSelectCell extends GridCellWidget { } @override - State createState() => _MultiSelectCellState(); + GridCellState createState() => _MultiSelectCellState(); } -class _MultiSelectCellState extends State { +class _MultiSelectCellState extends GridCellState { late SelectOptionCellBloc _cellBloc; + late final PopoverController _popover; @override void initState() { @@ -107,6 +115,7 @@ class _MultiSelectCellState extends State { widget.cellControllerBuilder.build() as GridSelectOptionCellController; _cellBloc = getIt(param1: cellController) ..add(const SelectOptionCellEvent.initial()); + _popover = PopoverController(); super.initState(); } @@ -119,7 +128,8 @@ class _MultiSelectCellState extends State { return SelectOptionWrap( selectOptions: state.selectedOptions, cellStyle: widget.cellStyle, - onFocus: (value) => widget.onCellEditing.value = value, + onCellEditing: widget.onCellEditing, + popoverController: _popover, cellControllerBuilder: widget.cellControllerBuilder, ); }, @@ -132,17 +142,23 @@ class _MultiSelectCellState extends State { _cellBloc.close(); super.dispose(); } + + @override + void requestBeginFocus() => _popover.show(); } class SelectOptionWrap extends StatefulWidget { final List selectOptions; - final void Function(bool)? onFocus; final SelectOptionCellStyle? cellStyle; final GridCellControllerBuilder cellControllerBuilder; + final PopoverController popoverController; + final ValueNotifier onCellEditing; + const SelectOptionWrap({ required this.selectOptions, required this.cellControllerBuilder, - this.onFocus, + required this.onCellEditing, + required this.popoverController, this.cellStyle, Key? key, }) : super(key: key); @@ -152,48 +168,29 @@ class SelectOptionWrap extends StatefulWidget { } class _SelectOptionWrapState extends State { - late PopoverController _popover; - - @override - void initState() { - _popover = PopoverController(); - super.initState(); - } - @override Widget build(BuildContext context) { Widget child = _buildOptions(context); - return Stack( - alignment: AlignmentDirectional.center, - fit: StackFit.expand, - children: [ - _wrapPopover(child), - InkWell(onTap: () => _popover.show()), - ], - ); - } - - Widget _wrapPopover(Widget child) { final constraints = BoxConstraints.loose(Size( SelectOptionCellEditor.editorPanelWidth, 300, )); return AppFlowyPopover( - controller: _popover, + controller: widget.popoverController, constraints: constraints, margin: EdgeInsets.zero, direction: PopoverDirection.bottomWithLeftAligned, popupBuilder: (BuildContext context) { WidgetsBinding.instance.addPostFrameCallback((_) { - widget.onFocus?.call(true); + widget.onCellEditing.value = true; }); return SelectOptionCellEditor( cellController: widget.cellControllerBuilder.build() as GridSelectOptionCellController, ); }, - onClose: () => widget.onFocus?.call(false), + onClose: () => widget.onCellEditing.value = false, child: Padding( padding: GridSize.cellContentInsets, child: child, diff --git a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/row/grid_row.dart b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/row/grid_row.dart index 9318d89599..35f81abc06 100755 --- a/frontend/app_flowy/lib/plugins/grid/presentation/widgets/row/grid_row.dart +++ b/frontend/app_flowy/lib/plugins/grid/presentation/widgets/row/grid_row.dart @@ -225,8 +225,8 @@ class RowContent extends StatelessWidget { return CellContainer( width: cellId.fieldInfo.width.toDouble(), - rowStateNotifier: - Provider.of(context, listen: false), + isPrimary: cellId.fieldInfo.isPrimary, + cellContainerNotifier: CellContainerNotifier(child), accessoryBuilder: (buildContext) { final builder = child.accessoryBuilder; List accessories = [];