flutter_textfield_search
flutter_textfield_search copied to clipboard
alexrindone
List container fix
based on issue
worked on:
- the result container expands to accommodate longer lists.
added
- scrollbar for a list with room for tweaking
- shapeborder to allow for styling the container (not related to the issue)
@alexrindone Please review
Thank you for the PR! I'll review this in a bit. Were there any tests you are able to write to add for code coverage?
Alright, we need some tests and an implementation for the example @maykhid
@alexrindone Okay. Have you looked at the code? Do you have any features in mind I should be doing a test for?
@maykhid I did take a look at the code, and you had some great ideas there! One thing that I'll show you is that I built out a way to accept a ScrollbarTheme so that we don't have to worry about the extra class you were creating with very specific properties. This makes it a little bit more extensible as a library, while allowing the user to use something that's already built within flutter for handling the design of the scrollbar. I'll have a PR up and deployed today hopefully.
As far as tests go, when writing them you would want to make sure the lines of code you write are covered by tests that make sure the widgets you added are in fact within the widget tree, and have the properties and styling you would expect for the widget you were using. Let me know if you want more clarity.
@alexrindone I saw your recent push to the master branch and I saw what you did with the Scrollbar theme, pretty cool. I guess with that implementation, I'll be removing my implementation of the Scroll from my code.
Also, what did you think of the way I Implemented the height of the search result overlay? I couldn't think of a way to make it truly flexible depending on the number of items on the list. I think making the list overlay extend longer would be pretty cool.
As for the tests I'll be jumping on that soon. Thanks.
So I actually implemented something for the height, which was the itemsInView property (naming is always tough) based on what you had showed for setting a height. It's a little more dynamic where it's default is 3 items high, but if you want more you can set it higher, but it will take whatever result is smaller, so if you have 3 items found, but set 5 items in view, then it will only be as high as the three items found. Does that work or would you rather have the white space if there are 3 items but you had the height much larger? As far as tests, take a look at the changes I made for tests and it should give you an idea on how to write them. Flutter front end testing can be a little strange at first!
On Wed, Apr 20, 2022, 12:27 AM Ifebunandu Henry @.***> wrote:
@alexrindone https://github.com/alexrindone I saw your recent push to the master branch and I saw what you did with the Scrollbar theme, pretty cool. I guess with that implementation, I'll be removing my implementation of the Scroll from my code.
Also, what did you think of the way I Implemented the height of the search result overlay? I couldn't think of a way to make it truly flexible depending on the number of items on the list. I think making the list overlay extend longer would be pretty cool.
As for the tests I'll be jumping on that soon. Thanks.
— Reply to this email directly, view it on GitHub https://github.com/alexrindone/flutter_textfield_search/pull/42#issuecomment-1103560971, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE5G6H5EEGVTU4DEKRKTAYTVF6WWHANCNFSM5TSFOJ6A . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Actually, no. I didn't.
On Sat, Apr 16, 2022, 20:33 Alex Rindone @.***> wrote:
Thank you for the PR! I'll review this in a bit. Were there any tests you are able to write to add for code coverage?
— Reply to this email directly, view it on GitHub https://github.com/alexrindone/flutter_textfield_search/pull/42#issuecomment-1100739375, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANCPQJYKCFGHBY2ZRETXY4TVFMIZNANCNFSM5TSFOJ6A . You are receiving this because you authored the thread.Message ID: @.***>