Stepper updates for flutter_configurator #13
Conversation
mighttymike
force-pushed
the
feature/bouwadvies-stepper-updates
branch
from
0151686 to
9e86b90
Compare
| @override | ||
| Widget build(BuildContext context) { | ||
| var step = steps[index]; | ||
| if (currentStep - 1 > index && stepperTheme.hideStepWhenDone) { |
There was a problem hiding this comment.
I find it weird why the currentStep is offset by one. I pretty sure this is all through the package which I find weird.
Apart from that I think the currentStep should be provided with the offset removed when provided as parameter instead in this widget.
| if (showOnlyCurrentStep) ...[ | ||
| SizedBox( | ||
| height: index == steps.length - 1 ? 0 : stepperTheme.emptyHeight, | ||
| height: index == steps.length - 1 ? 0 : lineHeight, |
There was a problem hiding this comment.
This is confusing. I have a different thought with the term "lineHeight" then "emptyHeight" but is used the same way.
There was a problem hiding this comment.
I think we can change this back
lib/src/stepper.dart
Outdated
| child: Column( | ||
| children: [ | ||
| GestureDetector( | ||
| onTap: () {}, |
There was a problem hiding this comment.
Does this serve any purpose?
lib/src/stepper.dart
Outdated
| currentStep: currentIndex, | ||
| lineHeight: currentIndex > index && | ||
| stepperTheme.hideStepWhenDone | ||
| ? 40 |
There was a problem hiding this comment.
Using a hard value. Could this be a problem with implementations in other projects?
There was a problem hiding this comment.
Should this be a new test instead of changing an existing one?
There was a problem hiding this comment.
Shouldn't we start splitting this file up in different files?
freekvandeven
changed the title
No description provided.