From 14c6193367c847faf9050ed59bd03d3948304563 Mon Sep 17 00:00:00 2001 From: ssbg04 Date: Tue, 15 Sep 2026 21:30:11 +0800 Subject: [PATCH] fix(search): fix history overlay interaction and premature dismissal on Windows --- .gitignore | 1 + .../lib/ui/shared/inputs/app_search_bar.dart | 325 +++++++++--------- frontend/test/app_search_bar_test.dart | 85 +++++ 3 files changed, 258 insertions(+), 153 deletions(-) diff --git a/.gitignore b/.gitignore index 5d8a3a2..c98d5f7 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ requirements TIS* !*.iss screenshots/ +issues/ # Environment secrets backend/.env diff --git a/frontend/lib/ui/shared/inputs/app_search_bar.dart b/frontend/lib/ui/shared/inputs/app_search_bar.dart index 7c1444e..ec54e74 100644 --- a/frontend/lib/ui/shared/inputs/app_search_bar.dart +++ b/frontend/lib/ui/shared/inputs/app_search_bar.dart @@ -44,6 +44,7 @@ class _AppSearchBarState extends ConsumerState { final LayerLink _layerLink = LayerLink(); final Object _tapRegionGroupId = Object(); bool _isOverlayOpen = false; + bool _isPointerOverOverlay = false; @override void didUpdateWidget(AppSearchBar oldWidget) { @@ -115,8 +116,8 @@ class _AppSearchBarState extends ConsumerState { if (_focusNode.hasFocus && widget.enableHistory) { _showOverlay(); } else { - Future.delayed(const Duration(milliseconds: 150), () { - if (mounted && !_focusNode.hasFocus) { + Future.delayed(const Duration(milliseconds: 200), () { + if (mounted && !_focusNode.hasFocus && !_isPointerOverOverlay) { _removeOverlay(); if (widget.collapsible && _controller.text.trim().isEmpty) { @@ -144,7 +145,7 @@ class _AppSearchBarState extends ConsumerState { WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted || !_isOverlayOpen || - !_focusNode.hasFocus || + (!_focusNode.hasFocus && !_isPointerOverOverlay) || _overlayEntry != null) { return; } @@ -169,7 +170,12 @@ class _AppSearchBarState extends ConsumerState { offset: const Offset(0, 48), child: TapRegion( groupId: _tapRegionGroupId, - child: _buildHistoryOverlay(), + behavior: HitTestBehavior.opaque, + child: MouseRegion( + onEnter: (_) => _isPointerOverOverlay = true, + onExit: (_) => _isPointerOverOverlay = false, + child: _buildHistoryOverlay(), + ), ), ), ); @@ -182,6 +188,7 @@ class _AppSearchBarState extends ConsumerState { void _removeOverlay() { _isOverlayOpen = false; + _isPointerOverOverlay = false; if (_overlayEntry != null) { if (_overlayEntry!.mounted) { _overlayEntry!.remove(); @@ -208,166 +215,173 @@ class _AppSearchBarState extends ConsumerState { if (displayList.isEmpty) return const SizedBox.shrink(); - return Material( - elevation: 4, - color: isDark ? AppColors.darkSurfaceCard : Colors.white, - borderRadius: BorderRadius.circular(12), - child: Container( - constraints: BoxConstraints(maxHeight: halfScreenHeight), - decoration: BoxDecoration( - borderRadius: BorderRadius.circular(12), - border: Border.all( - color: isDark ? AppColors.darkBorder : Colors.grey.shade200, - ), - ), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Padding( - padding: const EdgeInsets.fromLTRB(16, 10, 12, 6), - child: Row( - mainAxisAlignment: MainAxisAlignment.spaceBetween, - children: [ - Text( - 'Recent Searches', - style: TextStyle( - fontSize: 11, - fontWeight: FontWeight.w600, - color: isDark - ? AppColors.darkTextMuted - : Colors.grey.shade500, - letterSpacing: 0.3, - ), - ), - InkWell( - borderRadius: BorderRadius.circular(4), - canRequestFocus: false, - onTap: () { - ref - .read(searchHistoryProvider.notifier) - .clearHistory(); - _removeOverlay(); - if (!_focusNode.hasFocus) { - _focusNode.requestFocus(); - } - }, - child: Padding( - padding: const EdgeInsets.symmetric( - horizontal: 6, - vertical: 2, - ), - child: Text( - 'Clear all', - style: TextStyle( - fontSize: 11, - fontWeight: FontWeight.w500, - color: isDark - ? AppColors.darkTextSecondary - : Colors.grey.shade600, - ), - ), - ), - ), - ], - ), - ), - Divider( - height: 1, - thickness: 1, + return Focus( + canRequestFocus: false, + skipTraversal: true, + descendantsAreFocusable: false, + child: Material( + elevation: 4, + color: isDark ? AppColors.darkSurfaceCard : Colors.white, + borderRadius: BorderRadius.circular(12), + child: Container( + constraints: BoxConstraints(maxHeight: halfScreenHeight), + decoration: BoxDecoration( + borderRadius: BorderRadius.circular(12), + border: Border.all( color: isDark ? AppColors.darkBorder : Colors.grey.shade200, ), - Flexible( - child: ListView.builder( - padding: const EdgeInsets.symmetric(vertical: 4), - shrinkWrap: true, - itemCount: displayList.length, - itemBuilder: (context, index) { - final term = displayList[index]; - return Row( - children: [ - Expanded( - child: InkWell( - borderRadius: BorderRadius.circular(8), - canRequestFocus: false, - onTap: () { - final selected = term; - _controller.text = selected; - _controller.selection = TextSelection.collapsed( - offset: selected.length, - ); - setState(() { - _hasText = selected.isNotEmpty; - }); - ref - .read(searchHistoryProvider.notifier) - .addSearch(selected); - widget.onChanged?.call(selected); - widget.onSubmitted?.call(selected); - _removeOverlay(); - _focusNode.unfocus(); - }, - child: Padding( - padding: const EdgeInsets.symmetric( - horizontal: 16, - vertical: 9, - ), - child: Row( - children: [ - Icon( - Icons.history, - size: 18, - color: isDark - ? AppColors.darkTextSecondary - : Colors.grey.shade400, - ), - const SizedBox(width: 12), - Expanded( - child: Text( - term, - style: TextStyle( - fontSize: 14, - color: isDark - ? AppColors.darkTextPrimary - : Colors.black87, - ), - ), - ), - ], - ), + ), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Padding( + padding: const EdgeInsets.fromLTRB(16, 10, 12, 6), + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Text( + 'Recent Searches', + style: TextStyle( + fontSize: 11, + fontWeight: FontWeight.w600, + color: isDark + ? AppColors.darkTextMuted + : Colors.grey.shade500, + letterSpacing: 0.3, + ), + ), + InkWell( + borderRadius: BorderRadius.circular(4), + canRequestFocus: false, + onTap: () { + ref + .read(searchHistoryProvider.notifier) + .clearHistory(); + _removeOverlay(); + if (!_focusNode.hasFocus) { + _focusNode.requestFocus(); + } + }, + child: Padding( + padding: const EdgeInsets.symmetric( + horizontal: 6, + vertical: 2, + ), + child: Text( + 'Clear all', + style: TextStyle( + fontSize: 11, + fontWeight: FontWeight.w500, + color: isDark + ? AppColors.darkTextSecondary + : Colors.grey.shade600, ), ), ), - InkResponse( - radius: 16, - canRequestFocus: false, - onTap: () { - ref - .read(searchHistoryProvider.notifier) - .removeSearch(term); - if (!_focusNode.hasFocus) { - _focusNode.requestFocus(); - } - }, - child: Padding( - padding: const EdgeInsets.symmetric( - horizontal: 12, - vertical: 9, + ), + ], + ), + ), + Divider( + height: 1, + thickness: 1, + color: isDark ? AppColors.darkBorder : Colors.grey.shade200, + ), + Flexible( + child: ListView.builder( + padding: const EdgeInsets.symmetric(vertical: 4), + shrinkWrap: true, + itemCount: displayList.length, + itemBuilder: (context, index) { + final term = displayList[index]; + return Row( + children: [ + Expanded( + child: InkWell( + borderRadius: BorderRadius.circular(8), + canRequestFocus: false, + onTap: () { + final selected = term; + _controller.text = selected; + _controller.selection = TextSelection.collapsed( + offset: selected.length, + ); + setState(() { + _hasText = selected.isNotEmpty; + }); + ref + .read(searchHistoryProvider.notifier) + .addSearch(selected); + widget.onChanged?.call(selected); + widget.onSubmitted?.call(selected); + _removeOverlay(); + _focusNode.unfocus(); + }, + child: Padding( + padding: const EdgeInsets.symmetric( + horizontal: 16, + vertical: 9, + ), + child: Row( + children: [ + Icon( + Icons.history, + size: 18, + color: isDark + ? AppColors.darkTextSecondary + : Colors.grey.shade400, + ), + const SizedBox(width: 12), + Expanded( + child: Text( + term, + style: TextStyle( + fontSize: 14, + color: isDark + ? AppColors.darkTextPrimary + : Colors.black87, + ), + ), + ), + ], + ), + ), ), - child: Icon( + ), + IconButton( + icon: Icon( Icons.close_rounded, size: 16, color: isDark ? AppColors.darkTextMuted : Colors.grey.shade400, ), + splashRadius: 16, + padding: const EdgeInsets.symmetric( + horizontal: 10, + vertical: 8, + ), + constraints: const BoxConstraints(), + tooltip: 'Remove', + onPressed: () { + ref + .read(searchHistoryProvider.notifier) + .removeSearch(term); + if (!_focusNode.hasFocus) { + _focusNode.requestFocus(); + } + if (_overlayEntry != null) { + _overlayEntry!.markNeedsBuild(); + } + }, ), - ), - ], - ); - }, + ], + ); + }, + ), ), - ), - ], + ], + ), ), ), ); @@ -408,6 +422,8 @@ class _AppSearchBarState extends ConsumerState { !_isOverlayOpen && widget.enableHistory) { _showOverlay(); + } else if (_isOverlayOpen) { + _overlayEntry?.markNeedsBuild(); } }); @@ -443,11 +459,14 @@ class _AppSearchBarState extends ConsumerState { }, child: TapRegion( groupId: _tapRegionGroupId, + behavior: HitTestBehavior.opaque, onTapOutside: (event) { - if (_focusNode.hasFocus) { - _focusNode.unfocus(); + if (!_isPointerOverOverlay) { + if (_focusNode.hasFocus) { + _focusNode.unfocus(); + } + _removeOverlay(); } - _removeOverlay(); }, child: CompositedTransformTarget( link: _layerLink, diff --git a/frontend/test/app_search_bar_test.dart b/frontend/test/app_search_bar_test.dart index e6e5f87..df04b11 100644 --- a/frontend/test/app_search_bar_test.dart +++ b/frontend/test/app_search_bar_test.dart @@ -161,4 +161,89 @@ void main() { // Overlay is dismissed expect(find.text('Recent Searches'), findsNothing); }); + + testWidgets('AppSearchBar inside showDialog supports clear all and item deletion', (tester) async { + final container = ProviderContainer(); + addTearDown(container.dispose); + + container.read(searchHistoryProvider); + await tester.pump(const Duration(milliseconds: 100)); + + final searchController = TextEditingController(); + final searchFocusNode = FocusNode(); + + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: MaterialApp( + home: Scaffold( + body: Builder( + builder: (context) { + return ElevatedButton( + onPressed: () { + showDialog( + context: context, + barrierColor: Colors.black54, + builder: (ctx) { + return Align( + alignment: Alignment.topCenter, + child: Padding( + padding: const EdgeInsets.fromLTRB(16, 80, 16, 0), + child: Material( + child: AppSearchBar( + controller: searchController, + focusNode: searchFocusNode, + collapsible: false, + hint: 'Search students by LRN or Name...', + maxWidth: 600, + ), + ), + ), + ); + }, + ); + searchFocusNode.requestFocus(); + }, + child: const Text('Open Search Dialog'), + ); + }, + ), + ), + ), + ), + ); + + await tester.pump(); + await tester.tap(find.text('Open Search Dialog')); + await tester.pumpAndSettle(); + + expect(find.text('Recent Searches'), findsOneWidget); + expect(find.text('test item 1'), findsOneWidget); + expect(find.text('test item 2'), findsOneWidget); + + // Find the close buttons in the history list + final closeIcons = find.byIcon(Icons.close_rounded); + expect(closeIcons, findsWidgets); + + // Tap the first history item's delete button + await tester.tap(closeIcons.first); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 200)); + + // Item 1 should be removed, item 2 should still be there, and overlay should remain open! + expect(container.read(searchHistoryProvider), isNot(contains('test item 1'))); + expect(container.read(searchHistoryProvider), contains('test item 2')); + expect(find.text('Recent Searches'), findsOneWidget); + expect(find.text('test item 2'), findsOneWidget); + + // Tap Clear all + await tester.tap(find.text('Clear all')); + await tester.pumpAndSettle(); + + // All items cleared and overlay closed + expect(container.read(searchHistoryProvider), isEmpty); + expect(find.text('Recent Searches'), findsNothing); + }); } + +