New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

propably fix #53 #56

Open

jayjah wants to merge 2 commits into Limbou:main from jayjah:main

jayjah commented Sep 20, 2023

Should be a fix to Issue#53
It just makes sure, that clamp get's called on a valid range otherwise it might just throw an error like the following:

Invalid argument(s): 0
       at int.clamp(dart:core)
       at _ExpandablePageViewState.initState(expandable_page_view.dart:210)
       at StatefulElement._firstBuild(framework.dart:5430)
       at ComponentElement.mount(framework.dart:5273)
       at Element.inflateWidget(framework.dart:4182)
       at MultiChildRenderObjectElement.inflateWidget(framework.dart:6569)
       at MultiChildRenderObjectElement.mount(framework.dart:6581)
       at Element.inflateWidget(framework.dart:4182)
       at Element.updateChild(framework.dart:3707)
       at SingleChildRenderObjectElement.mount(framework.dart:6434)
       at Element.inflateWidget(framework.dart:4182)
...

jayjah added 2 commits

September 20, 2023 14:42


          propably fix Limbou#53

ee8dff0


          fix:RangeError (length): Invalid value: Valid value range is empty: a…

7c9311b

…t _ExpandablePageViewState._previousSize(expandable_page_view.dart

ciriousjoker suggested changes

View reviewed changes

ciriousjoker left a comment

Boils down to this:

I'd rather use max(0, _sizes.length - 1) instead of adding manual logic.

Haven't tested this code, but it should work out of the box.

lib/src/expandable_page_view.dart

                   _sizes = _prepareSizes();
                   _pageController = widget.controller ?? PageController();
                   _pageController.addListener(_updatePage);
-                  _currentPage = _pageController.initialPage.clamp(0, _sizes.length - 1);
+                  final sizesLength = _sizes.length;
+                  final sizesUpperBounds = sizesLength == 0 ? 0 : sizesLength - 1;

ciriousjoker May 24, 2024

use _sizes.isEmpty instead of checking if length is 0. Irrelevant though, see below

lib/src/expandable_page_view.dart

-                  _currentPage = _pageController.initialPage.clamp(0, _sizes.length - 1);
+                  final sizesLength = _sizes.length;
+                  final sizesUpperBounds = sizesLength == 0 ? 0 : sizesLength - 1;
+                  _currentPage = _pageController.initialPage.clamp(0, sizesUpperBounds);

ciriousjoker May 24, 2024

Suggestion:
_currentPage = _pageController.initialPage.clamp(0, max(0, _sizes.length - 1));

lib/src/expandable_page_view.dart

                   _sizes = _prepareSizes();
                   _pageController = widget.controller ?? PageController();
                   _pageController.addListener(_updatePage);
-                  _currentPage = _pageController.initialPage.clamp(0, _sizes.length - 1);
+                  final sizesLength = _sizes.length;
+                  final sizesUpperBounds = sizesLength == 0 ? 0 : sizesLength - 1;

ciriousjoker May 24, 2024

I'd generally advise against creating pointless variables for this. It doesn't save any space and it prevents the linter from telling you to use isEmpty instead of .length == 0

lib/src/expandable_page_view.dart

                     widget.onPageChanged?.call(_currentPage);
                     _previousPage = (_currentPage + differenceFromPreviousToCurrent)
-                        .clamp(0, _sizes.length - 1);
+                        .clamp(0, sizesLength - 1);

ciriousjoker May 24, 2024

Same solution as above:
max(0, _sizes.length - 1)

lib/src/expandable_page_view.dart

                   }
-                  _previousPage = _previousPage.clamp(0, _sizes.length - 1);
+                  final sizesUpperBounds = sizesLength == 0 ? 0 : sizesLength - 1;
+                  _previousPage = _previousPage.clamp(0, sizesUpperBounds);

ciriousjoker May 24, 2024

Same solution as above:
max(0, _sizes.length - 1)

Author

jayjah commented May 29, 2024

@ciriousjoker I think you're completely right. I'll check your suggestion and make adjustments.

ciriousjoker commented Jul 1, 2024

@jayjah any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet