search

smartnsoft / flappy_search_bar

SearchBar widget to handle most of search cases
MIT License
173 stars 93 forks source link

Made the cancellation widget's area next to the search bar more dynamic. #6

Open urwrstkn8mare opened 4 years ago

urwrstkn8mare commented 4 years ago

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. image 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: image

urwrstkn8mare commented 4 years ago

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).

ThomasEcalle commented 4 years ago

Hi @urwrstkn8mare !

First of all, I am sorry for the delay. The situation in my company (in France) with the Covid-19 has forced us to put our open source projects on the side for a moment... But I am back ! :)

Thanks a lot for your work anyway ! I'll try to be more present on this package, despite the difficult context that had lead us to change a lot the way we work here :)

urwrstkn8mare commented 4 years ago

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.