search

alexrindone / flutter_textfield_search

FTFS is a Flutter package which uses a TextField Widget to search and select a value from a list. It's a simple, lightweight, and fully tested package unlike other "autocomplete" or textfield search packages with 100% code coverage.
BSD 2-Clause "Simplified" License
27 stars 33 forks source link

List container fix #42

Closed maykhid closed 2 months ago

maykhid commented 2 years ago

based on issue

worked on:

added

@alexrindone Please review

alexrindone commented 2 years ago

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?

maykhid commented 2 years ago

I didn't write any new tests

alexrindone commented 2 years ago

Alright, we need some tests and an implementation for the example @maykhid

maykhid commented 2 years ago

@alexrindone Okay. Have you looked at the code? Do you have any features in mind I should be doing a test for?

alexrindone commented 2 years ago

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

maykhid commented 2 years ago

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

arindone1679 commented 2 years ago

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: @.***>

maykhid commented 2 years ago

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: @.***>