Merge pull request #334 from AhmedLSayed9/fix_menu_limits

Fix menu limits when using searchable dropdown with separators
AhmedLSayed9 · Dec 13, 2024 · 58ed330 · 58ed330
2 parents 73f173b + 8fbf8cb
commit 58ed330
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 59 deletions.
diff --git a/packages/dropdown_button2/CHANGELOG.md b/packages/dropdown_button2/CHANGELOG.md
@@ -1,3 +1,7 @@
+## UNRELEASED
+
+- Fix menu limits when using searchable dropdown with separators, closes #214.
+
 ## 3.0.0-beta.20
 
 - Remove an assert from updateSelectedIndex method.

diff --git a/packages/dropdown_button2/lib/src/dropdown_menu.dart b/packages/dropdown_button2/lib/src/dropdown_menu.dart
@@ -209,9 +209,11 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
                                     if (index >= childrenLength) {
                                       return 100;
                                     }
-                                    return separator != null && index.isOdd
-                                        ? separator.height
-                                        : route.itemHeights[index];
+                                    return separator == null
+                                        ? route.items[index].height
+                                        : index.isOdd
+                                            ? separator.height
+                                            : route.items[index ~/ 2].height;
                                   },
                             childrenDelegate: separator == null
                                 ? SliverChildBuilderDelegate(

diff --git a/packages/dropdown_button2/lib/src/dropdown_route.dart b/packages/dropdown_button2/lib/src/dropdown_route.dart
@@ -19,11 +19,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
     required this.menuItemStyle,
     required this.searchData,
     this.dropdownSeparator,
-  })  : itemHeights = addSeparatorsHeights(
-          itemHeights: items.map((item) => item.height).toList(),
-          separatorHeight: dropdownSeparator?.height,
-        ),
-        barrierColor = barrierCoversButton ? barrierColor : null,
+  })  : barrierColor = barrierCoversButton ? barrierColor : null,
         _altBarrierColor = barrierColor;
 
   final List<DropdownItem<T>> items;
@@ -40,7 +36,6 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
   final DropdownSearchData<T>? searchData;
   final DropdownSeparator<T>? dropdownSeparator;
 
-  final List<double> itemHeights;
   ScrollController? scrollController;
 
   @override
@@ -114,17 +109,32 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
     }
   }
 
+  double _itemHeightWithSeparator(int itemIndex) {
+    final itemHeight = items[itemIndex].height;
+    final separatorHeight = dropdownSeparator?.height ?? 0;
+    return itemIndex != 0 ? itemHeight + separatorHeight : itemHeight;
+  }
+
+  double _calculateHeightUntilIndex(
+    int index, {
+    bool Function(DropdownItem<T> item)? itemPredicate,
+  }) {
+    var itemsHeight = 0.0;
+    for (int i = 0; i < index; i++) {
+      if (itemPredicate == null || itemPredicate(items[i])) {
+        itemsHeight += _itemHeightWithSeparator(i);
+      }
+    }
+    return itemsHeight;
+  }
+
   double getItemOffset(int index) {
     final double paddingTop = dropdownStyle.padding != null
         ? dropdownStyle.padding!.resolve(null).top
         : kMaterialListPadding.top;
     double offset = paddingTop;
 
     if (items.isNotEmpty && index > 0) {
-      assert(
-        items.length + (dropdownSeparator != null ? items.length - 1 : 0) ==
-            itemHeights.length,
-      );
       if (searchData?.searchController?.text case final searchText?) {
         final searchMatchFn =
             searchData?.searchMatchFn ?? _defaultSearchMatchFn();
@@ -133,24 +143,19 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
           offset += _getSearchItemsHeight(index, searchText);
         }
       } else {
-        for (int i = 0; i < index; i++) {
-          offset += itemHeights[i];
-        }
+        offset += _calculateHeightUntilIndex(index);
       }
     }
 
     return offset;
   }
 
   double _getSearchItemsHeight(int index, String searchText) {
-    var itemsHeight = 0.0;
     final searchMatchFn = searchData?.searchMatchFn ?? _defaultSearchMatchFn();
-    for (int i = 0; i < index; i++) {
-      if (searchMatchFn(items[i], searchText)) {
-        itemsHeight += itemHeights[i];
-      }
-    }
-    return itemsHeight;
+    return _calculateHeightUntilIndex(
+      index,
+      itemPredicate: (item) => searchMatchFn(item, searchText),
+    );
   }
 
   // Returns the vertical extent of the menu and the initial scrollOffset
@@ -177,7 +182,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
       final searchText = searchData?.searchController?.text;
       actualMenuHeight += searchText != null
           ? _getSearchItemsHeight(items.length, searchText)
-          : itemHeights.reduce((double total, double height) => total + height);
+          : _calculateHeightUntilIndex(items.length);
     }
 
     // Use actualMenuHeight if it's less than maxHeight.
@@ -215,13 +220,11 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
       final double actualMenuNetHeight = actualMenuHeight - innerWidgetHeight;
       // The offset should be zero if the selected item is in view at the beginning
       // of the menu. Otherwise, the scroll offset should center the item if possible.
-      final actualIndex = dropdownSeparator?.height != null ? index * 2 : index;
-      final double selectedItemOffset = getItemOffset(actualIndex);
+      final double selectedItemOffset = getItemOffset(index);
       scrollOffset = math.max(
-          0.0,
-          selectedItemOffset -
-              (menuNetHeight / 2) +
-              (itemHeights[actualIndex] / 2));
+        0.0,
+        selectedItemOffset - (menuNetHeight / 2) + (items[index].height / 2),
+      );
       // If the selected item's scroll offset is greater than the maximum scroll offset,
       // set it instead to the maximum allowed scroll offset.
       final double maxScrollOffset = actualMenuNetHeight - menuNetHeight;

diff --git a/packages/dropdown_button2/lib/src/utils.dart b/packages/dropdown_button2/lib/src/utils.dart
@@ -1,34 +1,5 @@
 part of 'dropdown_button2.dart';
 
-/// Adds separators to a list of heights.
-///
-/// The [itemHeights] property is the list of heights of the items.
-///
-/// The [separatorHeight] property is the height of the separator.
-///
-/// Returns a new list of heights with separators added.
-List<double> addSeparatorsHeights({
-  required List<double> itemHeights,
-  required double? separatorHeight,
-}) {
-  final List<double> heights = [];
-
-  bool addSeparator = false;
-  if (separatorHeight != null) {
-    for (final item in itemHeights) {
-      if (addSeparator) {
-        heights.add(separatorHeight);
-      }
-      heights.add(item);
-      addSeparator = true;
-    }
-  } else {
-    heights.addAll(itemHeights);
-  }
-
-  return heights;
-}
-
 void _uniqueValueAssert<T>(
   List<DropdownItem<T>>? items,
   ValueListenable<T?>? valueListenable,