Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/builder #27

Merged
merged 12 commits into from Jul 26, 2021
Merged

Feature/builder #27

merged 12 commits into from Jul 26, 2021

Conversation

theiskaa
Copy link
Member

Resolves: #26

Require to take suggestionList, textController, and itemBuilder.

class BuilderExample extends StatelessWidget {
  final textEditingController = TextEditingController();
  List<String> suggestionsList = ['test@gmail.com', 'test1@gmail.com'];

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: FieldSuggestion.builder(
        hint: 'Email',
        textController: textEditingController,
        suggestionList: suggestionsList,
        itemBuilder: (BuildContext context, int index) {
          return GestureDetector(
            onTap: () => textEditingController.text = suggestionsList[index],
            child: Card(
              child: ListTile(
                title: Text(suggestionsList[index]),
                leading: Container(
                  height: 30,
                  width: 30,
                  decoration: BoxDecoration(
                    color: Colors.blueGrey,
                    shape: BoxShape.circle,
                  ),
                  child: Center(
                    child: Text(suggestionsList[index][0].toUpperCase()),
                  ),
                ),
              ),
            ),
          );
        },
      ),
    );
  }
}

@theiskaa theiskaa added the feature New feature or request label Jul 24, 2021
@theiskaa theiskaa self-assigned this Jul 24, 2021
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #27 (97dd91e) into develop (b9d8a44) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #27      +/-   ##
===========================================
+ Coverage    99.52%   99.55%   +0.02%     
===========================================
  Files            4        4              
  Lines          211      224      +13     
===========================================
+ Hits           210      223      +13     
  Misses           1        1              
Impacted Files Coverage Δ
lib/src/field_suggestion.dart 99.43% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9d8a44...97dd91e. Read the comment docs.

@g-scosta
Copy link

g-scosta commented Jul 24, 2021

Hey @theiskaa, thanks for the PR. Correct me if I'm wrong but I think this index that are exposed in the builder refers to the index of matchers. So if we have a list of 10 items and find 2 matchs the index will be 0 and 1.

Before passing the index to the exposed builder we could do suggestionsList.indexOf(matchers[index]].

@theiskaa
Copy link
Member Author

@g-scosta Hi man, I can't figure out what you exactly mean, can you tell me more about it?

@g-scosta
Copy link

g-scosta commented Jul 24, 2021

@theiskaa of course! The index that you are passing to the itemBuilder is the index coming from the itemBuilder inside ListView.separated that receives the matchers.length as a argument. So this item builder will always have the index from 0 to the length of the matchers.

Inside suggestionListItem(index) you use the index in matchers[index] but we don't have access to matchers outside FieldSuggestion.

So if we have a list of 10 items and the last three is in the matchers, the index will be 0-2 and not 7-9. If we do suggestionsList[index] in the itemBuilder we will get the first three values, not the last. The index will always start in 0 and go to the length of the matchers, don't matter where the matcher value is in the original list.

EDIT: I realized that we can't do widget.suggestionList.indexOf(matchers[index]) to get the right index (the one that refers to the list, not to the matchers) because the suggestionList is a List of objects and matchers is a list of maps. We could pass the value instead the index (matchers[index]) in the itemBuilder or we could use reduce to transform the list in a list of maps and then find the right index.

@theiskaa
Copy link
Member Author

@g-scosta Understood, Okay I'll think about that problem, thanks for reporting.

@theiskaa
Copy link
Member Author

Yeah, everything is fine now, @g-scosta you can make a review/test and if you are done, then we can merge the PR and close the issue.

@g-scosta
Copy link

Hey @theiskaa I tested and now it's working, but I noticed a great perfomance issue, I have 500+- items to be filtered and the fps dropped a lot. This is probably due to transforming the list of objects into a list of maps everytime inside itemBuilder.

I did some adjustments and tested and with these fixes it's working fast. I'll post these adjustments here:

Instead of transforming the list inside itemBuilder I've created a variable List<dynamic> suggestionListJson = <dynamic>[]; inside _FieldSuggestionState and after initState I did the transformation

super.initState();
    suggestionListJson =
        widget.suggestionList.map((e) => e.toJson().toString()).toList();
    // Add listener to textController, for listen field and create matchers list.
    widget.textController.addListener(_textListener);

So the part that you assign the indexes variable became

// We need indexes to determine right index of concrete item.
                    var indexes = matchers.map((e) {
                      // If suggestion list isn't Object list then just return index of "e".
                      if (!isObjList(widget.suggestionList))
                        return widget.suggestionList.indexOf(e);

                      return suggestionListJson.indexOf(e.toString());
                    }).toList();

I also noticed that return the ListView it's unecessary, it can return the itemBuilder like that

return widget.itemBuilder!(context, indexes[i]);

I'm testing with these fixes and it's on normal perfomance. I think changing that would be enough to release.

@theiskaa
Copy link
Member Author

Really perfect detections, thanks again 😄Okay, I'll add them.

@theiskaa
Copy link
Member Author

@g-scosta Done! Everything applied/added what you mentioned above. Just check it and if you're ok then we can merge the PR.

@g-scosta
Copy link

Great! I think it's ready for merge

@theiskaa
Copy link
Member Author

Ok, Thank you again for your time and request and for the great idea!

@theiskaa theiskaa merged commit 15472b9 into develop Jul 26, 2021
@theiskaa theiskaa deleted the feature/builder branch July 26, 2021 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: builder factory i.e FieldSuggestion.builder()
2 participants