From 64d0c356ae3d2552031a3de6dbbdc591a16159cb Mon Sep 17 00:00:00 2001 From: Bruno D'Luka Date: Wed, 5 Jul 2023 18:53:36 -0300 Subject: [PATCH 1/4] TreeView's built-in checkbox should not receive focus --- lib/src/controls/navigation/tree_view.dart | 16 ++++++++++------ lib/src/styles/theme.dart | 1 - 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/src/controls/navigation/tree_view.dart b/lib/src/controls/navigation/tree_view.dart index 1cd077e61..57092ce27 100644 --- a/lib/src/controls/navigation/tree_view.dart +++ b/lib/src/controls/navigation/tree_view.dart @@ -953,12 +953,16 @@ class _TreeViewItem extends StatelessWidget { start: 8.0, end: narrowSpacing ? 0.0 : _whiteSpace, ), - child: Checkbox( - checked: item.selected, - onChanged: (value) { - onSelect(); - onInvoked(TreeViewItemInvokeReason.selectionToggle); - }, + child: ExcludeFocus( + child: Checkbox( + checked: item.selected, + onChanged: (value) { + onSelect(); + onInvoked( + TreeViewItemInvokeReason.selectionToggle, + ); + }, + ), ), ), diff --git a/lib/src/styles/theme.dart b/lib/src/styles/theme.dart index 4e827dc7a..98cae37f8 100644 --- a/lib/src/styles/theme.dart +++ b/lib/src/styles/theme.dart @@ -301,7 +301,6 @@ class FluentThemeData with Diagnosticable { typography = Typography.fromBrightness(brightness: brightness) .merge(typography) .apply(fontFamily: fontFamily); - ; focusTheme ??= const FocusThemeData(); buttonTheme ??= const ButtonThemeData(); checkboxTheme ??= const CheckboxThemeData(); From 71280998c56c0b93375944d874e77d4a703478dc Mon Sep 17 00:00:00 2001 From: Bruno D'Luka Date: Wed, 5 Jul 2023 19:00:45 -0300 Subject: [PATCH 2/4] Checkbox should be selected when actioned with keyboard --- lib/src/controls/navigation/tree_view.dart | 27 +++++++++++++--------- lib/src/controls/utils/hover_button.dart | 8 +++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/src/controls/navigation/tree_view.dart b/lib/src/controls/navigation/tree_view.dart index 57092ce27..a512ed98f 100644 --- a/lib/src/controls/navigation/tree_view.dart +++ b/lib/src/controls/navigation/tree_view.dart @@ -843,12 +843,20 @@ class _TreeViewItem extends StatelessWidget { final Widget loadingWidgetFallback; final bool narrowSpacing; + void _onCheckboxInvoked() { + onSelect(); + onInvoked( + TreeViewItemInvokeReason.selectionToggle, + ); + } + @override Widget build(BuildContext context) { if (!item._visible) return const SizedBox.shrink(); final theme = FluentTheme.of(context); final selected = item.selected ?? false; final direction = Directionality.of(context); + return GestureDetector( onSecondaryTapDown: onSecondaryTap, child: HoverButton( @@ -893,6 +901,7 @@ class _TreeViewItem extends StatelessWidget { : () { onInvoked(TreeViewItemInvokeReason.pressed); }, + onFocusTap: _onCheckboxInvoked, autofocus: item.autofocus, focusNode: item.focusNode, semanticLabel: item.semanticLabel, @@ -901,11 +910,12 @@ class _TreeViewItem extends StatelessWidget { horizontal: 4.0, ), builder: (context, states) { - final itemForegroundColor = states.isDisabled - ? theme.resources.textFillColorDisabled - : states.isPressing - ? theme.resources.textFillColorSecondary - : theme.resources.textFillColorPrimary; + final itemForegroundColor = ButtonState.forStates( + states, + disabled: theme.resources.textFillColorDisabled, + pressed: theme.resources.textFillColorSecondary, + none: theme.resources.textFillColorPrimary, + ); return FocusBorder( focused: states.isFocused, @@ -956,12 +966,7 @@ class _TreeViewItem extends StatelessWidget { child: ExcludeFocus( child: Checkbox( checked: item.selected, - onChanged: (value) { - onSelect(); - onInvoked( - TreeViewItemInvokeReason.selectionToggle, - ); - }, + onChanged: (value) => _onCheckboxInvoked(), ), ), ), diff --git a/lib/src/controls/utils/hover_button.dart b/lib/src/controls/utils/hover_button.dart index 58dbb050d..bcd5f62ad 100644 --- a/lib/src/controls/utils/hover_button.dart +++ b/lib/src/controls/utils/hover_button.dart @@ -27,6 +27,7 @@ class HoverButton extends StatefulWidget { this.onHorizontalDragStart, this.onHorizontalDragUpdate, this.onHorizontalDragEnd, + this.onFocusTap, this.onFocusChange, this.autofocus = false, this.actionsEnabled = true, @@ -59,6 +60,12 @@ class HoverButton extends StatefulWidget { final GestureDragUpdateCallback? onHorizontalDragUpdate; final GestureDragEndCallback? onHorizontalDragEnd; + /// When the button is focused and is actioned, with either the enter or space + /// keys + /// + /// [focusEnabled] must not be `false` for this to work + final VoidCallback? onFocusTap; + final ButtonStateWidgetBuilder builder; /// {@macro flutter.widgets.Focus.focusNode} @@ -144,6 +151,7 @@ class _HoverButtonState extends State { Future handleActionTap() async { if (!enabled) return; setState(() => _pressing = true); + widget.onFocusTap?.call(); widget.onPressed?.call(); await Future.delayed(const Duration(milliseconds: 100)); if (mounted) setState(() => _pressing = false); From 4e0a5bd86c137e13e72b01b768eb51477e096334 Mon Sep 17 00:00:00 2001 From: Bruno D'Luka Date: Wed, 5 Jul 2023 19:44:42 -0300 Subject: [PATCH 3/4] TreeViewItems, on single selection mode, should turn selected when focused --- lib/src/controls/navigation/tree_view.dart | 358 +++++++++++---------- 1 file changed, 183 insertions(+), 175 deletions(-) diff --git a/lib/src/controls/navigation/tree_view.dart b/lib/src/controls/navigation/tree_view.dart index a512ed98f..c43fe70d0 100644 --- a/lib/src/controls/navigation/tree_view.dart +++ b/lib/src/controls/navigation/tree_view.dart @@ -690,106 +690,109 @@ class TreeViewState extends State with AutomaticKeepAliveClientMixin { assert(debugCheckHasDirectionality(context)); assert(debugCheckHasFluentTheme(context)); - return FocusTraversalGroup( - policy: WidgetOrderTraversalPolicy(), - child: ConstrainedBox( - constraints: const BoxConstraints(minHeight: 28.0), - child: ListView.builder( - // If shrinkWrap is true, then we default to not using the primary - // scroll controller (should not normally need any controller in - // this case). - primary: widget.scrollPrimary ?? (widget.shrinkWrap ? false : null), - controller: widget.scrollController, - shrinkWrap: widget.shrinkWrap, - cacheExtent: widget.cacheExtent, - itemExtent: widget.itemExtent, - addRepaintBoundaries: widget.addRepaintBoundaries, - prototypeItem: widget.usePrototypeItem && _items.isNotEmpty - ? _TreeViewItem( - item: _items.first, - selectionMode: widget.selectionMode, - narrowSpacing: widget.narrowSpacing, - onInvoked: (_) {}, - onSelect: () {}, - onSecondaryTap: (details) {}, - onExpandToggle: () {}, - loadingWidgetFallback: widget.loadingWidget, - ) - : null, - itemCount: _items.length, - itemBuilder: (context, index) { - final item = _items[index]; - - return _TreeViewItem( - key: item.key ?? ValueKey(item), - item: item, - selectionMode: widget.selectionMode, - narrowSpacing: widget.narrowSpacing, - onSecondaryTap: (details) { - widget.onSecondaryTap?.call(item, details); - }, - onSelect: () async { - final onSelectionChanged = widget.onSelectionChanged; - switch (widget.selectionMode) { - case TreeViewSelectionMode.single: - setState(() { - _items.executeForAll((item) { - item.selected = false; + return FocusScope( + child: FocusTraversalGroup( + policy: WidgetOrderTraversalPolicy(), + child: ConstrainedBox( + constraints: const BoxConstraints(minHeight: 28.0), + child: ListView.builder( + // If shrinkWrap is true, then we default to not using the primary + // scroll controller (should not normally need any controller in + // this case). + primary: widget.scrollPrimary ?? (widget.shrinkWrap ? false : null), + controller: widget.scrollController, + shrinkWrap: widget.shrinkWrap, + cacheExtent: widget.cacheExtent, + itemExtent: widget.itemExtent, + addRepaintBoundaries: widget.addRepaintBoundaries, + prototypeItem: widget.usePrototypeItem && _items.isNotEmpty + ? _TreeViewItem( + item: _items.first, + selectionMode: widget.selectionMode, + narrowSpacing: widget.narrowSpacing, + onInvoked: (_) {}, + onSelect: () {}, + onSecondaryTap: (details) {}, + onExpandToggle: () {}, + loadingWidgetFallback: widget.loadingWidget, + ) + : null, + itemCount: _items.length, + itemBuilder: (context, index) { + final item = _items[index]; + + return _TreeViewItem( + key: item.key ?? ValueKey(item), + item: item, + selectionMode: widget.selectionMode, + narrowSpacing: widget.narrowSpacing, + onSecondaryTap: (details) { + widget.onSecondaryTap?.call(item, details); + }, + onSelect: () async { + final onSelectionChanged = widget.onSelectionChanged; + switch (widget.selectionMode) { + case TreeViewSelectionMode.single: + setState(() { + _items.executeForAll((item) { + item.selected = false; + }); + item.selected = true; }); - item.selected = true; - }); - if (onSelectionChanged != null) { - await onSelectionChanged([item]); + if (onSelectionChanged != null) { + await onSelectionChanged([item]); + } + break; + case TreeViewSelectionMode.multiple: + setState(() { + final newSelectionState = + item.selected == null || item.selected == false; + item.setSelectionStateForMultiSelectionMode( + newSelectionState, + widget.deselectParentWhenChildrenDeselected); + }); + if (onSelectionChanged != null) { + final selectedItems = widget.items.selectedItems( + widget.includePartiallySelectedItems); + await onSelectionChanged(selectedItems); + } + break; + default: + break; + } + }, + onExpandToggle: () async { + await _invokeItem( + item, TreeViewItemInvokeReason.expandToggle); + + if (item.collapsable) { + if (item.lazy) { + // Triggers a loading indicator. + setState(() => item.loading = true); } - break; - case TreeViewSelectionMode.multiple: + + await Future.wait([ + if (widget.onItemExpandToggle != null) + widget.onItemExpandToggle!(item, !item.expanded), + if (item.onExpandToggle != null) + item.onExpandToggle!(item, !item.expanded), + ]); + + // Remove the loading indicator. + // Toggle the expand icon. setState(() { - final newSelectionState = - item.selected == null || item.selected == false; - item.setSelectionStateForMultiSelectionMode( - newSelectionState, - widget.deselectParentWhenChildrenDeselected); + item + ..loading = false + ..expanded = !item.expanded; + _buildItems(); }); - if (onSelectionChanged != null) { - final selectedItems = widget.items - .selectedItems(widget.includePartiallySelectedItems); - await onSelectionChanged(selectedItems); - } - break; - default: - break; - } - }, - onExpandToggle: () async { - await _invokeItem(item, TreeViewItemInvokeReason.expandToggle); - - if (item.collapsable) { - if (item.lazy) { - // Triggers a loading indicator. - setState(() => item.loading = true); } - - await Future.wait([ - if (widget.onItemExpandToggle != null) - widget.onItemExpandToggle!(item, !item.expanded), - if (item.onExpandToggle != null) - item.onExpandToggle!(item, !item.expanded), - ]); - - // Remove the loading indicator. - // Toggle the expand icon. - setState(() { - item - ..loading = false - ..expanded = !item.expanded; - _buildItems(); - }); - } - }, - onInvoked: (reason) => _invokeItem(item, reason), - loadingWidgetFallback: widget.loadingWidget, - ); - }, + }, + onInvoked: (reason) => _invokeItem(item, reason), + loadingWidgetFallback: widget.loadingWidget, + ); + }, + ), ), ), ); @@ -845,14 +848,13 @@ class _TreeViewItem extends StatelessWidget { void _onCheckboxInvoked() { onSelect(); - onInvoked( - TreeViewItemInvokeReason.selectionToggle, - ); + onInvoked(TreeViewItemInvokeReason.selectionToggle); } @override Widget build(BuildContext context) { if (!item._visible) return const SizedBox.shrink(); + final theme = FluentTheme.of(context); final selected = item.selected ?? false; final direction = Directionality.of(context); @@ -897,18 +899,26 @@ class _TreeViewItem extends StatelessWidget { ? () { onSelect(); onInvoked(TreeViewItemInvokeReason.pressed); + FocusScope.of(context).unfocus( + disposition: UnfocusDisposition.previouslyFocusedChild, + ); } : () { onInvoked(TreeViewItemInvokeReason.pressed); + FocusScope.of(context).unfocus( + disposition: UnfocusDisposition.previouslyFocusedChild, + ); }, onFocusTap: _onCheckboxInvoked, + onFocusChange: selectionMode == TreeViewSelectionMode.single + ? (focused) { + if (focused) onSelect(); + } + : null, autofocus: item.autofocus, focusNode: item.focusNode, semanticLabel: item.semanticLabel, - margin: const EdgeInsets.symmetric( - vertical: 2.0, - horizontal: 4.0, - ), + margin: const EdgeInsets.symmetric(vertical: 2.0, horizontal: 4.0), builder: (context, states) { final itemForegroundColor = ButtonState.forStates( states, @@ -954,91 +964,89 @@ class _TreeViewItem extends StatelessWidget { ), borderRadius: BorderRadius.circular(6.0), ), - child: Row( - children: [ - // Checkbox for multi selection mode - if (selectionMode == TreeViewSelectionMode.multiple) - Padding( - padding: EdgeInsetsDirectional.only( - start: 8.0, - end: narrowSpacing ? 0.0 : _whiteSpace, - ), - child: ExcludeFocus( - child: Checkbox( - checked: item.selected, - onChanged: (value) => _onCheckboxInvoked(), - ), + child: Row(children: [ + // Checkbox for multi selection mode + if (selectionMode == TreeViewSelectionMode.multiple) + Padding( + padding: EdgeInsetsDirectional.only( + start: 8.0, + end: narrowSpacing ? 0.0 : _whiteSpace, + ), + child: ExcludeFocus( + child: Checkbox( + checked: item.selected, + onChanged: (value) => _onCheckboxInvoked(), ), ), + ), - // Expand icon with hitbox - if (item.isExpandable) - if (item.loading) - item.loadingWidget ?? loadingWidgetFallback - else - GestureDetector( - behavior: HitTestBehavior.deferToChild, - onTap: onExpandToggle, - child: Container( - // The hitbox for the chevron is three times the - // chevron's (max) width. - width: 24, - // The hitbox fills the available height. - height: double.infinity, - decoration: BoxDecoration( - color: Colors.transparent, - borderRadius: BorderRadius.circular(5.0), - ), - child: Icon( - item.expanded - ? FluentIcons.chevron_down - : direction == TextDirection.ltr - ? FluentIcons.chevron_right - : FluentIcons.chevron_left, - size: 8.0, - color: itemForegroundColor, - ), - ), - ) + // Expand icon with hitbox + if (item.isExpandable) + if (item.loading) + item.loadingWidget ?? loadingWidgetFallback else - // Add padding to make all items of the same depth level - // have the same indentation, regardless whether or not - // they are expandable. - const Padding( - padding: EdgeInsetsDirectional.only(start: 24.0), - ), - - // Leading icon - if (item.leading != null) - Container( - margin: EdgeInsetsDirectional.only( - start: narrowSpacing ? 0 : _whiteSpace, - end: narrowSpacing ? _whiteSpace : 2 * _whiteSpace, - ), - width: 20.0, - child: IconTheme.merge( - data: IconThemeData( - size: 20.0, + GestureDetector( + behavior: HitTestBehavior.deferToChild, + onTap: onExpandToggle, + child: Container( + // The hitbox for the chevron is three times the + // chevron's (max) width. + width: 24, + // The hitbox fills the available height. + height: double.infinity, + decoration: BoxDecoration( + color: Colors.transparent, + borderRadius: BorderRadius.circular(5.0), + ), + child: Icon( + item.expanded + ? FluentIcons.chevron_down + : direction == TextDirection.ltr + ? FluentIcons.chevron_right + : FluentIcons.chevron_left, + size: 8.0, color: itemForegroundColor, ), - child: item.leading!, ), ) - else if (!narrowSpacing) - const SizedBox(width: _whiteSpace), - - // Item content - Expanded( - child: DefaultTextStyle.merge( - style: TextStyle( - fontSize: 12.0, + else + // Add padding to make all items of the same depth level + // have the same indentation, regardless whether or not + // they are expandable. + const Padding( + padding: EdgeInsetsDirectional.only(start: 24.0), + ), + + // Leading icon + if (item.leading != null) + Container( + margin: EdgeInsetsDirectional.only( + start: narrowSpacing ? 0 : _whiteSpace, + end: narrowSpacing ? _whiteSpace : 2 * _whiteSpace, + ), + width: 20.0, + child: IconTheme.merge( + data: IconThemeData( + size: 20.0, color: itemForegroundColor, ), - child: item.content, + child: item.leading!, ), + ) + else if (!narrowSpacing) + const SizedBox(width: _whiteSpace), + + // Item content + Expanded( + child: DefaultTextStyle.merge( + style: TextStyle( + fontSize: 12.0, + color: itemForegroundColor, + ), + child: item.content, ), - ], - ), + ), + ]), ), if (selected && selectionMode == TreeViewSelectionMode.single) PositionedDirectional( From bca8c4408504bb4a716061fc3899f21a2f05adea Mon Sep 17 00:00:00 2001 From: Bruno D'Luka Date: Wed, 5 Jul 2023 20:06:46 -0300 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d56aec530..33116647e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - Add `PaneItemExpander.initiallyExpanded` ([#864](https://github.com/bdlukaa/fluent_ui/issues/864)) - Add `NumberFormBox` ([#862](https://github.com/bdlukaa/fluent_ui/issues/862)) - `PaneItem.onTap` from `PaneItemExpander.items`, when displayed in popup, are now correctly invoked ([#859](https://github.com/bdlukaa/fluent_ui/issues/859)) +- `TreeViewItem`, if selection mode is `single`, gets selected when focused with the keyboard ([#835](https://github.com/bdlukaa/fluent_ui/issues/835)) +- In multiple selection mode, `TreeView`'s built-in checkbox now doesn't receive focus. It can now be focused by invoking it with the keyboard ([#877](https://github.com/bdlukaa/fluent_ui/pull/877)) ## 4.6.2