Made the cancellation widget's area next to the search bar more dynamic. #6
Conversation
There was a problem hiding this comment.
Hi !
Thank you a lot for your help for this package :)
I really like your idea to make the size of the cancellation widget more reasonable.
I perfectly understand that MediaQuery.of(context).size.width * .2 is quite a random size !
I like the way you thought about it but I am not sure it would work.
Indeed, the cancellationWidget is not fully rendered until the animation end.
That leads to _cancellationWidgetKey.currentContext being null at first call and a weird wrap of the text when growing.
So I am not sure about the implementation, although I really like and understand the idea.
What do you think of letting the user give the wanted size ?
Something like :
cancellationWidgetWidth : 200
This way, there is no calculation needed in the SearchBar's package and the User can put any size he wants.
By default, the package would keep the MediaQuery.of(context).size.width * .2
lib/flappy_search_bar.dart
Outdated
| @@ -405,12 +406,45 @@ class _SearchBarState<T> extends State<SearchBar<T>> | |||
| duration: Duration(milliseconds: _animate ? 1000 : 0), | |||
| child: AnimatedContainer( | |||
| duration: Duration(milliseconds: 200), | |||
| width: | |||
| _animate ? MediaQuery.of(context).size.width * .2 : 0, | |||
| width: () { | |||
There was a problem hiding this comment.
May we make a separated method with this logic ?
In order to not make the render method too long
There was a problem hiding this comment.
oh yeah that is a good idea
lib/flappy_search_bar.dart
Outdated
| // Get the width of the cancellation widget. | ||
| final RenderBox renderBox = _cancellationWidgetKey | ||
| .currentContext | ||
| .findRenderObject(); |
There was a problem hiding this comment.
_cancellationWidgetKey.currentContext is null at first call and the app crash
lib/flappy_search_bar.dart
Outdated
| double toBeReturned; | ||
|
|
||
| if (_animate) { | ||
| final fullWidth = MediaQuery.of(context).size.width; |
There was a problem hiding this comment.
You have just forgotten the type : final double fullWidth
lib/flappy_search_bar.dart
Outdated
| allows the width of the widget to be used. | ||
| */ | ||
| child: Container( | ||
| key: _cancellationWidgetKey, |
There was a problem hiding this comment.
You can maybe use the Container above to put the key, not sure if this other one is really needed
There was a problem hiding this comment.
oh yeah i guess i just didn't notice that. I didn't want to change much of the existing code incase i broke something.
|
Thanks for the detailed feedback I really do appreciate it. As you can see I am very new to flutter. A lot of what you said made sense especially the line wrapping part as I noticed that in my testing but didn't know why. Also, most of my testing was with the icon widget for which it worked fine as far as I tested (which was honestly not much). Anyway, I agree with you and think to allow the user to define the size would make more sense since there would be less logic to run to build the widget. As a bonus, if the user wants he/she can easily just make do that logic themselves with their own code. That's just what I wanted to say but feel free to not accept the pull request and make your own better implementation or edit mine. Hopefully that made sense (I'm also new to contributing to stuff as well). |
|
Hi @urwrstkn8mare ! First of all, I am sorry for the delay. Thanks a lot for your work anyway ! |
|
No problem @ThomasEcalle. This is a weird time. For me, the schools have shut down which I thought meant more time until they gave me a bunch of work online so that's why I've been a little time-constrained. Anyway, thanks for the help with this. I basically redid it with your requested changes which seemed very good to me. I'm a little time restrained right now as well so I haven't been able to test it at the moment. I will try to test it within a week and come back. But if you want to you can test it instead. |
Basically I used an Icon widget instead of a text widget and found that there was way too much space on either side of the widget. The change I made gets the width of the cancellation widget, adds a little bit of padding to the left and right, and then makes the total width plus padding the space taken away from the search bar for the cancellation widget.
NOTE: The image below is using my updated code but I just adjusted the padding to illustrate close to what it looked like before.
As you can see in the image above there was too much space on either side. The changes I've made just makes it a little better: